Project

General

Profile

Actions

Bug #14250

closed

ld should resolve discarded COMDAT symbols against their mates

Added by Rich Lowe 6 months ago. Updated 6 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
tools - gate/build tools
Start date:
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:

Description

We got a bug report regarding building clang, and also building rust that errors like the following were seen:

Text relocation remains                     referenced
    against symbol            offset    in file
_ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x14          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x8b          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x1a          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x403         ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x55          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZZN4llvm7hashing6detail18get_execution_seedEvE4seed 0xbc          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x2c          ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o
_ZZN4llvm7hashing6detail18get_execution_seedEvE4seed 0x438         ar-objs/libLLVMAArch64CodeGen/AArch64PBQPRegAlloc.cpp.o

jclulow noted that this went away if the object files involved were linked in the opposite order (after much narrowing of the test case).

It turns out this is because despite being in a COMDAT group together, the symbols in question have differing visibility

      [68]  0x0000000000000000 0x0000000000000008  OBJT WEAK  H    0 .bss._ZGVZN4ll _ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed
      [69]  0x0000000000000000 0x0000000000000008  OBJT WEAK  H    0 .bss._ZZN4llvm _ZZN4llvm7hashing6detail18get_execution_seedEvE4seed

versus

     [280]  0x0000000000000000 0x0000000000000008  OBJT WEAK  D    0 .bss._ZGVZN4ll _ZGVZN4llvm7hashing6detail18get_execution_seedEvE4seed
     [281]  0x0000000000000000 0x0000000000000008  OBJT WEAK  D    0 .bss._ZZN4llvm _ZZN4llvm7hashing6detail18get_execution_seedEvE4seed

This is against the rules of COMDAT, which are that the members of a group must be equivalent. Where the default v. hidden visibility here is clearly not. However, this is a very easy situation to run into, both because of compiler mistakes producing objects, or build system mistakes using the compilers -fvisibility* options inconsistently.

In this case this causes problems, because if we take the section with default visibility it is not reduced in scope to local during the link-edit and so non-pic relocations against it can't occur at link-edit time, resulting in the outstanding text relocations.

It turns out that what other link-editors are doing (though not necessarily explicitly for this reason, in the case of lld it is an attempt to deal with another common COMDAT mishap) is allowing symbol resolution to occur here, which is defined to reduce a symbol to the most restrictive visibility of its inputs. This means that the symbol will be hidden regardless, and relocations can be processed during the link-edit.

We should follow suit and turn symbol definitions in discarded COMDAT groups into eliminated symbol references so that symbol resolution occurs and visibility etc are propagated (but if all of a group is discarded, no symbol makes it into the output symbol table).

While a stickler for the rules would suggest that, at most, we should handle this under -zrelaxreloc, we have no obvious clue to enable this automatically as we do in other cases, and forcing it from the user is unfriendly.

Actions #1

Updated by Electric Monk 6 months ago

  • Gerrit CR set to 1817
Actions #2

Updated by Rich Lowe 6 months ago

testing:

Built and wsdiff'd illumos (no changes except the SGS version number).

Built llvm/clang (which failed before). Confirmed it failed still on old code but worked on new code. Ran the (extensive) llvm/clang test suites, no failures that are attributable to this change.

Built rust (and llvm), which failed before for the same reason as llvm, but also has extensive tests which I ran with no failures attributable to these changes.

Actions #3

Updated by Electric Monk 6 months ago

  • Status changed from In Progress to Closed
  • % Done changed from 0 to 100

git commit 55d6cb5d63bcf69dfa47b8c41c770a2d34f169b0

commit  55d6cb5d63bcf69dfa47b8c41c770a2d34f169b0
Author: Richard Lowe <richlowe@richlowe.net>
Date:   2021-12-06T20:11:40.000Z

    14250 ld should resolve discarded COMDAT symbols against their mates
    Reviewed by: Robert Mustacchi <rm@fingolfin.org>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Actions

Also available in: Atom PDF