* Does the LD --wrap feature work for library internal references? @ 2018-12-19 13:11 Sebastian Huber 2019-01-03 15:49 ` Nick Clifton 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2018-12-19 13:11 UTC (permalink / raw) To: binutils Hello, I would like to use the LD --wrap feature to wrap a library internal function (in libbsd.a in this example). https://sourceware.org/binutils/docs/ld/Options.html#index-_002d_002dwrap_003dsymbol For one function this seems to work: /build/rtems/5/bin/powerpc-rtems5-gcc -Wl,--wrap=_bsd_ether_ifattach -qrtems -B/build/rtems/5/powerpc-rtems5/lib/ -B/build/rtems/5/powerpc-rtems5/qoriq_e6500_32/lib/ --specs bsp_specs -mcpu=e6500 -mcpu=e6500 -m32 -m32 -msdata=sysv -msdata=sysv -fno-common -fno-common -ffunction-sections -ffunction-sections -fdata-sections -fdata-sections -Wl,--gc-sections testsuite/media01/test_main.c.38.o testsuite/media01/dpaa_if_input.c.38.o -o /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/media01.exe -Wl,-Bstatic -L. -lbsd -Wl,-Bdynamic -lftpd -ltelnetd -lbsd -lm /build/rtems/5/lib/gcc/powerpc-rtems5/7.4.0/../../../../powerpc-rtems5/bin/ld: ./libbsd.a(if_bridge.c.18.o): in function `bridge_clone_create': /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/../../freebsd/sys/net/if_bridge.c:702: undefined reference to `__wrap__bsd_ether_ifattach' /build/rtems/5/lib/gcc/powerpc-rtems5/7.4.0/../../../../powerpc-rtems5/bin/ld: ./libbsd.a(if_lagg.c.18.o): in function `lagg_clone_create': /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/../../freebsd/sys/net/if_lagg.c:528: undefined reference to `__wrap__bsd_ether_ifattach' /build/rtems/5/lib/gcc/powerpc-rtems5/7.4.0/../../../../powerpc-rtems5/bin/ld: ./libbsd.a(if_vlan.c.18.o): in function `vlan_clone_create': /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/../../freebsd/sys/net/if_vlan.c:1071: undefined reference to `__wrap__bsd_ether_ifattach' /build/rtems/5/lib/gcc/powerpc-rtems5/7.4.0/../../../../powerpc-rtems5/bin/ld: ./libbsd.a(if_fmanmac.c.18.o): in function `fman_mac_dev_attach': /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/../../rtemsbsd/sys/powerpc/drivers/net/ethernet/freescale/dpaa/if_fmanmac.c:471: undefined reference to `__wrap__bsd_ether_ifattach' If I add void __wrap__bsd_ether_ifattach(struct ifnet *ifp, const u_int8_t *lla) { puts("ether_ifattach"); __real__bsd_ether_ifattach(ifp, lla); } then I get the expected output. nm media01.exe | grep ether_ifattach 0001a670 T _bsd_ether_ifattach 00005a30 T __wrap__bsd_ether_ifattach For another it doesn't work: /build/rtems/5/bin/powerpc-rtems5-gcc -Wl,--wrap=_bsd_ether_output -qrtems -B/build/rtems/5/powerpc-rtems5/lib/ -B/build/rtems/5/powerpc-rtems5/qoriq_e6500_32/lib/ --specs bsp_specs -mcpu=e6500 -mcpu=e6500 -m32 -m32 -msdata=sysv -msdata=sysv -fno-common -fno-common -ffunction-sections -ffunction-sections -fdata-sections -fdata-sections -Wl,--gc-sections testsuite/media01/test_main.c.38.o testsuite/media01/dpaa_if_input.c.38.o -o /scratch/git-rtems-libbsd/build/powerpc-rtems5-qoriq_e6500_32-default/media01.exe -Wl,-Bstatic -L. -lbsd -Wl,-Bdynamic -lftpd -ltelnetd -lbsd -lm It links, I would have expected an undefined reference to __wrap__bsd_ether_output. In the executable I find this: nm media01.exe | grep ether_output 0001b640 T _bsd_ether_output It is not clear to me why it works with one function and not with another. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2018-12-19 13:11 Does the LD --wrap feature work for library internal references? Sebastian Huber @ 2019-01-03 15:49 ` Nick Clifton 2019-01-04 7:03 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Nick Clifton @ 2019-01-03 15:49 UTC (permalink / raw) To: Sebastian Huber, binutils Hi Sebastian, > For another it doesn't work: > > /build/rtems/5/bin/powerpc-rtems5-gcc -Wl,--wrap=_bsd_ether_output -qrtems -B/build/rtems/5/powerpc- > It links, I would have expected an undefined reference to __wrap__bsd_ether_output. In the executable I find this: > > nm media01.exe | grep ether_output > 0001b640 T _bsd_ether_output > > It is not clear to me why it works with one function and not with another. Is it possible that the _bsd_ether_output function has been inlined into all the places where it is used ? The wrap feature of the linker works by replacing symbols, but this is only effective if there are undefined references to the symbols being replaced. If all of the references have already been resolved (or just don't exist, eg because of inlining) then wrapping will have no effect. Cheers Nick ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-03 15:49 ` Nick Clifton @ 2019-01-04 7:03 ` Sebastian Huber 2019-01-04 13:49 ` Nick Clifton 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-04 7:03 UTC (permalink / raw) To: Nick Clifton, binutils Hello Nick, On 03/01/2019 16:49, Nick Clifton wrote: > Hi Sebastian, > >> For another it doesn't work: >> >> /build/rtems/5/bin/powerpc-rtems5-gcc -Wl,--wrap=_bsd_ether_output -qrtems -B/build/rtems/5/powerpc- >> It links, I would have expected an undefined reference to __wrap__bsd_ether_output. In the executable I find this: >> >> nm media01.exe | grep ether_output >> 0001b640 T _bsd_ether_output >> >> It is not clear to me why it works with one function and not with another. > Is it possible that the _bsd_ether_output function has been inlined into > all the places where it is used ? > > The wrap feature of the linker works by replacing symbols, but this is > only effective if there are undefined references to the symbols being > replaced. If all of the references have already been resolved (or just > don't exist, eg because of inlining) then wrapping will have no effect. It is not unlined, but its only reference is done via a function pointer assignment here: https://git.rtems.org/rtems-libbsd/tree/freebsd/sys/net/if_ethersubr.c#n950 [...]| /*|| * Perform common duties while attaching to interface list |||| */ | |void ether_ifattach(struct ifnet *ifp, const u_int8_t *lla) { int i; struct ifaddr *ifa; struct sockaddr_dl *sdl; ifp->if_addrlen = ETHER_ADDR_LEN; ifp->if_hdrlen = ETHER_HDR_LEN; if_attach(ifp); ifp->if_mtu = ETHERMTU; ifp->if_output = ether_output; ifp->if_input = ether_input; |[...] Does the wrap feature work for function pointer assignments? -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-04 7:03 ` Sebastian Huber @ 2019-01-04 13:49 ` Nick Clifton 2019-01-04 14:12 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Nick Clifton @ 2019-01-04 13:49 UTC (permalink / raw) To: Sebastian Huber, binutils Hi Sebastian, >> Is it possible that the _bsd_ether_output function has been inlined into > It is not inlined, but its only reference is done via a function pointer assignment here: > ifp->if_output = ether_output; > ifp->if_input = ether_input; > Does the wrap feature work for function pointer assignments? Hmmm, interesting. Is the ether_input function also only referenced in this way ? (Because you said that the ether_input function was being wrapped, yes ?) Generally speaking function pointer assignment should be subject to linker wrapping, but there are still some possible explanations for this behaviour. For example, is the if_output function pointer ever used in the code ? If not, then the compiler might be dropping the initialization of the if_output field, since it is never needed. Cheers Nick ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-04 13:49 ` Nick Clifton @ 2019-01-04 14:12 ` Sebastian Huber 2019-01-04 14:27 ` Nick Clifton 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-04 14:12 UTC (permalink / raw) To: Nick Clifton, binutils On 04/01/2019 14:49, Nick Clifton wrote: > Hi Sebastian, > >>> Is it possible that the _bsd_ether_output function has been inlined into >> It is not inlined, but its only reference is done via a function pointer assignment here: >> ifp->if_output = ether_output; >> ifp->if_input = ether_input; >> Does the wrap feature work for function pointer assignments? > Hmmm, interesting. Is the ether_input function also only referenced > in this way ? (Because you said that the ether_input function was being > wrapped, yes ?) Sorry, for the confusion. Both ether_output() and ether_input() are only referenced by function pointer assignments and both are not wrapped. I can wrap the function ether_ifattach() (this is the function assigning the function pointers). > > Generally speaking function pointer assignment should be subject to > linker wrapping, but there are still some possible explanations for > this behaviour. For example, is the if_output function pointer ever > used in the code ? If not, then the compiler might be dropping the > initialization of the if_output field, since it is never needed. I don't use link-time optimization. I am sure the functions ether_input() and ether_output() are present in the executable. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-04 14:12 ` Sebastian Huber @ 2019-01-04 14:27 ` Nick Clifton 2019-01-04 19:45 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Nick Clifton @ 2019-01-04 14:27 UTC (permalink / raw) To: Sebastian Huber, binutils Hi Sebastian, > I don't use link-time optimization. I am sure the functions ether_input() and ether_output() are present in the executable. OK, but my guess is that the compiler has already done something clever so that those symbols are never referenced, and hence wrapping the symbols has no effect. (You could look in the object file and see if the symbols are used in one or more relocations...) Alternatively maybe there really is a bug in the linker's wrapping code. But I have no idea what it might be, or why this particular situation is causing problems. If you do think that it is a linker bug though, please could you put together a small test case to demonstrate the problem ? Ideally using assembler source files rather than C/C++ sources, so that we do not have to worry about compiler optimizations. Cheers Nick ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-04 14:27 ` Nick Clifton @ 2019-01-04 19:45 ` Sebastian Huber 2019-01-05 18:51 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-04 19:45 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils [-- Attachment #1: Type: text/plain, Size: 1059 bytes --] Hello Nick, ----- Am 4. Jan 2019 um 15:27 schrieb Nick Clifton nickc@redhat.com: > Hi Sebastian, > >> I don't use link-time optimization. I am sure the functions ether_input() and >> ether_output() are present in the executable. > > OK, but my guess is that the compiler has already done something clever > so that those symbols are never referenced, and hence wrapping the symbols > has no effect. (You could look in the object file and see if the symbols > are used in one or more relocations...) > > Alternatively maybe there really is a bug in the linker's wrapping code. > But I have no idea what it might be, or why this particular situation is > causing problems. If you do think that it is a linker bug though, please > could you put together a small test case to demonstrate the problem ? > Ideally using assembler source files rather than C/C++ sources, so that > we do not have to worry about compiler optimizations. attached is a self-contained example with Makefile and C code. It seems the reference via function pointers is the problem. [-- Attachment #2: ld-wrap.tar.gz --] [-- Type: application/x-compressed-tar, Size: 750 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-04 19:45 ` Sebastian Huber @ 2019-01-05 18:51 ` Sebastian Huber 2019-01-10 11:13 ` Nick Clifton 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-05 18:51 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils ----- Am 4. Jan 2019 um 20:45 schrieb Sebastian Huber sebastian.huber@embedded-brains.de: > Hello Nick, > > ----- Am 4. Jan 2019 um 15:27 schrieb Nick Clifton nickc@redhat.com: > >> Hi Sebastian, >> >>> I don't use link-time optimization. I am sure the functions ether_input() and >>> ether_output() are present in the executable. >> >> OK, but my guess is that the compiler has already done something clever >> so that those symbols are never referenced, and hence wrapping the symbols >> has no effect. (You could look in the object file and see if the symbols >> are used in one or more relocations...) >> >> Alternatively maybe there really is a bug in the linker's wrapping code. >> But I have no idea what it might be, or why this particular situation is >> causing problems. If you do think that it is a linker bug though, please >> could you put together a small test case to demonstrate the problem ? >> Ideally using assembler source files rather than C/C++ sources, so that >> we do not have to worry about compiler optimizations. > > attached is a self-contained example with Makefile and C code. It seems the > reference via function pointers is the problem. I slightly modified the test program. With a main() function like this: int main(void) { func gg; gg = f(); (*gg)(); g(); return 0; } I get this output: __wrap_f f g __wrap_g g The f() function returns a function pointer to g(). The indirect (*gg)() call is not wrapped, the direct g() call is wrapped. Is this a bug or a feature? ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-05 18:51 ` Sebastian Huber @ 2019-01-10 11:13 ` Nick Clifton 2019-01-10 12:14 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Nick Clifton @ 2019-01-10 11:13 UTC (permalink / raw) To: Sebastian Huber; +Cc: binutils Hi Sebastian, > The f() function returns a function pointer to g(). The indirect (*gg)() call is not wrapped, the direct g() call is wrapped. Yeah, I get this too. :-( > Is this a bug or a feature? Well I would have to say "bug", although I think that it is really a case of expecting too much of a feature that was really only intended to catch ordinary function calls. I have no idea how to fix it however, so maybe the best that we can is to update the documentation.... Cheers Nick ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 11:13 ` Nick Clifton @ 2019-01-10 12:14 ` Sebastian Huber 2019-01-10 13:09 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-10 12:14 UTC (permalink / raw) To: Nick Clifton; +Cc: binutils Hello Nick, On 10/01/2019 12:13, Nick Clifton wrote: >> Is this a bug or a feature? > Well I would have to say "bug", although I think that it is really a case > of expecting too much of a feature that was really only intended to catch > ordinary function calls. > > I have no idea how to fix it however, so maybe the best that we can is to > update the documentation.... from the documentation "Use a wrapper function for symbol. Any undefined reference to symbol will be resolved to __wrap_symbol. Any undefined reference to __real_symbol will be resolved to symbol. " it is absolutely not clear that function pointer references are anything special and are not covered by this mapping. I am not sure what is expected to work. There are not that many tests in the test suite. Maybe this: "Use a wrapper function for all direct calls to symbol. Any direct function call to symbol will be resolved to __wrap_symbol. Any direct function call to __real_symbol will be resolved to symbol. Indirect function calls will not be replaced for example." Would it be difficult to catch also indirect function calls? -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 12:14 ` Sebastian Huber @ 2019-01-10 13:09 ` Alan Modra 2019-01-10 13:12 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-10 13:09 UTC (permalink / raw) To: Sebastian Huber; +Cc: Nick Clifton, binutils [-- Attachment #1: Type: text/plain, Size: 424 bytes --] On Thu, Jan 10, 2019 at 01:14:22PM +0100, Sebastian Huber wrote: > from the documentation > > "Use a wrapper function for symbol. Any undefined reference to symbol will > be resolved to __wrap_symbol. Any undefined reference to __real_symbol will > be resolved to symbol. " Note "undefined reference". The references you want wrapped should not be defined in the same file. -- Alan Modra Australia Development Lab, IBM [-- Attachment #2: ld-wrap.tar.gz --] [-- Type: application/gzip, Size: 478 bytes --] ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 13:09 ` Alan Modra @ 2019-01-10 13:12 ` Sebastian Huber 2019-01-10 13:40 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-10 13:12 UTC (permalink / raw) To: Alan Modra; +Cc: Nick Clifton, binutils On 10/01/2019 14:08, Alan Modra wrote: > On Thu, Jan 10, 2019 at 01:14:22PM +0100, Sebastian Huber wrote: >> from the documentation >> >> "Use a wrapper function for symbol. Any undefined reference to symbol will >> be resolved to __wrap_symbol. Any undefined reference to __real_symbol will >> be resolved to symbol. " > Note "undefined reference". The references you want wrapped should > not be defined in the same file. In my example there is a file "f.c": #include "f.h" #include <stdio.h> func f(void) { puts(__PRETTY_FUNCTION__); return g; } There is also a "g.c": #include "f.h" #include <stdio.h> void g(void) { puts(__PRETTY_FUNCTION__); } -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 13:12 ` Sebastian Huber @ 2019-01-10 13:40 ` Alan Modra 2019-01-10 13:48 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-10 13:40 UTC (permalink / raw) To: Sebastian Huber; +Cc: Nick Clifton, binutils On Thu, Jan 10, 2019 at 02:12:53PM +0100, Sebastian Huber wrote: > On 10/01/2019 14:08, Alan Modra wrote: > > On Thu, Jan 10, 2019 at 01:14:22PM +0100, Sebastian Huber wrote: > > > from the documentation > > > > > > "Use a wrapper function for symbol. Any undefined reference to symbol will > > > be resolved to __wrap_symbol. Any undefined reference to __real_symbol will > > > be resolved to symbol. " > > Note "undefined reference". The references you want wrapped should > > not be defined in the same file. > > In my example there is a file "f.c": > There is also a "g.c": Not in the example you posted at https://sourceware.org/ml/binutils/2019-01/msg00022.html I modified your testcase to an example that works at https://sourceware.org/ml/binutils/2019-01/msg00070.html -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 13:40 ` Alan Modra @ 2019-01-10 13:48 ` Sebastian Huber 2019-01-10 14:05 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-10 13:48 UTC (permalink / raw) To: Alan Modra; +Cc: Nick Clifton, binutils On 10/01/2019 14:40, Alan Modra wrote: > On Thu, Jan 10, 2019 at 02:12:53PM +0100, Sebastian Huber wrote: >> On 10/01/2019 14:08, Alan Modra wrote: >>> On Thu, Jan 10, 2019 at 01:14:22PM +0100, Sebastian Huber wrote: >>>> from the documentation >>>> >>>> "Use a wrapper function for symbol. Any undefined reference to symbol will >>>> be resolved to __wrap_symbol. Any undefined reference to __real_symbol will >>>> be resolved to symbol. " >>> Note "undefined reference". The references you want wrapped should >>> not be defined in the same file. >> In my example there is a file "f.c": >> There is also a "g.c": > Not in the example you posted at > https://sourceware.org/ml/binutils/2019-01/msg00022.html > > I modified your testcase to an example that works at > https://sourceware.org/ml/binutils/2019-01/msg00070.html > Oh, sorry. I unpacked your attachment and thought it was just a copy of my original example. So this wrapping works only across object files and not for references inside an object file. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 13:48 ` Sebastian Huber @ 2019-01-10 14:05 ` Alan Modra 2019-01-10 19:04 ` Joel Sherrill 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-10 14:05 UTC (permalink / raw) To: Sebastian Huber; +Cc: Nick Clifton, binutils On Thu, Jan 10, 2019 at 02:48:16PM +0100, Sebastian Huber wrote: > So this wrapping works only across object files and not > for references inside an object file. Yes. If a reference is to a function defined in the same file you don't have an undefined symbol for the given function in that file. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 14:05 ` Alan Modra @ 2019-01-10 19:04 ` Joel Sherrill 2019-01-14 9:41 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Joel Sherrill @ 2019-01-10 19:04 UTC (permalink / raw) To: Alan Modra; +Cc: Sebastian Huber, Nick Clifton, binutils On Thu, Jan 10, 2019, 8:05 AM Alan Modra <amodra@gmail.com wrote: > On Thu, Jan 10, 2019 at 02:48:16PM +0100, Sebastian Huber wrote: > > So this wrapping works only across object files and not > > for references inside an object file. > > Yes. If a reference is to a function defined in the same file you > don't have an undefined symbol for the given function in that file. > And per function section compilation and linking have no impact on that? Shouldn't there be more reference tracking with that enabled? Sebastian.. I thought we used per function compilation everywhere. Even in the network stack. --joel > > -- > Alan Modra > Australia Development Lab, IBM > ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-10 19:04 ` Joel Sherrill @ 2019-01-14 9:41 ` Sebastian Huber 2019-01-18 0:24 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-14 9:41 UTC (permalink / raw) To: joel; +Cc: binutils On 10/01/2019 20:04, Joel Sherrill wrote: > > > On Thu, Jan 10, 2019, 8:05 AM Alan Modra <amodra@gmail.com > <mailto:amodra@gmail.com> wrote: > > On Thu, Jan 10, 2019 at 02:48:16PM +0100, Sebastian Huber wrote: > > So this wrapping works only across object files and not > > for references inside an object file. > > Yes. If a reference is to a function defined in the same file you > don't have an undefined symbol for the given function in that file. > > > And per function section compilation and linking have no impact on > that? Shouldn't there be more reference tracking with that enabled? > > Sebastian.. I thought we used per function compilation everywhere. > Even in the network stack. Yes, I used -ffunction-sections. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-14 9:41 ` Sebastian Huber @ 2019-01-18 0:24 ` Alan Modra 2019-01-18 9:04 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-18 0:24 UTC (permalink / raw) To: Sebastian Huber; +Cc: joel, binutils On Mon, Jan 14, 2019 at 10:40:56AM +0100, Sebastian Huber wrote: > On 10/01/2019 20:04, Joel Sherrill wrote: > > > > > > On Thu, Jan 10, 2019, 8:05 AM Alan Modra <amodra@gmail.com > > <mailto:amodra@gmail.com> wrote: > > > > On Thu, Jan 10, 2019 at 02:48:16PM +0100, Sebastian Huber wrote: > > > So this wrapping works only across object files and not > > > for references inside an object file. > > > > Yes. If a reference is to a function defined in the same file you > > don't have an undefined symbol for the given function in that file. > > > > > > And per function section compilation and linking have no impact on that? No, -ffunction-sections will make no difference. Really, --wrap was intended for wrapping system functions, which I guess is why the feature was implemented only for undefined symbols. I don't see a fundamental reason why --wrap couldn't be made to work with defined symbols, provided the compiler and assembler don't optimise references to local functions. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-18 0:24 ` Alan Modra @ 2019-01-18 9:04 ` Sebastian Huber 2019-01-18 13:41 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-18 9:04 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils On 18/01/2019 01:24, Alan Modra wrote: > On Mon, Jan 14, 2019 at 10:40:56AM +0100, Sebastian Huber wrote: >> On 10/01/2019 20:04, Joel Sherrill wrote: >>> >>> On Thu, Jan 10, 2019, 8:05 AM Alan Modra <amodra@gmail.com >>> <mailto:amodra@gmail.com> wrote: >>> >>> On Thu, Jan 10, 2019 at 02:48:16PM +0100, Sebastian Huber wrote: >>> > So this wrapping works only across object files and not >>> > for references inside an object file. >>> >>> Yes. If a reference is to a function defined in the same file you >>> don't have an undefined symbol for the given function in that file. >>> >>> >>> And per function section compilation and linking have no impact on that? > No, -ffunction-sections will make no difference. Really, --wrap was > intended for wrapping system functions, which I guess is why the > feature was implemented only for undefined symbols. I don't see a > fundamental reason why --wrap couldn't be made to work with defined > symbols, provided the compiler and assembler don't optimise references > to local functions. In case it is acceptable to extend --wrap to work also for defined symbol references, then I would have a look at this and try to implement it. If you already have an idea which functions needs to be touched for this new feature, then this would be helpful for me. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-18 9:04 ` Sebastian Huber @ 2019-01-18 13:41 ` Alan Modra 2019-01-23 9:11 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-18 13:41 UTC (permalink / raw) To: Sebastian Huber; +Cc: joel, binutils On Fri, Jan 18, 2019 at 10:04:36AM +0100, Sebastian Huber wrote: > On 18/01/2019 01:24, Alan Modra wrote: > > No, -ffunction-sections will make no difference. Really, --wrap was > > intended for wrapping system functions, which I guess is why the > > feature was implemented only for undefined symbols. I don't see a > > fundamental reason why --wrap couldn't be made to work with defined > > symbols, provided the compiler and assembler don't optimise references > > to local functions. > > In case it is acceptable to extend --wrap to work also for defined symbol > references, then I would have a look at this and try to implement it. If you > already have an idea which functions needs to be touched for this new > feature, then this would be helpful for me. It might just be a matter of calling bfd_wrapped_link_hash_lookup in more places where we currently call bfd_link_hash_lookup and its derivatives like elf_link_hash_lookup. Knowing which places to change is the difficult part. Anything involved with relocation processing for a start. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-18 13:41 ` Alan Modra @ 2019-01-23 9:11 ` Sebastian Huber 2019-01-25 8:37 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-23 9:11 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils On 18/01/2019 14:41, Alan Modra wrote: > On Fri, Jan 18, 2019 at 10:04:36AM +0100, Sebastian Huber wrote: >> On 18/01/2019 01:24, Alan Modra wrote: >>> No, -ffunction-sections will make no difference. Really, --wrap was >>> intended for wrapping system functions, which I guess is why the >>> feature was implemented only for undefined symbols. I don't see a >>> fundamental reason why --wrap couldn't be made to work with defined >>> symbols, provided the compiler and assembler don't optimise references >>> to local functions. >> In case it is acceptable to extend --wrap to work also for defined symbol >> references, then I would have a look at this and try to implement it. If you >> already have an idea which functions needs to be touched for this new >> feature, then this would be helpful for me. > It might just be a matter of calling bfd_wrapped_link_hash_lookup in > more places where we currently call bfd_link_hash_lookup and its > derivatives like elf_link_hash_lookup. Knowing which places to change > is the difficult part. Anything involved with relocation processing > for a start. > I think to wrap defined references you have to add things to other places than spots which call bfd_link_hash_lookup() or bfd_wrapped_link_hash_lookup(). I built ld with -O0 -g and used the following test program: void f(void) { } void g(void); void _start(void) { f(); g(); } I use this GDB script: b bfd_wrapped_link_hash_lookup if string[0] != '.' && (string[0] == 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g') commands c end b bfd_link_hash_lookup if string[0] != '.' && (string[0] == 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g') commands c end r This yields: gdb --command=main.gdb --args ld-new -wrap=f -wrap=g main.o -o main.exe Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd0 "f", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 511 if (table == NULL || string == NULL) Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd2 "_start", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 511 if (table == NULL || string == NULL) Breakpoint 1, bfd_wrapped_link_hash_lookup (abfd=0x8eec40, info=0x8c2de0 <link_info>, string=0x8f2bd9 "g", create=1, copy=0, follow=0) at ../../bfd/linker.c:541 541 if (info->wrap_hash != NULL) Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f35c0 "__wrap_g", create=1, copy=1, follow=0) at ../../bfd/linker.c:511 511 if (table == NULL || string == NULL) /home/EB/sebastian_h/archive/binutils-git/build/ld/ld-new: main.o: in function `_start': main.c:(.text+0x11): undefined reference to `__wrap_g' The lookup is performed only once for "f". I think this is when ld finds the definition of f(). For the call in _start() we end up in other areas of ld. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-23 9:11 ` Sebastian Huber @ 2019-01-25 8:37 ` Sebastian Huber 2019-01-25 13:55 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-25 8:37 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils On 23/01/2019 10:11, Sebastian Huber wrote: > On 18/01/2019 14:41, Alan Modra wrote: >> On Fri, Jan 18, 2019 at 10:04:36AM +0100, Sebastian Huber wrote: >>> On 18/01/2019 01:24, Alan Modra wrote: >>>> No, -ffunction-sections will make no difference. Really, --wrap was >>>> intended for wrapping system functions, which I guess is why the >>>> feature was implemented only for undefined symbols. I don't see a >>>> fundamental reason why --wrap couldn't be made to work with defined >>>> symbols, provided the compiler and assembler don't optimise references >>>> to local functions. >>> In case it is acceptable to extend --wrap to work also for defined >>> symbol >>> references, then I would have a look at this and try to implement >>> it. If you >>> already have an idea which functions needs to be touched for this new >>> feature, then this would be helpful for me. >> It might just be a matter of calling bfd_wrapped_link_hash_lookup in >> more places where we currently call bfd_link_hash_lookup and its >> derivatives like elf_link_hash_lookup. Knowing which places to change >> is the difficult part. Anything involved with relocation processing >> for a start. >> > > I think to wrap defined references you have to add things to other > places than spots which call bfd_link_hash_lookup() or > bfd_wrapped_link_hash_lookup(). I built ld with -O0 -g and used the > following test program: > > void f(void) > { > } > > void g(void); > > void _start(void) > { > f(); > g(); > } > > I use this GDB script: > > b bfd_wrapped_link_hash_lookup if string[0] != '.' && (string[0] == > 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g') > commands > c > end > b bfd_link_hash_lookup if string[0] != '.' && (string[0] == 'f' || > string[7] == 'f' || string[0] == 'g' || string[7] == 'g') > commands > c > end > r > > This yields: > > gdb --command=main.gdb --args ld-new -wrap=f -wrap=g main.o -o main.exe > Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd0 > "f", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 > 511 if (table == NULL || string == NULL) > > Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd2 > "_start", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 > 511 if (table == NULL || string == NULL) > > Breakpoint 1, bfd_wrapped_link_hash_lookup (abfd=0x8eec40, > info=0x8c2de0 <link_info>, string=0x8f2bd9 "g", create=1, copy=0, > follow=0) at ../../bfd/linker.c:541 > 541 if (info->wrap_hash != NULL) > > Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f35c0 > "__wrap_g", create=1, copy=1, follow=0) at ../../bfd/linker.c:511 > 511 if (table == NULL || string == NULL) > /home/EB/sebastian_h/archive/binutils-git/build/ld/ld-new: main.o: in > function `_start': > main.c:(.text+0x11): undefined reference to `__wrap_g' > > The lookup is performed only once for "f". I think this is when ld > finds the definition of f(). For the call in _start() we end up in > other areas of ld. > Wrapping of undefined symbol references are dealt with during load_symbols() which is performed early during the link process. For defined symbols references we have to look at the relocations. During the final link performed by bfd_elf_final_link() which calls elf_link_input_bfd() which calls the architecture-specific elf_x86_64_relocate_section() we end up in (elf-bfd.h): /* This macro is to avoid lots of duplicated code in the body of xxx_relocate_section() in the various elfxx-xxxx.c files. */ #define RELOC_FOR_GLOBAL_SYMBOL(info, input_bfd, input_section, rel, \ r_symndx, symtab_hdr, sym_hashes, \ h, sec, relocation, \ unresolved_reloc, warned, ignored) \ do \ { \ /* It seems this can happen with erroneous or unsupported \ input (mixing a.out and elf in an archive, for example.) */ \ if (sym_hashes == NULL) \ return FALSE; \ \ h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ \ if (info->wrap_hash != NULL \ && (input_section->flags & SEC_DEBUGGING) != 0) \ h = ((struct elf_link_hash_entry *) \ unwrap_hash_lookup (info, input_bfd, &h->root)); \ Why is there a special case for input_section->flags & SEC_DEBUGGING) != 0 here? If I extend this macro in case info->wrap_hash != NULL to do the symbol wrapping, then there is an issue with the __real_SYMBOL references which are no longer visible here. We only see SYMBOL and __wrap_SYMBOL. The SYMBOL could be a defined reference or a former __real_SYMBOL undefined reference. Would it be feasible to the the wrapping at the relocation step and not during load_symbols()? -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-25 8:37 ` Sebastian Huber @ 2019-01-25 13:55 ` Sebastian Huber 2019-01-25 23:52 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-25 13:55 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils [-- Attachment #1: Type: text/plain, Size: 6089 bytes --] On 25/01/2019 09:37, Sebastian Huber wrote: > > > On 23/01/2019 10:11, Sebastian Huber wrote: >> On 18/01/2019 14:41, Alan Modra wrote: >>> On Fri, Jan 18, 2019 at 10:04:36AM +0100, Sebastian Huber wrote: >>>> On 18/01/2019 01:24, Alan Modra wrote: >>>>> No, -ffunction-sections will make no difference. Really, --wrap was >>>>> intended for wrapping system functions, which I guess is why the >>>>> feature was implemented only for undefined symbols. I don't see a >>>>> fundamental reason why --wrap couldn't be made to work with defined >>>>> symbols, provided the compiler and assembler don't optimise >>>>> references >>>>> to local functions. >>>> In case it is acceptable to extend --wrap to work also for defined >>>> symbol >>>> references, then I would have a look at this and try to implement >>>> it. If you >>>> already have an idea which functions needs to be touched for this new >>>> feature, then this would be helpful for me. >>> It might just be a matter of calling bfd_wrapped_link_hash_lookup in >>> more places where we currently call bfd_link_hash_lookup and its >>> derivatives like elf_link_hash_lookup. Knowing which places to change >>> is the difficult part. Anything involved with relocation processing >>> for a start. >>> >> >> I think to wrap defined references you have to add things to other >> places than spots which call bfd_link_hash_lookup() or >> bfd_wrapped_link_hash_lookup(). I built ld with -O0 -g and used the >> following test program: >> >> void f(void) >> { >> } >> >> void g(void); >> >> void _start(void) >> { >> f(); >> g(); >> } >> >> I use this GDB script: >> >> b bfd_wrapped_link_hash_lookup if string[0] != '.' && (string[0] == >> 'f' || string[7] == 'f' || string[0] == 'g' || string[7] == 'g') >> commands >> c >> end >> b bfd_link_hash_lookup if string[0] != '.' && (string[0] == 'f' || >> string[7] == 'f' || string[0] == 'g' || string[7] == 'g') >> commands >> c >> end >> r >> >> This yields: >> >> gdb --command=main.gdb --args ld-new -wrap=f -wrap=g main.o -o main.exe >> Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd0 >> "f", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 >> 511 if (table == NULL || string == NULL) >> >> Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f2bd2 >> "_start", create=1, copy=0, follow=0) at ../../bfd/linker.c:511 >> 511 if (table == NULL || string == NULL) >> >> Breakpoint 1, bfd_wrapped_link_hash_lookup (abfd=0x8eec40, >> info=0x8c2de0 <link_info>, string=0x8f2bd9 "g", create=1, copy=0, >> follow=0) at ../../bfd/linker.c:541 >> 541 if (info->wrap_hash != NULL) >> >> Breakpoint 2, bfd_link_hash_lookup (table=0x8df4f0, string=0x8f35c0 >> "__wrap_g", create=1, copy=1, follow=0) at ../../bfd/linker.c:511 >> 511 if (table == NULL || string == NULL) >> /home/EB/sebastian_h/archive/binutils-git/build/ld/ld-new: main.o: in >> function `_start': >> main.c:(.text+0x11): undefined reference to `__wrap_g' >> >> The lookup is performed only once for "f". I think this is when ld >> finds the definition of f(). For the call in _start() we end up in >> other areas of ld. >> > > Wrapping of undefined symbol references are dealt with during > load_symbols() > which is performed early during the link process. > > For defined symbols references we have to look at the relocations. > During the > final link performed by bfd_elf_final_link() which calls > elf_link_input_bfd() > which calls the architecture-specific elf_x86_64_relocate_section() we > end up > in (elf-bfd.h): > > /* This macro is to avoid lots of duplicated code in the body > of xxx_relocate_section() in the various elfxx-xxxx.c files. */ > #define RELOC_FOR_GLOBAL_SYMBOL(info, input_bfd, input_section, rel, \ > r_symndx, symtab_hdr, sym_hashes, \ > h, sec, relocation, \ > unresolved_reloc, warned, ignored) \ > do \ > { \ > /* It seems this can happen with erroneous or unsupported \ > input (mixing a.out and elf in an archive, for example.) */ \ > if (sym_hashes == NULL) \ > return FALSE; \ > \ > h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ > \ > if (info->wrap_hash != NULL \ > && (input_section->flags & SEC_DEBUGGING) != 0) \ > h = ((struct elf_link_hash_entry *) \ > unwrap_hash_lookup (info, input_bfd, &h->root)); \ > > Why is there a special case for input_section->flags & SEC_DEBUGGING) > != 0 here? > > If I extend this macro in case info->wrap_hash != NULL to do the > symbol wrapping, then there is an issue with the __real_SYMBOL > references which are no longer visible here. We only see SYMBOL and > __wrap_SYMBOL. The SYMBOL could be a defined reference or a former > __real_SYMBOL undefined reference. > > Would it be feasible to the the wrapping at the relocation step and > not during load_symbols()? > With this quick and dirty hack I can wrap at relocation level. There is a NULL pointer access in case of unresolved references. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. [-- Attachment #2: 0001-HACK-Do-LD-wrap-at-relocation-level.patch --] [-- Type: text/x-patch, Size: 5832 bytes --] From 35c495430dd37955ae345b64a3da2c15c3fa7e97 Mon Sep 17 00:00:00 2001 From: Sebastian Huber <sebastian.huber@embedded-brains.de> Date: Fri, 25 Jan 2019 14:53:04 +0100 Subject: [PATCH] HACK: Do LD --wrap at relocation level There is a NULL pointer access in case of unresolved references in RELOC_FOR_GLOBAL_SYMBOL(). --- bfd/elf-bfd.h | 2 ++ bfd/linker.c | 104 ++++++++++++++++++++++++++++-------------------------- include/bfdlink.h | 3 ++ 3 files changed, 59 insertions(+), 50 deletions(-) diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index 5741c60264..5a1f29b819 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section; return FALSE; \ \ h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ + h = ((struct elf_link_hash_entry *) \ + wrap_hash_lookup (info, input_bfd, &h->root)); \ \ if (info->wrap_hash != NULL \ && (input_section->flags & SEC_DEBUGGING) != 0) \ diff --git a/bfd/linker.c b/bfd/linker.c index cf7d673f59..9194fb3a0c 100644 --- a/bfd/linker.c +++ b/bfd/linker.c @@ -536,28 +536,69 @@ bfd_wrapped_link_hash_lookup (bfd *abfd, bfd_boolean copy, bfd_boolean follow) { - bfd_size_type amt; + (void) abfd; + return bfd_link_hash_lookup (info->hash, string, create, copy, follow); +} + +/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_" + and the remainder is found in wrap_hash, return the real symbol. */ +struct bfd_link_hash_entry * +unwrap_hash_lookup (struct bfd_link_info *info, + bfd *input_bfd, + struct bfd_link_hash_entry *h) +{ + const char *l = h->root.string; + + if (*l == bfd_get_symbol_leading_char (input_bfd) + || *l == info->wrap_char) + ++l; + +#undef WRAP +#define WRAP "__wrap_" + + if (CONST_STRNEQ (l, WRAP)) + { + l += sizeof WRAP - 1; + + if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) + { + char save = 0; + if (l - (sizeof WRAP - 1) != h->root.string) + { + --l; + save = *l; + *(char *) l = *h->root.string; + } + h = bfd_link_hash_lookup (info->hash, l, FALSE, FALSE, FALSE); + if (save) + *(char *) l = save; + } + } + return h; +} + +struct bfd_link_hash_entry * +wrap_hash_lookup (struct bfd_link_info *info, + bfd *input_bfd, + struct bfd_link_hash_entry *h) +{ if (info->wrap_hash != NULL) { const char *l; char prefix = '\0'; + bfd_size_type amt; + char *n; - l = string; - if (*l == bfd_get_symbol_leading_char (abfd) || *l == info->wrap_char) + l = h->root.string; + if (*l == bfd_get_symbol_leading_char (input_bfd) || *l == info->wrap_char) { prefix = *l; ++l; } -#undef WRAP -#define WRAP "__wrap_" - if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) { - char *n; - struct bfd_link_hash_entry *h; - /* This symbol is being wrapped. We want to replace all references to SYM with references to __wrap_SYM. */ @@ -570,11 +611,13 @@ bfd_wrapped_link_hash_lookup (bfd *abfd, n[1] = '\0'; strcat (n, WRAP); strcat (n, l); - h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow); + h = bfd_link_hash_lookup (info->hash, n, FALSE, FALSE, FALSE); free (n); return h; } +#undef WRAP + #undef REAL #define REAL "__real_" @@ -583,9 +626,6 @@ bfd_wrapped_link_hash_lookup (bfd *abfd, && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1, FALSE, FALSE) != NULL) { - char *n; - struct bfd_link_hash_entry *h; - /* This is a reference to __real_SYM, where SYM is being wrapped. We want to replace all references to __real_SYM with references to SYM. */ @@ -598,7 +638,7 @@ bfd_wrapped_link_hash_lookup (bfd *abfd, n[0] = prefix; n[1] = '\0'; strcat (n, l + sizeof REAL - 1); - h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow); + h = bfd_link_hash_lookup (info->hash, n, FALSE, FALSE, FALSE); free (n); return h; } @@ -606,44 +646,8 @@ bfd_wrapped_link_hash_lookup (bfd *abfd, #undef REAL } - return bfd_link_hash_lookup (info->hash, string, create, copy, follow); -} - -/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_" - and the remainder is found in wrap_hash, return the real symbol. */ - -struct bfd_link_hash_entry * -unwrap_hash_lookup (struct bfd_link_info *info, - bfd *input_bfd, - struct bfd_link_hash_entry *h) -{ - const char *l = h->root.string; - - if (*l == bfd_get_symbol_leading_char (input_bfd) - || *l == info->wrap_char) - ++l; - - if (CONST_STRNEQ (l, WRAP)) - { - l += sizeof WRAP - 1; - - if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) - { - char save = 0; - if (l - (sizeof WRAP - 1) != h->root.string) - { - --l; - save = *l; - *(char *) l = *h->root.string; - } - h = bfd_link_hash_lookup (info->hash, l, FALSE, FALSE, FALSE); - if (save) - *(char *) l = save; - } - } return h; } -#undef WRAP /* Traverse a generic link hash table. Differs from bfd_hash_traverse in the treatment of warning symbols. When warning symbols are diff --git a/include/bfdlink.h b/include/bfdlink.h index bad52f9c50..478786c4fe 100644 --- a/include/bfdlink.h +++ b/include/bfdlink.h @@ -227,6 +227,9 @@ extern struct bfd_link_hash_entry *bfd_wrapped_link_hash_lookup extern struct bfd_link_hash_entry *unwrap_hash_lookup (struct bfd_link_info *, bfd *, struct bfd_link_hash_entry *); +extern struct bfd_link_hash_entry *wrap_hash_lookup + (struct bfd_link_info *, bfd *, struct bfd_link_hash_entry *); + /* Traverse a link hash table. */ extern void bfd_link_hash_traverse (struct bfd_link_hash_table *, -- 2.16.4 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-25 13:55 ` Sebastian Huber @ 2019-01-25 23:52 ` Alan Modra 2019-01-28 9:28 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-25 23:52 UTC (permalink / raw) To: Sebastian Huber; +Cc: joel, binutils On Fri, Jan 25, 2019 at 02:55:35PM +0100, Sebastian Huber wrote: > @@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section; > return FALSE; \ > \ > h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ Is it possible to arrange for the symbol hashes to be set up to point to the wrapped symbols? I'm thinking a change to always use bfd_wrapped_link_hash_lookup in _bfd_generic_link_add_one_symbol might work. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-25 23:52 ` Alan Modra @ 2019-01-28 9:28 ` Sebastian Huber 2019-01-29 0:31 ` Alan Modra 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-28 9:28 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils [-- Attachment #1: Type: text/plain, Size: 4914 bytes --] On 26/01/2019 00:52, Alan Modra wrote: > On Fri, Jan 25, 2019 at 02:55:35PM +0100, Sebastian Huber wrote: >> @@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section; >> return FALSE; \ >> \ >> h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ > Is it possible to arrange for the symbol hashes to be set up to point > to the wrapped symbols? I'm thinking a change to always use > bfd_wrapped_link_hash_lookup in _bfd_generic_link_add_one_symbol might > work. Is this hash table only used for the relocations? We need mappings for __real_S -> S __wrap_S -> __wrap_S S -> __wrap_S I don't know why there is a special case for debug sections in RELOC_FOR_GLOBAL_SYMBOL(): if (info->wrap_hash != NULL && (input_section->flags & SEC_DEBUGGING) != 0) eh = ((struct elf_link_hash_entry *) unwrap_hash_lookup (info, input_bfd, &eh->root)); Replacing bfd_link_hash_lookup() with bfd_wrapped_link_hash_lookup() in _bfd_generic_link_add_one_symbol() is not enough. The wrapping needs to be added at least to this area in the function as well: do { enum link_action action; int prev; prev = h->type; /* Treat symbols defined by early linker script pass as undefined. */ if (h->ldscript_def) prev = bfd_link_hash_undefined; cycle = FALSE; action = link_action[(int) row][prev]; switch (action) { case FAIL: abort (); 1473 switch (action) (gdb) p action $1 = DEF (gdb) bt #0 _bfd_generic_link_add_one_symbol (info=0x8c2de0 <link_info>, abfd=0x8eec40, name=0x8f2c00 "f", flags=2, section=0x8efe20, value=0, string=0x0, copy=0, collect=0, hashp=0x8f2bb0) at ../../bfd/linker.c:1473 #1 0x00000000004a3172 in elf_link_add_object_symbols (abfd=0x8eec40, info=0x8c2de0 <link_info>) at ../../bfd/elflink.c:4701 #2 0x00000000004a57dd in bfd_elf_link_add_symbols (abfd=0x8eec40, info=0x8c2de0 <link_info>) at ../../bfd/elflink.c:5740 #3 0x0000000000411cb5 in load_symbols (entry=0x8c6390, place=0x7fffffffd5e0) at ../../ld/ldlang.c:3080 #4 0x000000000041298c in open_input_bfds (s=0x8c6390, mode=OPEN_BFD_NORMAL) at ../../ld/ldlang.c:3529 #5 0x00000000004197c5 in lang_process () at ../../ld/ldlang.c:7383 #6 0x000000000041def9 in main (argc=6, argv=0x7fffffffd838) at ../../ld/ldmain.c:440 I change it like this to see what happens: diff --git a/bfd/linker.c b/bfd/linker.c index cf7d673f59..508ccac4c1 100644 --- a/bfd/linker.c +++ b/bfd/linker.c @@ -1431,20 +1431,12 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, else row = DEF_ROW; - if (hashp != NULL && *hashp != NULL) - h = *hashp; - else + h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE); + if (h == NULL) { - if (row == UNDEF_ROW || row == UNDEFW_ROW) - h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE); - else - h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE); - if (h == NULL) - { - if (hashp != NULL) - *hashp = NULL; - return FALSE; - } + if (hashp != NULL) + *hashp = NULL; + return FALSE; } if (info->notice_all This leads to multiple definition errors and unresolved references: build/ld/ld-new: main2.o: in function `__wrap_f': main2.c:(.text+0x11): multiple definition of `__wrap_f'; main2.o:main2.c:(.text+0x0): first defined here build/ld/ld-new: g.o: in function `g': g.c:(.text+0x0): multiple definition of `__wrap_g'; main2.o:main2.c:(.text+0x2c): first defined here build/ld/ld-new: main2.o: in function `__wrap_f': main2.c:(.text+0x25): undefined reference to `f' build/ld/ld-new: main2.o: in function `__wrap_g': main2.c:(.text+0x40): undefined reference to `g' I am not sure if the wrapping can be done using only the symbol hash table. We have the definition of the symbol and the references to it. Attached is a second version which does the wrapping in RELOC_FOR_GLOBAL_SYMBOL(). It removes bfd_wrapped_link_hash_lookup(). I am not sure if this is a good idea. This function is used in various places. Maybe the wrapping needs to be done on other spots and not only in RELOC_FOR_GLOBAL_SYMBOL(). -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. [-- Attachment #2: v2-0001-HACK-Do-LD-wrap-at-relocation-level.patch --] [-- Type: text/x-patch, Size: 13816 bytes --] From 0b0c3ab7d52bfcb3f8dc5f54194f9e20550a61c8 Mon Sep 17 00:00:00 2001 From: Sebastian Huber <sebastian.huber@embedded-brains.de> Date: Fri, 25 Jan 2019 14:53:04 +0100 Subject: [PATCH v2] HACK: Do LD --wrap at relocation level This patch removes bfd_wrapped_link_hash_lookup(). --- bfd/cofflink.c | 6 +- bfd/elf-bfd.h | 12 ++-- bfd/elflink.c | 11 +--- bfd/linker.c | 190 +++++++++++++++++++++--------------------------------- include/bfdlink.h | 8 +-- ld/ldexp.c | 18 ++---- ld/plugin.c | 6 +- 7 files changed, 98 insertions(+), 153 deletions(-) diff --git a/bfd/cofflink.c b/bfd/cofflink.c index e4031b9a31..3124d2cd7f 100644 --- a/bfd/cofflink.c +++ b/bfd/cofflink.c @@ -2884,9 +2884,9 @@ _bfd_coff_reloc_link_order (bfd *output_bfd, struct coff_link_hash_entry *h; h = ((struct coff_link_hash_entry *) - bfd_wrapped_link_hash_lookup (output_bfd, flaginfo->info, - link_order->u.reloc.p->u.name, - FALSE, FALSE, TRUE)); + bfd_link_hash_lookup (flaginfo->info->hash, + link_order->u.reloc.p->u.name, + FALSE, FALSE, TRUE)); if (h != NULL) { if (h->indx >= 0) diff --git a/bfd/elf-bfd.h b/bfd/elf-bfd.h index 5741c60264..b4985b442a 100644 --- a/bfd/elf-bfd.h +++ b/bfd/elf-bfd.h @@ -2781,10 +2781,14 @@ extern asection _bfd_elf_large_com_section; \ h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ \ - if (info->wrap_hash != NULL \ - && (input_section->flags & SEC_DEBUGGING) != 0) \ - h = ((struct elf_link_hash_entry *) \ - unwrap_hash_lookup (info, input_bfd, &h->root)); \ + if (info->wrap_hash != NULL) \ + { \ + h = (struct elf_link_hash_entry *) \ + wrap_hash_lookup_for_reloc (info, input_bfd, \ + input_section, &h->root); \ + if (h == NULL) \ + return FALSE; \ + } \ \ while (h->root.type == bfd_link_hash_indirect \ || h->root.type == bfd_link_hash_warning) \ diff --git a/bfd/elflink.c b/bfd/elflink.c index e50c0e4b38..cb722cf643 100644 --- a/bfd/elflink.c +++ b/bfd/elflink.c @@ -1069,11 +1069,7 @@ _bfd_elf_merge_symbol (bfd *abfd, sec = *psec; bind = ELF_ST_BIND (sym->st_info); - if (! bfd_is_und_section (sec)) - h = elf_link_hash_lookup (elf_hash_table (info), name, TRUE, FALSE, FALSE); - else - h = ((struct elf_link_hash_entry *) - bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, FALSE, FALSE)); + h = elf_link_hash_lookup (elf_hash_table (info), name, TRUE, FALSE, FALSE); if (h == NULL) return FALSE; *sym_hash = h; @@ -11259,9 +11255,8 @@ elf_reloc_link_order (bfd *output_bfd, /* Treat a reloc against a defined symbol as though it were actually against the section. */ h = ((struct elf_link_hash_entry *) - bfd_wrapped_link_hash_lookup (output_bfd, info, - link_order->u.reloc.p->u.name, - FALSE, FALSE, TRUE)); + bfd_link_hash_lookup (info->hash, link_order->u.reloc.p->u.name, + FALSE, FALSE, TRUE)); if (h != NULL && (h->root.type == bfd_link_hash_defined || h->root.type == bfd_link_hash_defweak)) diff --git a/bfd/linker.c b/bfd/linker.c index cf7d673f59..c72719397e 100644 --- a/bfd/linker.c +++ b/bfd/linker.c @@ -524,126 +524,98 @@ bfd_link_hash_lookup (struct bfd_link_hash_table *table, return ret; } -/* Look up a symbol in the main linker hash table if the symbol might - be wrapped. This should only be used for references to an - undefined symbol, not for definitions of a symbol. */ - struct bfd_link_hash_entry * -bfd_wrapped_link_hash_lookup (bfd *abfd, - struct bfd_link_info *info, - const char *string, - bfd_boolean create, - bfd_boolean copy, - bfd_boolean follow) +wrap_hash_lookup_for_reloc (struct bfd_link_info *info, + bfd *input_bfd, + struct bfd_section *input_section, + struct bfd_link_hash_entry *h) { + const char *l; + char prefix = '\0'; bfd_size_type amt; - - if (info->wrap_hash != NULL) - { - const char *l; - char prefix = '\0'; - - l = string; - if (*l == bfd_get_symbol_leading_char (abfd) || *l == info->wrap_char) - { - prefix = *l; - ++l; - } + char *n; #undef WRAP #define WRAP "__wrap_" - if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) - { - char *n; - struct bfd_link_hash_entry *h; - - /* This symbol is being wrapped. We want to replace all - references to SYM with references to __wrap_SYM. */ + BFD_ASSERT (info->wrap_hash != NULL); - amt = strlen (l) + sizeof WRAP + 1; - n = (char *) bfd_malloc (amt); - if (n == NULL) - return NULL; - - n[0] = prefix; - n[1] = '\0'; - strcat (n, WRAP); - strcat (n, l); - h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow); - free (n); - return h; - } + l = h->root.string; + if (*l == bfd_get_symbol_leading_char (input_bfd) || *l == info->wrap_char) + { + prefix = *l; + ++l; + } -#undef REAL -#define REAL "__real_" + if ((input_section->flags & SEC_DEBUGGING) != 0 && CONST_STRNEQ (l, WRAP)) + { + const char *s = l + sizeof WRAP - 1; - if (*l == '_' - && CONST_STRNEQ (l, REAL) - && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1, - FALSE, FALSE) != NULL) + if (bfd_hash_lookup (info->wrap_hash, s, FALSE, FALSE) != NULL) { - char *n; - struct bfd_link_hash_entry *h; - - /* This is a reference to __real_SYM, where SYM is being - wrapped. We want to replace all references to __real_SYM - with references to SYM. */ - - amt = strlen (l + sizeof REAL - 1) + 2; - n = (char *) bfd_malloc (amt); - if (n == NULL) - return NULL; - - n[0] = prefix; - n[1] = '\0'; - strcat (n, l + sizeof REAL - 1); - h = bfd_link_hash_lookup (info->hash, n, create, TRUE, follow); - free (n); + char save = 0; + if (s - (sizeof WRAP - 1) != h->root.string) + { + --s; + save = *s; + *(char *) s = *h->root.string; + } + h = bfd_link_hash_lookup (info->hash, s, FALSE, FALSE, FALSE); + if (save) + *(char *) s = save; return h; } - -#undef REAL } - return bfd_link_hash_lookup (info->hash, string, create, copy, follow); -} - -/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_" - and the remainder is found in wrap_hash, return the real symbol. */ + if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) + { + /* This symbol is being wrapped. We want to replace all + references to SYM with references to __wrap_SYM. */ + + amt = strlen (l) + sizeof WRAP + 1; + n = (char *) bfd_malloc (amt); + if (n == NULL) + return NULL; + + n[0] = prefix; + n[1] = '\0'; + strcat (n, WRAP); + strcat (n, l); + h = bfd_link_hash_lookup (info->hash, n, TRUE, TRUE, FALSE); + free (n); + return h; + } -struct bfd_link_hash_entry * -unwrap_hash_lookup (struct bfd_link_info *info, - bfd *input_bfd, - struct bfd_link_hash_entry *h) -{ - const char *l = h->root.string; +#undef WRAP - if (*l == bfd_get_symbol_leading_char (input_bfd) - || *l == info->wrap_char) - ++l; +#undef REAL +#define REAL "__real_" - if (CONST_STRNEQ (l, WRAP)) + if (*l == '_' + && CONST_STRNEQ (l, REAL) + && bfd_hash_lookup (info->wrap_hash, l + sizeof REAL - 1, + FALSE, FALSE) != NULL) { - l += sizeof WRAP - 1; - - if (bfd_hash_lookup (info->wrap_hash, l, FALSE, FALSE) != NULL) - { - char save = 0; - if (l - (sizeof WRAP - 1) != h->root.string) - { - --l; - save = *l; - *(char *) l = *h->root.string; - } - h = bfd_link_hash_lookup (info->hash, l, FALSE, FALSE, FALSE); - if (save) - *(char *) l = save; - } + /* This is a reference to __real_SYM, where SYM is being + wrapped. We want to replace all references to __real_SYM + with references to SYM. */ + + amt = strlen (l + sizeof REAL - 1) + 2; + n = (char *) bfd_malloc (amt); + if (n == NULL) + return NULL; + + n[0] = prefix; + n[1] = '\0'; + strcat (n, l + sizeof REAL - 1); + h = bfd_link_hash_lookup (info->hash, n, TRUE, TRUE, FALSE); + free (n); + return h; } + +#undef REAL return h; } -#undef WRAP /* Traverse a generic link hash table. Differs from bfd_hash_traverse in the treatment of warning symbols. When warning symbols are @@ -1400,8 +1372,7 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, /* Create the indirect symbol here. This is for the benefit of the plugin "notice" function. STRING is the name of the symbol we want to indirect to. */ - inh = bfd_wrapped_link_hash_lookup (abfd, info, string, TRUE, - copy, FALSE); + inh = bfd_link_hash_lookup (info->hash, string, TRUE, copy, FALSE); if (inh == NULL) return FALSE; } @@ -1435,10 +1406,7 @@ _bfd_generic_link_add_one_symbol (struct bfd_link_info *info, h = *hashp; else { - if (row == UNDEF_ROW || row == UNDEFW_ROW) - h = bfd_wrapped_link_hash_lookup (abfd, info, name, TRUE, copy, FALSE); - else - h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE); + h = bfd_link_hash_lookup (info->hash, name, TRUE, copy, FALSE); if (h == NULL) { if (hashp != NULL) @@ -2045,11 +2013,6 @@ _bfd_generic_link_output_symbols (bfd *output_bfd, the relocs in the output format being used. */ h = NULL; } - else if (bfd_is_und_section (bfd_get_section (sym))) - h = ((struct generic_link_hash_entry *) - bfd_wrapped_link_hash_lookup (output_bfd, info, - bfd_asymbol_name (sym), - FALSE, FALSE, TRUE)); else h = _bfd_generic_link_hash_lookup (_bfd_generic_hash_table (info), bfd_asymbol_name (sym), @@ -2352,9 +2315,8 @@ _bfd_generic_reloc_link_order (bfd *abfd, struct generic_link_hash_entry *h; h = ((struct generic_link_hash_entry *) - bfd_wrapped_link_hash_lookup (abfd, info, - link_order->u.reloc.p->u.name, - FALSE, FALSE, TRUE)); + bfd_link_hash_lookup (info->hash, link_order->u.reloc.p->u.name, + FALSE, FALSE, TRUE)); if (h == NULL || ! h->written) { @@ -2609,10 +2571,6 @@ default_indirect_link_order (bfd *output_bfd, generic_link_add_symbol_list. */ if (sym->udata.p != NULL) h = (struct bfd_link_hash_entry *) sym->udata.p; - else if (bfd_is_und_section (bfd_get_section (sym))) - h = bfd_wrapped_link_hash_lookup (output_bfd, info, - bfd_asymbol_name (sym), - FALSE, FALSE, TRUE); else h = bfd_link_hash_lookup (info->hash, bfd_asymbol_name (sym), diff --git a/include/bfdlink.h b/include/bfdlink.h index bad52f9c50..2f3586cf11 100644 --- a/include/bfdlink.h +++ b/include/bfdlink.h @@ -221,11 +221,9 @@ extern struct bfd_link_hash_entry *bfd_wrapped_link_hash_lookup (bfd *, struct bfd_link_info *, const char *, bfd_boolean, bfd_boolean, bfd_boolean); -/* If H is a wrapped symbol, ie. the symbol name starts with "__wrap_" - and the remainder is found in wrap_hash, return the real symbol. */ - -extern struct bfd_link_hash_entry *unwrap_hash_lookup - (struct bfd_link_info *, bfd *, struct bfd_link_hash_entry *); +extern struct bfd_link_hash_entry *wrap_hash_lookup_for_reloc + (struct bfd_link_info *, bfd *, struct bfd_section *, + struct bfd_link_hash_entry *); /* Traverse a link hash table. */ extern void bfd_link_hash_traverse diff --git a/ld/ldexp.c b/ld/ldexp.c index 60b17ef576..f0bd5157f9 100644 --- a/ld/ldexp.c +++ b/ld/ldexp.c @@ -705,10 +705,8 @@ fold_name (etree_type *tree) break; case DEFINED: - h = bfd_wrapped_link_hash_lookup (link_info.output_bfd, - &link_info, - tree->name.name, - FALSE, FALSE, TRUE); + h = bfd_link_hash_lookup (link_info.hash, tree->name.name, FALSE, FALSE, + TRUE); new_number (h != NULL && (h->type == bfd_link_hash_defined || h->type == bfd_link_hash_defweak @@ -724,10 +722,8 @@ fold_name (etree_type *tree) new_rel_from_abs (expld.dot); else { - h = bfd_wrapped_link_hash_lookup (link_info.output_bfd, - &link_info, - tree->name.name, - TRUE, FALSE, TRUE); + h = bfd_link_hash_lookup (link_info.hash, tree->name.name, TRUE, + FALSE, TRUE); if (!h) einfo (_("%F%P: bfd_link_hash_lookup failed: %E\n")); else if (h->type == bfd_link_hash_defined @@ -943,10 +939,8 @@ is_sym_value (const etree_type *tree, bfd_vma val) && tree->type.node_code == NAME && (def = symbol_defined (tree->name.name)) != NULL && def->iteration == (lang_statement_iteration & 255) - && (h = bfd_wrapped_link_hash_lookup (link_info.output_bfd, - &link_info, - tree->name.name, - FALSE, FALSE, TRUE)) != NULL + && (h = bfd_link_hash_lookup (link_info.hash, tree->name.name, + FALSE, FALSE, TRUE)) != NULL && h->ldscript_def && h->type == bfd_link_hash_defined && h->u.def.section == bfd_abs_section_ptr diff --git a/ld/plugin.c b/ld/plugin.c index ea1a7f7064..a5566b449b 100644 --- a/ld/plugin.c +++ b/ld/plugin.c @@ -675,12 +675,8 @@ get_symbols (const void *handle, int nsyms, struct ld_plugin_symbol *syms, asection *owner_sec; int res; - if (syms[n].def != LDPK_UNDEF) - blhe = bfd_link_hash_lookup (link_info.hash, syms[n].name, + blhe = bfd_link_hash_lookup (link_info.hash, syms[n].name, FALSE, FALSE, TRUE); - else - blhe = bfd_wrapped_link_hash_lookup (link_info.output_bfd, &link_info, - syms[n].name, FALSE, FALSE, TRUE); if (!blhe) { /* The plugin is called to claim symbols in an archive element -- 2.16.4 ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-28 9:28 ` Sebastian Huber @ 2019-01-29 0:31 ` Alan Modra 2019-01-29 7:44 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Alan Modra @ 2019-01-29 0:31 UTC (permalink / raw) To: Sebastian Huber; +Cc: joel, binutils On Mon, Jan 28, 2019 at 10:28:24AM +0100, Sebastian Huber wrote: > On 26/01/2019 00:52, Alan Modra wrote: > > On Fri, Jan 25, 2019 at 02:55:35PM +0100, Sebastian Huber wrote: > > > @@ -2780,6 +2780,8 @@ extern asection _bfd_elf_large_com_section; > > > return FALSE; \ > > > \ > > > h = sym_hashes[r_symndx - symtab_hdr->sh_info]; \ > > Is it possible to arrange for the symbol hashes to be set up to point > > to the wrapped symbols? I'm thinking a change to always use > > bfd_wrapped_link_hash_lookup in _bfd_generic_link_add_one_symbol might > > work. > > Is this hash table only used for the relocations? We need mappings for No. There are other uses. > This leads to multiple definition errors and unresolved references: > > build/ld/ld-new: main2.o: in function `__wrap_f': > main2.c:(.text+0x11): multiple definition of `__wrap_f'; > main2.o:main2.c:(.text+0x0): first defined here Ah yes, I forgot the obvious. _bfd_generic_link_add_one_symbol itself uses the symbol hash. OK, so always substituting in _bfd_generic_link_add_one_symbol may not be a good idea. > I don't know why there is a special case for debug sections in > RELOC_FOR_GLOBAL_SYMBOL(): In general, debug info is describing the local function in an object file, not some other function. See commit 8a5da09b9e log: Unwrap symbols for debug information Fixes issues with dwz multi-file (-m) and ld's -wrap option. Symbols referenced from DWARF debug info in a separate file, eg. to specify low and high pc, must use the real symbol. The DWARF info is specifying attributes of the real function, not one interposed with --wrap. > Attached is a second version which does the wrapping in > RELOC_FOR_GLOBAL_SYMBOL(). It removes bfd_wrapped_link_hash_lookup(). I am > not sure if this is a good idea. This function is used in various places. > Maybe the wrapping needs to be done on other spots and not only in > RELOC_FOR_GLOBAL_SYMBOL(). Yes, wrapping should happen for more than just relocs. For instance, if a shared library references malloc and you're building an executable wih --wrap=malloc and have a __wrap_malloc, then I think the shared library reference ought to be satisfied by your __wrap_malloc. That has implications for ELF symbol merging. Note that I'm not even sure whether this works currently, but it is feasible that a shared library reference to malloc be satisfied by your __wrap_malloc while your __real_malloc results in malloc@@GLIBC_2.2.5 say. If that does work currently we wouldn't want to break it.. -- Alan Modra Australia Development Lab, IBM ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-29 0:31 ` Alan Modra @ 2019-01-29 7:44 ` Sebastian Huber 2019-01-29 11:02 ` Sebastian Huber 0 siblings, 1 reply; 28+ messages in thread From: Sebastian Huber @ 2019-01-29 7:44 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils On 29/01/2019 01:31, Alan Modra wrote: >> Attached is a second version which does the wrapping in >> RELOC_FOR_GLOBAL_SYMBOL(). It removes bfd_wrapped_link_hash_lookup(). I am >> not sure if this is a good idea. This function is used in various places. >> Maybe the wrapping needs to be done on other spots and not only in >> RELOC_FOR_GLOBAL_SYMBOL(). > Yes, wrapping should happen for more than just relocs. For instance, > if a shared library references malloc and you're building an > executable wih --wrap=malloc and have a __wrap_malloc, then I think > the shared library reference ought to be satisfied by your > __wrap_malloc. That has implications for ELF symbol merging. Note > that I'm not even sure whether this works currently, but it is > feasible that a shared library reference to malloc be satisfied by > your __wrap_malloc while your __real_malloc results in > malloc@@GLIBC_2.2.5 say. If that does work currently we wouldn't want > to break it.. Yes, there are LD testsuite failures with this patch exactly in this area. If I keep the symbol wrapping as is and in addition add it to RELOC_FOR_GLOBAL_SYMBOL(), then there is the problem that the __real_SYM -> SYM information was lost and we wrap SYM always to __wrap_SYM. This leads to an infinite recursion in: void __wrap_f(void) { __real_f(); } Looks like no matter how I try it, it almost works. There are probably some reasons why it wasn't added for defined references in the last 20 years. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
* Re: Does the LD --wrap feature work for library internal references? 2019-01-29 7:44 ` Sebastian Huber @ 2019-01-29 11:02 ` Sebastian Huber 0 siblings, 0 replies; 28+ messages in thread From: Sebastian Huber @ 2019-01-29 11:02 UTC (permalink / raw) To: Alan Modra; +Cc: joel, binutils On 29/01/2019 08:44, Sebastian Huber wrote: > On 29/01/2019 01:31, Alan Modra wrote: >>> Attached is a second version which does the wrapping in >>> RELOC_FOR_GLOBAL_SYMBOL(). It removes >>> bfd_wrapped_link_hash_lookup(). I am >>> not sure if this is a good idea. This function is used in various >>> places. >>> Maybe the wrapping needs to be done on other spots and not only in >>> RELOC_FOR_GLOBAL_SYMBOL(). >> Yes, wrapping should happen for more than just relocs. For instance, >> if a shared library references malloc and you're building an >> executable wih --wrap=malloc and have a __wrap_malloc, then I think >> the shared library reference ought to be satisfied by your >> __wrap_malloc. That has implications for ELF symbol merging. Note >> that I'm not even sure whether this works currently, but it is >> feasible that a shared library reference to malloc be satisfied by >> your __wrap_malloc while your __real_malloc results in >> malloc@@GLIBC_2.2.5 say. If that does work currently we wouldn't want >> to break it.. > > Yes, there are LD testsuite failures with this patch exactly in this > area. If I keep the symbol wrapping as is and in addition add it to > RELOC_FOR_GLOBAL_SYMBOL(), then there is the problem that the > __real_SYM -> SYM information was lost and we wrap SYM always to > __wrap_SYM. This leads to an infinite recursion in: > > void __wrap_f(void) > { > __real_f(); > } > > Looks like no matter how I try it, it almost works. There are probably > some reasons why it wasn't added for defined references in the last 20 > years. > I think we have three options, 1. leave it as is, 2. remove the wrapping via the symbol hash table and add the wrapping to all symbol table consumers, e.g. global symbol relocations, and whatever, or 3. add a new option, e.g. --wrap-reloc which is orthogonal to the existing --wrap option and just deals with wrappings of global symbol relocations. I don't know the linker code good enough to determine if 2. would work and how much effort it needs. -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.huber@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG. ^ permalink raw reply [flat|nested] 28+ messages in thread
end of thread, other threads:[~2019-01-29 11:02 UTC | newest] Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-12-19 13:11 Does the LD --wrap feature work for library internal references? Sebastian Huber 2019-01-03 15:49 ` Nick Clifton 2019-01-04 7:03 ` Sebastian Huber 2019-01-04 13:49 ` Nick Clifton 2019-01-04 14:12 ` Sebastian Huber 2019-01-04 14:27 ` Nick Clifton 2019-01-04 19:45 ` Sebastian Huber 2019-01-05 18:51 ` Sebastian Huber 2019-01-10 11:13 ` Nick Clifton 2019-01-10 12:14 ` Sebastian Huber 2019-01-10 13:09 ` Alan Modra 2019-01-10 13:12 ` Sebastian Huber 2019-01-10 13:40 ` Alan Modra 2019-01-10 13:48 ` Sebastian Huber 2019-01-10 14:05 ` Alan Modra 2019-01-10 19:04 ` Joel Sherrill 2019-01-14 9:41 ` Sebastian Huber 2019-01-18 0:24 ` Alan Modra 2019-01-18 9:04 ` Sebastian Huber 2019-01-18 13:41 ` Alan Modra 2019-01-23 9:11 ` Sebastian Huber 2019-01-25 8:37 ` Sebastian Huber 2019-01-25 13:55 ` Sebastian Huber 2019-01-25 23:52 ` Alan Modra 2019-01-28 9:28 ` Sebastian Huber 2019-01-29 0:31 ` Alan Modra 2019-01-29 7:44 ` Sebastian Huber 2019-01-29 11:02 ` Sebastian Huber
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).