Bug 13933 - rpath ability check not working
Summary: rpath ability check not working
Status: RESOLVED INVALID
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: Build (show other bugs)
Version: 4.10.2
Hardware: PPC AIX
: P5 normal (vote)
Target Milestone: ---
Assignee: Björn Jacke
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-08 14:17 UTC by Bert Jahn
Modified: 2019-06-27 12:10 UTC (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Bert Jahn 2019-05-08 14:17:19 UTC
System:
AIX 7100-04-01-1543
xlccmp.16.1.0             16.1.0.1    C     F    XL C compiler

during configure rpath ability is detected as working:
 Checking linker accepts ['-Wl,-R,.']        : yes
 Checking for rpath library support          : yes

build reports:
 ld: 0706-027 The -R libdir flag is ignored.
and rpath is not set in binaries

ld supports -R only if option -bsvr4 is set, otherwise option -R is ignored

But why does configure wrongly detect the abilities?

The check 'Checking linker accepts ['-Wl,-R,.']' succeeds because ld doesn't gives an error on -R (only warning). That is ok.

The check 'Checking for rpath library support' succeeds because it is broken to my understanding. On any platform I would say.

buildtools/wafsamba/samba_waf18.py:
        lib_node = bld.srcnode.make_node('libdir/liblc1.c')
        lib_node.parent.mkdir()
        lib_node.write('int lib_func(void) { return 42; }\n', 'w')
        main_node = bld.srcnode.make_node('main.c')
        main_node.write('int main(void) {return !(lib_func() == 42);}', 'w')
        linkflags = []
        if version_script:
            script = bld.srcnode.make_node('ldscript')
            script.write('TEST_1.0A2 { global: *; };\n', 'w')
            linkflags.append('-Wl,--version-script=%s' % script.abspath())
        bld(features='c cshlib', source=lib_node, target='lib1', linkflags=linkflags, name='lib1')
        o = bld(features='c cprogram', source=main_node, target='prog1', uselib_local='lib1')
        if rpath:
            o.rpath = [lib_node.parent.abspath()]
        def run_app(self):
             args = conf.SAMBA_CROSS_ARGS(msg=msg)
             env = dict(os.environ)
             env['LD_LIBRARY_PATH'] = self.inputs[0].parent.abspath() + os.pathsep + env.get('LD_LIBRARY_PATH', '')
             self.generator.bld.cmd_and_log([self.inputs[0].abspath()] + args, env=env)
        o.post()
        bld(rule=run_app, source=o.link_task.outputs[0])

it creates:
 libdir/liblc1.c
 main.c
compiles to:
 libdir/liblc1.c.1.o
 main.c.2.o
creates lib:
 liblib1.so
creates binary:
 prog1

And now checks if prog1 get the lib loaded. But the lib is located in the same directory as the binary! So this succeeds without a correct set rpath in the binary!

To fix I use the following patch which creates the lib in libdir/ (o.rpath setting should be made better, I'm not familar with WAF...)

--- buildtools/wafsamba/samba_waf18.py.orig	2019-04-25 13:45:49.000000000 +0200
+++ buildtools/wafsamba/samba_waf18.py	2019-04-25 15:05:52.000000000 +0200
@@ -212,10 +212,10 @@
              script = bld.srcnode.make_node('ldscript')
              script.write('TEST_1.0A2 { global: *; };\n', 'w')
              linkflags.append('-Wl,--version-script=%s' % script.abspath())
-         bld(features='c cshlib', source=lib_node, target='lib1', linkflags=linkflags, name='lib1')
+         bld(features='c cshlib', source=lib_node, target='libdir/lib1', linkflags=linkflags, name='lib1')
          o = bld(features='c cprogram', source=main_node, target='prog1', uselib_local='lib1')
          if rpath:
-             o.rpath = [lib_node.parent.abspath()]
+             o.rpath = [lib_node.parent.parent.abspath() + '/testbuild/default/libdir']
          def run_app(self):
               args = conf.SAMBA_CROSS_ARGS(msg=msg)
               env = dict(os.environ)
Comment 1 Andrew Bartlett 2019-06-11 13:01:41 UTC
Can you please send us a DCO and create a full GIT patch and submit it as a merge request?

https://wiki.samba.org/index.php/Using_Git_for_Samba_Development
https://wiki.samba.org/index.php/Samba_CI_on_gitlab#Creating_a_merge_request

The test idea looks reasonable, it would be good to get this upstream.

Thanks!
Comment 2 Bert Jahn 2019-06-17 15:17:57 UTC
Sorry, I don't understand your wiki.
Do I need to become a team member for this?
Comment 3 Andrew Bartlett 2019-06-17 18:02:32 UTC
(In reply to Bert Jahn from comment #2)
No, just join gitlab.com and let me know your username.  I can then let you in to that repo allowing you to schedule a full test and prepare a merge request.

Finally, make sure to send in your DCO and sign off your commits:
https://www.samba.org/samba/devel/copyright-policy.html
Comment 4 Bert Jahn 2019-06-18 08:59:15 UTC
I sent the DCO.
My username at git lab is: bert.jahn
Comment 5 Bert Jahn 2019-06-21 12:11:36 UTC
I have created a merge request.
Branch is bert.jahn-rpath
Comment 6 Björn Jacke 2019-06-21 13:50:05 UTC
the merge request broke the rpath handling on the build machine. I also don't understand what you are doing in your proposed patch and for what reason. Can you explain the changes that you propose here?
Comment 7 Bert Jahn 2019-06-21 14:31:15 UTC
The changes that I propose here are explained in this bug report.
I don't see why this change fails in one of CI builds because the changes do no change the way it will build:
 Checking compiler accepts ['-Werror']    : yes 
 Checking linker accepts ['-Wl,-rpath,.'] : yes 
 Checking for rpath library support       : yes 
Is it possible master branched from was not clean?
Comment 8 Björn Jacke 2019-06-27 12:10:20 UTC
as discussed in https://gitlab.com/samba-team/samba/merge_requests/562 - this patch doesn't actually do what it's expected to do.

From my experience the lxc compiler also generally has the problem that "-qhalt=w" does not halt on all warnings while we do have to expect it to throw an error for tests.

Bert closed the merge request already, I'll close this bug report accordingly. Thanks for your efferts anyway, if you find a good solution, feel free to reopen this bug again.