Bug 77267 - MPX does not work in a presence of "-Wl,-as-needed" option (Ubuntu default)
Summary: MPX does not work in a presence of "-Wl,-as-needed" option (Ubuntu default)
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: target (show other bugs)
Version: 6.1.0
: P3 normal
Target Milestone: ---
Assignee: Not yet assigned to anyone
URL:
Keywords:
Depends on:
Blocks: CHKP
  Show dependency treegraph
 
Reported: 2016-08-16 13:25 UTC by Alexander Ivchenko
Modified: 2017-10-25 13:26 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Ivchenko 2016-08-16 13:25:36 UTC
When compiling with -Wl,-as-needed libmpx.so and libmpxwrappers.so are not linked:
>gcc -fcheck-pointer-bounds -mmpx any_mpx_test.c -Wl,-as-needed
> ldd a.out 
        linux-vdso.so.1 (0x00007ffc687a0000)
        libc.so.6 => /lib64/libc.so.6 (0x00000035c8c00000)
        /lib64/ld-linux-x86-64.so.2 (0x000055e88586f000)

And hence, MPX does not work.

(In a working scenario it should be like that:
> ldd a.out 
        linux-vdso.so.1 (0x00007fff53fe7000)
        libmpx.so.0 => /lib64/libmpx.so.0 (0x00007f135ac34000)
        libmpxwrappers.so.0 => /lib64/libmpxwrappers.so.0 (0x00007f135aa31000)
        libc.so.6 => /lib64/libc.so.6 (0x00000035c8c00000)
        libpthread.so.0 => /lib64/libpthread.so.0 (0x00000035c9400000)
        /lib64/ld-linux-x86-64.so.2 (0x0000558619680000)
)

This case is important because on Debian/Ubuntu -no-as-needed option is always implicitly added while linking.
Comment 1 Sergey 2016-08-22 13:13:17 UTC
  Why can't we engage usual linkage mechanism by adding some undefined weak symbol   in mpx code and its definition in mpx library ? ( probably two different ones, for libmpx and libmpxwrappers ). Then we wouldn't have to juggle with (no-)as-needed.
Comment 2 Peter Wu 2016-09-02 21:55:20 UTC
Building Wireshark on Arch Linux with the -Wl,--as-needed linker option also results in issues.

