Bug 7209 - --as-needed cannot be disabled
Summary: --as-needed cannot be disabled
Status: RESOLVED FIXED
Alias: None
Product: Samba 3.5
Classification: Unclassified
Component: Build environment (show other bugs)
Version: 3.5.0
Hardware: All Linux
: P3 enhancement
Target Milestone: ---
Assignee: Björn Jacke
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-03-04 06:09 UTC by Karolin Seeger
Modified: 2020-12-11 07:23 UTC (History)
7 users (show)

See Also:


Attachments
Patch created by Metze (1.37 KB, patch)
2010-03-04 06:11 UTC, Karolin Seeger
no flags Details
Alternative approach working around this isse (862 bytes, patch)
2010-05-14 07:19 UTC, Olaf Flebbe
no flags Details
Simple test case to demonstrate the GNU LD Bug (400 bytes, application/x-compressed-tar)
2010-05-14 08:03 UTC, Olaf Flebbe
no flags Details
configure.in patch (1.62 KB, patch)
2010-05-24 17:11 UTC, Olaf Flebbe
metze: review-
Details
Patch without offending comment (1.57 KB, patch)
2010-05-25 06:25 UTC, Olaf Flebbe
no flags Details
patch (1.71 KB, patch)
2010-05-27 01:00 UTC, Olaf Flebbe
no flags Details
patch with description and bugid (1.75 KB, patch)
2010-05-27 01:08 UTC, Olaf Flebbe
bjacke: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Karolin Seeger 2010-03-04 06:09:26 UTC
Due to the "--as-needed" option, Samba 3.5. is not linked against libncurses any longer. On RHEL5 x86_64, libreadline is not linked against libncurses (but should be). That's why 'net' is not working on RHEL5-x86_64:

net
net: symbol lookup error: /usr/lib64/libreadline.so.5: undefined symbol: PC

This is not a Samba bug, but it would be nice to be able to disable "--as-needed".
Comment 1 Karolin Seeger 2010-03-04 06:11:27 UTC
Created attachment 5456 [details]
Patch created by Metze

Patch has been pushed to master as 22d316926b9589608d332143c1fa134229b75b3c.
Comment 2 Björn Jacke 2010-03-04 07:53:19 UTC
this is not required, see the commit message of e1ebadb85b250f950d0f5415eda83209f94378c1:

<quote>
Also change order of options so that user supplied LDFLAGS are put *after* the automatic --aѕ-needed flag. This way it's pollible to force not use as-needed by setting LDFLAGS environment variable to "-Wl,--no-as-needed".
</quote>

I think we don't need the extra configure option. IMHO on those broken RHEL5 installations people should just manually set LDFLAGS accordingly.
Comment 3 Björn Jacke 2010-03-04 08:33:21 UTC
gd, simo, andreas: it would be cool if you convince the maintainer of readline to fix the readline linking mess in RHEL5! ;-)
Comment 4 Simo Sorce 2010-03-04 08:37:45 UTC
(In reply to comment #3)
> gd, simo, andreas: it would be cool if you convince the maintainer of readline
> to fix the readline linking mess in RHEL5! ;-)
> 

Björn,
can you open a bug on RH bugzilla ? I will then be able to ask for it to be fixed.
Comment 5 Björn Jacke 2010-03-04 09:01:02 UTC
Simo: there is already one: https://bugzilla.redhat.com/show_bug.cgi?id=499837
Comment 6 Simo Sorce 2010-03-04 09:10:30 UTC
(In reply to comment #5)
> Simo: there is already one: https://bugzilla.redhat.com/show_bug.cgi?id=499837

Ok, I've just read the comments, and they have a good reason not to link readline with ncurses, this means you will have to make sure samba on RHEL5 is linked with libncurses or libtermcap and not leave them out.

To me this looks really a samba bug and we should provide means to fix it.

Simo.
Comment 7 Björn Jacke 2010-03-04 10:13:49 UTC
unresolved symbols of a system library and missing dependencies of it are a bug of samba? ;-) You would need a cristal ball to link against all the libs that  other libs might have failed to link against. It's just a hack of this specific readline package.
Comment 8 Simo Sorce 2010-03-04 10:29:11 UTC
Björn, good or bad as it is, there is a clear explanation in the RH bug as why that can't be changed for now.

