[PATCH] PR ld/19636: pie changes program behavior and generate unnecessary dynamic symbols

[H.J. Lu]

On Wed, Feb 17, 2016 at 10:31 PM, Alan Modra <amodra@gmail.com> wrote:
> On Wed, Feb 17, 2016 at 08:24:06PM -0800, H.J. Lu wrote:
>> On Wed, Feb 17, 2016 at 4:42 PM, Alan Modra <amodra@gmail.com> wrote:
>> > On Wed, Feb 17, 2016 at 03:33:51PM -0800, H.J. Lu wrote:
>> >> On Tue, Feb 16, 2016 at 9:23 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
>> >> > On Mon, Feb 15, 2016 at 07:00:32AM -0800, H.J. Lu wrote:
>> >> >> I fixed x86.  Other backends need similar fix.  Any comments?
>> >
>> > If you aren't going to fix other backends too, have you at least run
>> > the testsuite for all the other ELF targets that use SYMBOLIC_BIND?
>>
>> Should I enable them for all ELF target with PIE/PIC support?
>> All of them fail since none of them handle PIE correctly.
>
> No, I meant the existing testsuite, not your new tests.  These changes
> are quite likely to break existing tests.
>
>> >> -/* Will a symbol be bound to the definition within the shared
>> >> -   library, if any.  A unique symbol can never be bound locally.  */
>> >> -#define SYMBOLIC_BIND(INFO, H) \
>> >> -    (!(H)->unique_global \
>> >> -     && ((INFO)->symbolic || ((INFO)->dynamic && !(H)->dynamic)))
>> >> +/* Will a symbol be bound to the definition within the PIC object, if
>> >> +   any.  A unique symbol can never be bound locally.  Symbols are always
>> >> +   bound locally in PIE, similar to -shared -Bsymbolic.  */
>> >> +#define SYMBOLIC_BIND(INFO, H)                                               \
>> >> +    (!(H)->unique_global                                             \
>> >> +     && ((INFO)->symbolic                                            \
>> >> +      || ((INFO)->dynamic && !(H)->dynamic)                          \
>> >> +      || bfd_link_pie (INFO)))
>> >
>> > This probably should be bfd_link_executable rather than bfd_link_pie.
>>
>> SYMBOLIC_BIND was intended for shared library and is used only when
>> PIC is true.  It is never applied to bfd_link_hash_undefweak.  That is why
>> I added bfd_link_pie.  Change it to bfd_link_executable is a good start if
>> we want to extend it to executable where bfd_link_hash_undefweak is a
>> special case.  bfd_link_hash_undefweak always binds locally in executable
>> and never binds locally in shared library.   Should we extend SYMBOLIC_BIND
>> to cover all cases so that we don't have check PIC nor undefweak before
>> using it?
>
> The reason I suggested bfd_link_executable was simply that it is a
> property of ELF executables in general that symbols bind locally, not
> just a property of PIEs.  I wouldn't try to extend SYMBOLIC_BIND to
> other cases just yet, and your claim about bfd_link_hash_undefweak is
> not true.  That was the whole point of my comment below.

I removed the SYMBOLIC_BIND change.

>> > This change is incorrect.  Some targets (even x86_64 with -fPIC)
>> > support
>> >   if (fun)
>> >     fun ();
>> > for an undefined weak fun at link time, making fun dynamic and
>> > emitting dynamic relocs against fun.  So when the executable is linked
>> > against a new shared library that provides fun, fun will be called.
>
>> Yes, it should be moved before
>
> No, that change should not be added at all!

I moved it to x86 backend via elf_backend_fixup_symbol.

>> It exposed more issues in x86 backends :-).
>
> I'm not surprised.  :)  I tried something similar many years ago, but
> ran into "issues" in the powerpc backends, and decided I didn't care
> enough about a few extra symbols.
>

Here is the final patch.  There are some extra failures:

crisv32-linux:
New failures:
FAIL: ld-elf/pr19636-1d
FAIL: ld-elf/pr19636-1e
FAIL: ld-elf/pr19636-2a

frv-linux:
New failures:
FAIL: FRV uClinux PIC relocs to weak undefined symbols, pie linking
FAIL: ld-elf/pr19636-1d
FAIL: ld-elf/pr19636-1e

i370-linux:
New failures:
FAIL: ld-elf/pr19636-1d
FAIL: ld-elf/pr19636-1e
FAIL: ld-elf/pr19636-2b
FAIL: ld-elf/pr19636-2c

frv-elf:
New failures:
FAIL: FRV uClinux PIC relocs to weak undefined symbols, pie linking

They are due to the existing issues in those backends.

-- 
H.J.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Bind-symbols-to-their-definitions-in-executable.patch
Type: text/x-patch
Size: 83134 bytes
Desc: not available
URL: <https://sourceware.org/pipermail/binutils/attachments/20160221/d309d7bf/attachment.bin>