pmodels / mpich

Official MPICH Repository
http://www.mpich.org
Other
545 stars 281 forks source link

Fix attribute weak,alias usage in MPICH #2002

Closed mpichbot closed 8 years ago

mpichbot commented 8 years ago

Originally by @raffenet on 2014-01-13 18:27:59 -0600


Jed Brown reported this issue to the mailing list. His report is below. The clang issue he references has been fixed in newer versions, but we should test for it in case users haven't updated.

On 01/06/2014 12:14 PM, Jed Brown wrote:

MPICH's test for weak symbols is:

int foo(int) attribute((weak,alias("__foo")));

where __foo is not defined in the same compilation unit. This fails with gcc (tested with 4.8 and 4.4):

conftest.c:37:5: error: 'foo' aliased to undefined symbol 'foo' int foo(int) __attribute((weak,alias("__foo"))); ^

This is (evidently) a change from gcc-3.x, but is intentional because knowledge of the weak alias is not desirable/meaningful in other compilation units.

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20652

My understanding of gcc's intent is that the attribute only be stated in the implementation. The caller can't do anything with the information that foo is a weak alias; it still needs to generate a PLT entry and the loader will need to perform a relocation.

Now my problem is that clang accepts the test code above (i.e., it does not insist that __foo be defined in the same compilation unit) which causes

define MPICH_ATTR_WEAK_ALIAS(fname) attribute((weak,alias(fname)))

but this breaks PMPI and when -fvisibility=hidden is used to compile user code, it generates (objdump -r)

RELOCATION RECORDS FOR [.text]: OFFSET TYPE VALUE 0000000000000005 R_X86_64_PC32 __foo-0x0000000000000004

instead of the linkable (but not desired because it prevents interposing "foo")

RELOCATION RECORDS FOR [.text]: OFFSET TYPE VALUE 0000000000000005 R_X86_64_PLT32 __foo-0x0000000000000004

What we actually want is an honest relocation for "foo":

RELOCATION RECORDS FOR [.text]: OFFSET TYPE VALUE 0000000000000005 R_X86_64_PLT32 foo-0x0000000000000004

When you try to link the first version later, you get

/usr/bin/ld.gold: error: bar.o: requires dynamic R_X86_64_PC32 reloc against '__foo' which may overflow at runtime; recompile with -fPIC

which is not actually an -fPIC error, but rather a result of clang using R_X86_64_PC32 for a symbol that we really want to be R_X86_64_PLT32. I think this should probably be reported to Clang/LLVM (because it delays a confusing error condition), but I also think that MPICH should fix its usage.

So in summary, we can't have attribute((weak,alias("PMPI_..."))) in the header because:

  1. it does not compile with gcc
  2. it breaks PMPI interposition with clang
  3. it breaks -fvisibility=hidden with clang

Instead, it should go in the implementation files.

mpichbot commented 8 years ago

Originally by apenya on 2014-03-17 14:57:18 -0500


As the old behavior seems to be treated as a bug and the compilers have switched to the new one, I'll adjust our test accordingly. In case the user is having an old compiler, the weak,alias support will be disabled.

mpichbot commented 8 years ago

Originally by Ken Raffenetti raffenet@mcs.anl.gov on 2014-07-07 16:37:18 -0500


In 0011344aee9fadd4c2e879da9b14e764bfbf837a: Fix weak alias support detection

Since GCC4, it is required that when declaring a function alias, the target function is defined in the same compilation unit.

See #2002

Signed-off-by: Ken Raffenetti raffenet@mcs.anl.gov

mpichbot commented 8 years ago

Originally by Ken Raffenetti raffenet@mcs.anl.gov on 2014-07-07 16:37:18 -0500


In b208dd70c27812397afe3568ba97fd835c00030e: Fix usage of weak,alias attributes

Moved the weak,alias attribute declarations from header files to the implementation. Complies with the requirement that alias targets are defined in the same compilation unit.

Fixes #2002

Signed-off-by: Ken Raffenetti raffenet@mcs.anl.gov

mpichbot commented 8 years ago

Originally by apenya on 2014-07-07 16:41:05 -0500


Attachment added: parse.py (3.4 KiB) Helper script to include the attribute weak,alias definition in the implementation files