We have 2 options, take that in account, or leave 3.5.0 broken on RHEL5.

Asking RH to change RHEL5 in a way that would break other existing applications is simply not possible. As the bug also says this is not a problem in RHEL 6, so this is just another quirk for an old, established platform we need to take in account when building samba.

Simo.
Comment 9 Björn Jacke 2010-03-04 13:24:39 UTC
you just have to disable --as-needed and use --allow-shlib-undefined on RHEL5 by setting before configure
LDFLAGS="-Wl,--allow-shlib-undefined,--no-as-needed".

We might mention this in the release notes, in the wiki or so. Automatic handling would require a configure check for this specific underlinking issue.
Comment 10 Karolin Seeger 2010-03-08 12:24:25 UTC

*** This bug has been marked as a duplicate of bug 6984 ***
Comment 11 Michael Adam 2010-03-09 09:10:40 UTC
(In reply to comment #2)
> this is not required, see the commit message of
> e1ebadb85b250f950d0f5415eda83209f94378c1:
> 
> <quote>
> Also change order of options so that user supplied LDFLAGS are put *after* the
> automatic --aѕ-needed flag. This way it's pollible to force not use as-needed
> by setting LDFLAGS environment variable to "-Wl,--no-as-needed".
> </quote>

Apparently, according to Simos comments, it not as simple as saying that
rhel's libreadline is broken. And I think it is very inconvenient to have
to specify chatty ld flags via the environment. A single configure option
like in Metze's patch is much easier to use and also much easier to find!

I'd vote for the --enable/disable-as-needed patch !!

Michael

> I think we don't need the extra configure option. IMHO on those broken RHEL5
> installations people should just manually set LDFLAGS accordingly.

Comment 12 Michael Adam 2010-03-09 09:24:36 UTC
I think this is not a duplicate of bug 6984.
Rather, 6984 can be a considert a special incarnation of this bug.

I vote for having the use of --as-needed configurable via a ./configure switch.
This bahaviour was introduced with 3.5, and there is no use to make it more difficult than necessary to build on RHEL5 and possibly other platforms.

It is a valid requist to want to be able to turn of the use of --as-needed.

==> I am reopening this bug.

Michael
Comment 13 Björn Jacke 2010-03-09 16:03:22 UTC
It's not that simple. The rhel5 build is only working accidentally if as-needed is disabled. As soon as ncurses linking flags is not in the global LDFALGS any more (there is not much reason for having it there I think...). So it's just a matter of time until simply disabling as-needed will not be enough.

The proper fix for the "special" rhel5 readline lib is to check if linking a test prog against readline without linking against ncurses results in a broken testprog. Then test if the testprog is working with readline also linked in. You actually need to add the ncurses linker flag manually for a proper fix *and* make sure to set --no-as-needed.

Given that, the --disable-as-needed will be of much help. The switch alone will anyway not help those people who try to build there and see that failure.
Comment 14 Michael Adam 2010-03-16 10:13:41 UTC
(In reply to comment #13)
> It's not that simple. The rhel5 build is only working accidentally if as-needed
> is disabled. As soon as ncurses linking flags is not in the global LDFALGS any
> more (there is not much reason for having it there I think...). So it's just a
> matter of time until simply disabling as-needed will not be enough.
> 
> The proper fix for the "special" rhel5 readline lib is to check if linking a
> test prog against readline without linking against ncurses results in a broken
> testprog. Then test if the testprog is working with readline also linked in.
> You actually need to add the ncurses linker flag manually for a proper fix
> *and* make sure to set --no-as-needed.
> 
> Given that, the --disable-as-needed will be of much help. The switch alone will
> anyway not help those people who try to build there and see that failure.


Ah! Seems like there is someone with a clue how to fix this properly! ... :-)

Cheers - Michael
Comment 15 Olaf Flebbe 2010-05-14 06:52:51 UTC
Please add Metzes patch:


An other workaround would be to add something like


extern void tgetent();
static void (*dummyptr)() = tgetent;

into somewhere at lib/replace. This would force the linker to add -lncurses
(termlib, termcap) because of an explicit dependency.

Example code to be compiled on Centos5/RHEL5
---
#ifdef FIX
extern void tgetent();
static void (*dummyptr)() = tgetent;
#endif

int main() 
{
	readline();
}
----
 Works: gcc aa.c -lreadline -lncurses
--
 Does not work: gcc -Wl,--as-needed aa.c -lreadline -lncurses

/usr/lib/gcc/x86_64-redhat-linux/4.1.1/../../../../lib64/libreadline.so: undefined reference to `PC'
/usr/lib/gcc/x86_64-redhat-linux/4.1.1/../../../../lib64/libreadline.so: undefined reference to `tgetflag'
/usr/lib/gcc/x86_64-redhat-linux/4.1.1/../../../../lib64/libreadline.so: undefin
.....
---
Works: gcc -DFIX -Wl,--as-needed aa.c -lreadline -lncurses
----
Comment 16 Olaf Flebbe 2010-05-14 07:19:54 UTC
Created attachment 5708 [details]
Alternative approach working around this isse
Comment 17 Olaf Flebbe 2010-05-14 08:03:37 UTC
Created attachment 5709 [details]
Simple test case to demonstrate the GNU LD Bug
Comment 18 werner maes 2010-05-20 02:46:09 UTC
has this bug been fixed in the latest 3.5.3?
Comment 19 Karolin Seeger 2010-05-20 02:49:27 UTC
(In reply to comment #18)
> has this bug been fixed in the latest 3.5.3?
> 

No, unfortunately not yet. Opinions seem to differ...
Comment 20 Karolin Seeger 2010-05-20 02:50:49 UTC
(In reply to comment #16)
> Created an attachment (id=5708) [details]
> Alternative approach working around this isse

Any volunteer for patch review?

Comment 21 Volker Lendecke 2010-05-20 03:08:59 UTC
It's a fun way to solve this. We should have a configure test for it though or a huge comment that this is a RHEL5 specific thing. Otherwise it will cause grey hair in a couple of years when people try to understand why this is done.

Volker
Comment 22 Stefan Metzmacher 2010-05-20 03:54:59 UTC
I don't like this patch as introduce an explicit dependency on all systems
Comment 23 Volker Lendecke 2010-05-20 04:01:36 UTC
Right. That's why I mentioned the configure check.

Volker
Comment 24 Olaf Flebbe 2010-05-20 05:15:59 UTC
Yes indeed, my patch was more or less a joke.

Hint: The testcase appended can be extended to work as a configure check. 

Comment 25 Björn Jacke 2010-05-20 16:44:18 UTC
the check from comment 17 checks for some bug in ld but that seems not to be related to an underlinked readline.
Comment 26 Olaf Flebbe 2010-05-21 01:29:55 UTC
(In reply to comment #25)
> the check from comment 17 checks for some bug in ld but that seems not to be
> related to an underlinked readline.
> 

Of course it is. 

The linker line reads:  .. objects.o ... --as-needed -lreadline -lncurses.

-lncurses is wrongly removed by ld because there is no direct symbol resolving in the objects. It does not honor second-order dependent symbols from readline library, even if stated on the command line. this is why my silly patch proposal works ...

It is an LD bug. 
Comment 27 Stefan Metzmacher 2010-05-21 01:40:33 UTC
Isn't that exactly what --as-needed is for? It should skip unused libraries
which are provided on the cmdline. And we use it because cups-options and krb5-config sometimes list unused libraries.
Comment 28 Olaf Flebbe 2010-05-21 04:21:09 UTC
Yes, but there are subtile cases.

--as-needed should only delete libraries which are unused. But it does delete libraries which are needed, in the special case of secon order dependencies. 

Please consider the example attached: On rhel5 it does

On RHEL5 

+ gcc -c -fPIC sha.c
+ gcc -c -fPIC shb.c
+ gcc --shared -fPIC -o liba.so sha.c
+ gcc --shared -fPIC -o libb.so shb.c
+ echo if this fails, gnu ld has a bug
if this fails, gnu ld has a bug
+ gcc -o a.out a.c -Wl,--as-needed -L. -la -lb
./liba.so: undefined reference to `b'
collect2: ld gab 1 als Ende-Status zurück


On Ubuntu 10.04 with

GNU ld (GNU Binutils for Ubuntu) 2.20.1-system.20100303


+ gcc -c -fPIC sha.c
+ gcc -c -fPIC shb.c
+ gcc --shared -fPIC -o liba.so sha.c
+ gcc --shared -fPIC -o libb.so shb.c
+ echo if this fails, gnu ld has a bug
if this fails, gnu ld has a bug
+ gcc -o a.out a.c -Wl,-as-needed -L. -la -lb

No Error
--------


In order to translate to the samba/rhel5 case:

libb.so == libncurses.so
b() == tgetent ....

liba.so == libreadline.so
a() == readline()

a.c == smbclient ...

SuSE and others link their readline differently:

gcc --shared -fPIC -o libb.so shb.c 
gcc --shared -fPIC -o liba.so sha.c -L. -lb

readline does now have an explicit dependency to ncurses. But not (by design) in RHEL. 
Comment 29 Björn Jacke 2010-05-22 08:17:29 UTC
In the compile of a.out you link explicitly against libb and liba even though you only use a symbol from liba. You already did the mistake to not link liba against libb so you produce an underlinked library which is the main problem here. If you would have linked liba against libb then liba would have liba in the DT_NEEDED elf header. I already said that a cristal ball would be needed to know which libraries we use lack which dependencies which we would then need to manually add ourself (like ncurses which we don't need to link against ourself in most places).
Comment 30 Olaf Flebbe 2010-05-24 17:11:18 UTC
Created attachment 5736 [details]
configure.in patch

configure.in patch: Tries to link with readline, if it worked before.
if this fails revert detection of --as-needed.
Comment 31 Olaf Flebbe 2010-05-24 17:13:41 UTC
(In reply to comment #29)

you miss the point: libreadline *is* underlinked on RHEL.  
Comment 32 Stefan Metzmacher 2010-05-25 00:31:45 UTC
Comment on attachment 5736 [details]
configure.in patch

This comment is wrong:

+# check wether a libreadline triggers a bug in ld with --as-needed
+# i.e. RHEL5

This isn't a bug in ld, it's a bug in libreadline on RHEL5.
Comment 33 Stefan Metzmacher 2010-05-25 00:32:31 UTC
Comment on attachment 5736 [details]
configure.in patch

I guess the patch itself looks good, but I haven't tested it myself.
Comment 34 Olaf Flebbe 2010-05-25 02:56:48 UTC
(In reply to comment #32)
> (From update of attachment 5736 [details])
> This comment is wrong:
> 
> +# check wether a libreadline triggers a bug in ld with --as-needed
> +# i.e. RHEL5
> 
> This isn't a bug in ld, it's a bug in libreadline on RHEL5.
> 

This *is* an ld Bug. 

Please compare the output for instance if "ldd gpg" und "ldd lftp" on Centos5/RHEl5.

one of them it is linked with both termcap and readline and the other with ncurses and the same readline.

This is by design. You have the choice to use readline with any terminal library you like. I double checked with Florian La Roche, who was with RedHat for some time. 

If you don't trust me, PLEASE try my demonstration on for instance ubuntu 10.04 with an recent ld , you will see that underlinking does not trigger this kind of bug any more.

Comment 35 Björn Jacke 2010-05-25 03:24:16 UTC
maybe it becomes more clear that we do not suffer from a GNU ld bug when we remove the useless linking agains ncurses from the Makefile? The configure check will also need the test for ncurses dependency to be complete, see comment #13. We will probably remove the ncurses linker flags and than simply not using --as-needed will make the undelinked readline fail to link again.
Comment 36 Olaf Flebbe 2010-05-25 04:17:55 UTC
Do not understand me: I am not an advocate for the RedHat design decision. I only want you to understand, what they were trying to achieve:

The -lncurses is not useless

Which terminal library you do want to link against? You'll have to explicitly state which terminal library you want. For Linux, this is a RedHat "Feature": You have the choice to link readline against -lncurses or -ltermap. Both is possible. Other Linux distros do not support this.

And BTW: There are UNIX systems which do not know about library dependencies. For instance having a static libreadline.a with PIC code. So you'll have to test which one works.




Comment 37 Björn Jacke 2010-05-25 04:40:57 UTC
The pros and cons of this specially linked library the ABI problems it has and whatever else is not in the scope of this samba bug and should really be discusses elsewhere.

All which is of interest at this place is what a complete configure check for that would need, see above.

As this bug is about "--as-needed cannot be disabled" and as-needed is just the GNU name of what other linkers do with "-z ignore" I would suggest we add a configure option "--ignore-underlinked-libs" which does almost the same as Metze's initial patch but in turn also disables the -z defs settings.
Comment 38 Olaf Flebbe 2010-05-25 04:49:44 UTC
I proposed a working and complete configure.in patch. So please apply.
Comment 39 Olaf Flebbe 2010-05-25 06:25:40 UTC
Created attachment 5737 [details]
Patch without offending comment
Comment 40 Andrew Bartlett 2010-05-26 05:07:00 UTC
The patch should include as comments, just as Volker notes, a clear, unemotional description of the issue.  Bugzilla references are good, but given how long this will be with us, and we may change bug tracker in the next 10 years, a long clear description is better. 
Comment 41 Olaf Flebbe 2010-05-27 00:43:43 UTC
(In reply to comment #40)
> The patch should include as comments, just as Volker notes, a clear,
> unemotional description of the issue.  Bugzilla references are good, but given
> how long this will be with us, and we may change bug tracker in the next 10
> years, a long clear description is better. 
> 

I already did it before.

Ok, now the scientific way:

Take /usr/lib/libreadline.so and /usr/lib/libncurses.so from a RHEL5 system (I will append a tgz to this report) and put it in a directory

use this test.c in the same dir.
main() { readline(); }

Do this on almost any non-RHEL5 system (I tested on SuSE > 11.0, Ubuntu 10.04)
cc -Wl,--as-needed test.c -L. -lreadline -lncurses

No Problem.

No do it on RHEL5:
cc -Wl,--as-needed test.c -L. -lreadline -lncurses
... unresolved symbols ...
 
The only relevant parameter I changed (well, forget about the compiler,libc) is the linker. So I have to deduce it is an linker bug.

So Metzes  negative review is invalid. 


Comment 42 Olaf Flebbe 2010-05-27 00:47:27 UTC
Sorry to correct myself: The example works on RHEL6 Beta, Ubuntu 10.04, not on Suse >11.0


> Do this on almost any non-RHEL5 system (I tested on SuSE > 11.0, Ubuntu 10.04)
> cc -Wl,--as-needed test.c -L. -lreadline -lncurses
> 
Comment 43 Olaf Flebbe 2010-05-27 00:51:37 UTC
O.k. you got me: Not a ld bug ... while retesting...
Comment 44 Olaf Flebbe 2010-05-27 01:00:43 UTC
Created attachment 5741 [details]
patch

Patch with longer description
Comment 45 Stefan Metzmacher 2010-05-27 01:03:07 UTC
Comment on attachment 5737 [details]
Patch without offending comment

Sorry, I already changed the negative review flag into a positive one,
but didn't managed to store it (as I didn't noticed that firefox asked me for a password...)
Comment 46 Stefan Metzmacher 2010-05-27 01:04:54 UTC
Comment on attachment 5741 [details]
patch

Can you readd the reference to the bug id?
Comment 47 Olaf Flebbe 2010-05-27 01:08:17 UTC
Created attachment 5742 [details]
patch with description and bugid

surely I can reference the bugid
Comment 48 Volker Lendecke 2010-06-11 14:00:38 UTC
I would call this a blocker. It is pretty embarassing that on RHEL5 ./configure;make does not work out of the box.

Volker
Comment 49 Simo Sorce 2010-06-11 14:47:54 UTC
Karolin,
the patch works for me on RHEL5 and looks like the right approach.
I am going to push it in master, and would like to see it in 3.5
Comment 50 Simo Sorce 2010-06-11 15:09:00 UTC
Signed off and pushed to master
Comment 51 Simo Sorce 2010-06-11 15:10:21 UTC
Reassigning to Karolin for inclusion in 3.5.x
Comment 52 Björn Jacke 2010-06-11 16:56:51 UTC
Comment on attachment 5742 [details]
patch with description and bugid

simo, this is no working fix. Did you test it? There needs to be tested more things. Partly this is written above, I know that metze has a good plan how to cleanly add all the required tests at the apropriate places.
Comment 53 Simo Sorce 2010-06-11 17:26:38 UTC
(In reply to comment #52)
> (From update of attachment 5742 [details])
> simo, this is no working fix. Did you test it?

Of course I did.
Current master does not build on RHEL5, and this patches makes it build just fine.

> There needs to be tested more things.

make on RHEL5 tested it just fine ;-)

> Partly this is written above, I know that metze has a good plan how to
> cleanly add all the required tests at the apropriate places.

We can improve these patches if you want, but they do allow me to build on RHEL5. Given nobody seem to have better patches yet, and these patches makes 3.5.x buildable on RHEL, they are good enough to go in.

Simo.
Comment 54 Karolin Seeger 2010-06-14 02:01:51 UTC
Reassigning to Metze as he had an idea for a proper fix if I remember correctly.
Comment 55 Volker Lendecke 2010-06-16 07:35:48 UTC
RH has supported packages. SerNet has supported current packages. The ability to build from source for RHEL is a nice enhancement but not a blocker.

Volker
Comment 56 Olaf Flebbe 2010-06-16 07:53:25 UTC
You claim that it breakes something without giving an explanation why it breaks or at least an example where or does breaks code.


Comment 57 Björn Jacke 2010-06-16 10:55:58 UTC
last time I tried with just leaving out --as-needed at least the net binary compiled but refused to work (and complaining about unresolved symbols) unless also  --allow-shlib-undefined was also used at compile time. It retested with the current 3.5 release and it actually worked this time however. I'll therefore remove my - flag as it's not broken like I thought.
The whole test for as-needed should eventually be moved up in configure.in so that all the other linker tests are being done with the as-needed flags that we use in the final compile.
Comment 58 Karolin Seeger 2010-06-17 05:26:22 UTC
Ok, as there are two positive reviews now, I just pushed the patch to v3-5-test.
Will be included in 3.5.4.

Closing out bug report.

Thanks a lot! :-)
Comment 59 Volker Lendecke 2010-07-18 04:04:14 UTC
In FreeBSD, -lkrb5 does not explicitly depend on -lasn1. This leads to initialize_asn1_error_table_r not being available.

Volker
Comment 60 Volker Lendecke 2010-07-18 05:17:35 UTC
Fixed with b9835a1f9d8b81. as-needed is gone :-)
Comment 61 Gordon Lack 2011-04-07 16:24:18 UTC
Just FYI (in case anyone else hits this).

The fix doesn't work for RedHat4.x, as there the link succeeds (so configure thinks it is OK to use --as-needed), but the *execution* fails (so it actuality isn't). 

This patch (for 3.5.8) runs the test program too.

--- source3/configure.orig      2011-03-06 18:58:41.000000000 +0000
+++ source3/configure   2011-04-06 17:47:09.000000000 +0100
@@ -52315,7 +52315,7 @@
         test ! -s conftest.err
        } && test -s conftest$ac_exeext && {
         test "$cross_compiling" = yes ||
-        $as_test_x conftest$ac_exeext
+        ( $as_test_x conftest$ac_exeext && ./conftest$ac_exeext )
        }; then
   { $as_echo "$as_me:$LINENO: result: yes" >&5
 $as_echo "yes" >&6; }