Does the LD --wrap feature work for library internal references?

Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Nick Clifton]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Nick Clifton]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Nick Clifton]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

[-- 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 --]

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

----- 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?

Re: Does the LD --wrap feature work for library internal references?

[Nick Clifton]

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

 

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

[-- 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 --]

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Joel Sherrill]

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
>

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

[-- 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

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

[-- 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

Re: Does the LD --wrap feature work for library internal references?

[Alan Modra]

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

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.

Re: Does the LD --wrap feature work for library internal references?

[Sebastian Huber]

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.