Bug 26580 - Size and alignment from as-needed shared library bss symbols affect commons
Summary: Size and alignment from as-needed shared library bss symbols affect commons
Status: RESOLVED FIXED
Alias: None
Product: binutils
Classification: Unclassified
Component: ld (show other bugs)
Version: 2.36
: P2 normal
Target Milestone: 2.36
Assignee: Alan Modra
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-09-08 02:35 UTC by Alan Modra
Modified: 2020-09-10 09:29 UTC (History)
1 user (show)

See Also:
Host:
Target:
Build:
Last reconfirmed:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Modra 2020-09-08 02:35:20 UTC
An as-needed shared library that isn't needed can affect common symbol size and alignment.

cat > a.c <<\EOF
#include <stdio.h>

extern void __attribute__ ((weak)) foo (void);

char x, y, z;

long
lowest_align (void *a, void *b, void *c)
{
  unsigned long bits = (long) a | (long) b | (long) c;
  return bits & -bits;
}

int
main (void)
{
  printf ("library %sloaded\n", &foo ? "" : "not ");
  printf ("alignment %ld\n", lowest_align (&x, &y, &z));
  return 0;
}
EOF
cat > b.c <<\EOF
long long x, y, z;

void foo (void) {}
EOF
gcc -O2 -fpic -fcommon -shared -o libb.so b.c
gcc -O2 -fpic -fcommon -o y1 a.c -Wl,--no-as-needed,-R,. libb.so
./y1
library loaded
alignment 8
gcc -O2 -fpic -fcommon -o y2 a.c -Wl,--as-needed,-R,. libb.so
./y2
library not loaded
alignment 8
gcc -O2 -fpic -fcommon -o y3 a.c
./y3
library not loaded
alignment 1

I reckon y2 and y3 should print the same result.
Comment 1 Sourceware Commits 2020-09-08 13:02:00 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=7ba115508aa02ffbb01a09613b5dffdd0c6563e3

commit 7ba115508aa02ffbb01a09613b5dffdd0c6563e3
Author: Alan Modra <amodra@gmail.com>
Date:   Tue Sep 8 13:02:31 2020 +0930

    PR26580, Size and alignment of commons vs as-needed shared lib
    
    Two pieces to this puzzle:
    1) Revert HJ's fix for PR13250 so that size and alignment isn't
       sticky, instead attack the real underlying problem that
       _bfd_generic_link_add_one_symbol does the wrong thing in making a
       common section in a shared library bfd.
    2) Save and restore common u.c.p fields, which hold the section and
       alignment.
    
    A better fix for (2) would be to throw away all of that horrible code
    saving and restoring the hash table when loading as-needed library
    symbols, and instead do a scan over as-needed library symbols before
    adding anything.
    
    bfd/
            PR 13250
            PR 26580
            * elflink.c (_bfd_elf_merge_symbol): Make "override" a bfd**.
            Return oldbfd in override when old common should override new
            common.
            (_bfd_elf_add_default_symbol): Adjust to suit.
            (elf_link_add_object_symbols): Likewise.  Pass "override" to
            _bfd_generic_link_add_one_symbol.  Save and restore common u.c.p
            field for --as-needed shared libraries.  Revert pr13250 changes.
    ld/
            * testsuite/ld-elf/pr26580-a.s,
            * testsuite/ld-elf/pr26580-b.s,
            * testsuite/ld-elf/pr26580-1.sd,
            * testsuite/ld-elf/pr26580-2.sd: New tests
            * testsuite/ld-elf/comm-data.exp: Run new tests.
            * testsuite/ld-elf/pr26580-a.c,
            * testsuite/ld-elf/pr26580-b.c,
            * testsuite/ld-elf/pr26580-3.out,
            * testsuite/ld-elf/pr26580-4.out: New tests.
            * testsuite/ld-elf/shared.exp: Run new tests.
Comment 2 Alan Modra 2020-09-09 08:23:49 UTC
Fixed
Comment 3 Nick Clifton 2020-09-09 11:07:37 UTC
Hi Alan,

I am seeing new linker testsuite failures for the pr26580-2 test for the following targets:

  mips-elf

    extra regexps in ld/testsuite/ld-elf/pr26580-2.sd 
    starting with "^.* 8 OBJECT  GLOBAL DEFAULT .* one$"

  mn10300-elf

    ./ld-new: tmpdir/pr26580-2: error: PHDR segment not covered by LOAD segment

  score-elf

    ./ld-new: tmpdir/pr26580-2: error: PHDR segment not covered by LOAD segment

Are these expected, or do we need to investigate further ?

Cheers
  Nick
Comment 4 Alan Modra 2020-09-09 13:12:24 UTC
bfin-elf
./ld-new: the bfin target does not currently support the generation of copy relocations

and other mips targets too.  I saw all of these myself before committing the test, and decided they were sufficently unusual that I shouldn't just hide them with an xfail.  At least not until investigating further.
Comment 5 Sourceware Commits 2020-09-10 09:29:22 UTC
The master branch has been updated by Alan Modra <amodra@sourceware.org>:

https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=36efe0aea5f7e5088f79e4c8d0265de0c0ec2be9

commit 36efe0aea5f7e5088f79e4c8d0265de0c0ec2be9
Author: Alan Modra <amodra@gmail.com>
Date:   Thu Sep 10 18:46:14 2020 +0930

    Re: PR26580, Size and alignment of commons vs as-needed shared lib
    
    Some MIPS targets, for reasons I didn't analyse, use the larger common
    symbol in a shared lib rather than a smaller common in an executable.
    That doesn't seem unreasonable, so allow that to pass for pr26580-2.
    
    bfin-elf complains about not supporting copy relocs, but it's quite
    silly to want a copy reloc for common symbols, so leave the fail
    there.  mn10300-elf and score-elf both fail the test with "PHDR
    segment not covered by LOAD segment".  Other tests fail similarly so
    one more doesn't hurt.  The failure is a consequence of supporting
    dynamic objects but setting EMBEDDED in ld scripts.
    
            PR 26580
            * testsuite/ld-elf/pr26580-2.sd: Accept undefined symbol.