Example output during the linker stage (with -mmpx -fcheck-pointer-bounds):
/usr/lib/gcc/x86_64-pc-linux-gnu/6.2.1/../../../../lib/libmpxwrappers.so: undefined reference to `get_bd'
collect2: error: ld returned 1 exit status

$ readelf -s /usr/lib/libmpxwrappers.so.2.0.0 | grep get_bd
     8: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND get_bd
    72: 0000000000000000     0 NOTYPE  GLOBAL DEFAULT  UND get_bd
$ readelf -s /usr/lib/libmpx.so.2.0.0 | grep get_bd
    38: 0000000000001720     8 FUNC    GLOBAL DEFAULT   13 get_bd@@LIBMPX_2.0
   115: 0000000000001720     8 FUNC    GLOBAL DEFAULT   13 get_bd

What helped here was explicitly adding -lmpxwrappers -lmpx to the linker flags.
Comment 3 Ilya Enkovich 2016-09-05 20:08:37 UTC
(In reply to Sergey from comment #1)
>   Why can't we engage usual linkage mechanism by adding some undefined weak
> symbol   in mpx code and its definition in mpx library ? ( probably two
> different ones, for libmpx and libmpxwrappers ). Then we wouldn't have to
> juggle with (no-)as-needed.

How would we exactly use that symbol?  Is there any chance linker throw away this weak symbol usage as unneeded?  This scheme looks like possible but requires careful design.
Comment 4 hjl@gcc.gnu.org 2016-09-09 21:38:38 UTC
Author: hjl
Date: Fri Sep  9 21:38:06 2016
New Revision: 240057

URL: https://gcc.gnu.org/viewcvs?rev=240057&root=gcc&view=rev
Log:
Fix PR target/77267

2016-09-10  Alexander Ivchenko  <alexander.ivchenko@intel.com>

	PR target/77267
	* config.in: Regenerate.
	* config/i386/linux-common.h (MPX_LD_AS_NEEDED_GUARD_PUSH):
	New macro.
	(MPX_LD_AS_NEEDED_GUARD_PUSH): Ditto.
	(LIBMPXWRAPPERS_SPEC): Remove "--no-whole-archive" from
	static-libmpxwrappers case.
	(LIBMPX_SPEC): Add guards with MPX_LD_AS_NEEDED_GUARD_PUSH and
	MPX_LD_AS_NEEDED_GUARD_POP.
	* configure: Regenerate.
	* configure.ac (HAVE_LD_PUSHPOPSTATE_SUPPORT): New variable.
	defined if linker support "--push-state"/"--pop-state".

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config.in
    trunk/gcc/config/i386/linux-common.h
    trunk/gcc/configure
    trunk/gcc/configure.ac
Comment 5 Matthias Klose 2016-09-16 18:18:25 UTC
I think it's wrong to have HAVE_LD_PUSHPOPSTATE_SUPPORT baked into the driver while it can be easily changed with -fuse-ld=bfd.  It would be very nice to have these push/pop options implemented in gold as well and avoid this kind of work arounds.
Comment 6 Ilya Enkovich 2016-09-16 22:41:23 UTC
(In reply to Matthias Klose from comment #5)
> I think it's wrong to have HAVE_LD_PUSHPOPSTATE_SUPPORT baked into the
> driver while it can be easily changed with -fuse-ld=bfd.  It would be very
> nice to have these push/pop options implemented in gold as well and avoid
> this kind of work arounds.

Should we really not allow gold for MPX?  Also having this feature in gold wouldn't allow us to get rid of this macro because users may use old linker versions.
Comment 7 Matthias Klose 2016-09-17 00:22:44 UTC
yes, you are right about the old linker versions, I forgot about these cases. But it doesn't help the case when you enable the push/pop for a bfd default linker, and the fail with gold.
Comment 8 Alexander Ivchenko 2016-09-19 14:00:58 UTC
Thanks, Matthias, that's a valid point about changing linker on a runtime. In my defense, I see that right now MPX does not work with '-fuse-ld=bfd' anyways:

>gcc test.c -fcheck-pointer-bounds -mmpx -fuse-ld=gold
/usr/bin/ld.gold: bndplt: unknown -z option
/usr/bin/ld.gold: use the --help option for usage information

That's not a surprise, because the check for '--push-state' is done exactly like the check for '-z bndplt'.

Would it be possible to probe the 'other' linker (the one that is not the default one) during a configure time? If so we may end up with something like HAVE_BFD_PUSHPOPSTATE_SUPPORT/HAVE_GOLD_PUSHPOPSTATE_SUPPORT
Comment 9 Alexander Ivchenko 2016-10-10 14:52:27 UTC
Looks like exactly the same discussion happened when '-z bndplt' option was discussed. Here is the reference: https://patchwork.ozlabs.org/patch/456557/ . It's a long one, but seems that the consensus was formulated by Jeff Law:

"I guess the one thing I don't like here is that whether or not to pass 
-z bndplt is made at the time we configure GCC.  Yet, it's trivial for 
someone to change the system linker later, either to gold or from an old 
BFD linker that doesn't support -z bndplt to one that does support -z 
bndplt.

[ Note we have the same issue with certain assembler feature tests. ]

I'm not aware of any real infrastructure in GCC to query the behavior of 
the linker at link time and then customize the options passed.  So if 
it's going to be configurable, then that's the only time to do the test."
...
"So, in an ideal world, we'd query the linker at link time and pass the 
flag anytime we have a linker that supports the capability and perhaps 
warn if the linker doesn't support that capability.

Given we're not in that ideal world, I think Ilya's patch is reasonable 
and should be installed."
Comment 10 Alexander Ivchenko 2016-10-10 14:56:51 UTC
Matthias, could you apply the fix for that bug (https://gcc.gnu.org/viewcvs?rev=240057&root=gcc&view=rev) to the Ubuntu version of gcc?
Comment 11 Alexander Ivchenko 2016-12-02 17:36:59 UTC
Fixed with r240057
Comment 12 Matthias Klose 2017-10-25 13:26:38 UTC
fyi, this is now fixed in Ubuntu 16.04 LTS