Page MenuHomePhabricator

RFC: [ELF] -Bsymbolic-non-weak-functions
AcceptedPublic

Authored by MaskRay on May 15 2021, 11:02 PM.

Details

Summary

This option is a subset of -Bsymbolic-functions. It applies to STB_GLOBAL
STT_FUNC definitions.

The address of a vague linkage function (STB_WEAK STT_FUNC, e.g. an inline
function, a template instantiation) seen by a -Bsymbolic-functions linked
shared object may be different from the address seen from outside the shared
object. Such cases are rare. (ELF/Mach-O programs may use
-fvisibility-inlines-hidden to break such pointer equality. On Windows,
correct dllexport and dllimport are needed to make pointer equality work.
Windows link.exe enables /OPT:ICF by default so different inline functions may
have the same address.)

// a.cc -> a.o -> a.so (-Bsymbolic-functions)
inline void f() {}
void *g() { return (void *)&f; }

// b.cc -> b.o -> exe
// The address is different!
inline void f() {}

-Bsymbolic-non-weak-functions is a safer (C++ conforming) subset of
-Bsymbolic-functions.

Implementations usually emit a vague linkage definition in a COMDAT group. We
could detect the group (with more code) but I feel that we should just check
STB_WEAK for simplicity. A weak definition will thus serve as an escape hatch
for rare cases when users want interposition on definitions.

GNU ld feature request: https://sourceware.org/bugzilla/show_bug.cgi?id=27871

Longer write-up: https://maskray.me/blog/2021-05-16-elf-interposition-and-bsymbolic

Diff Detail

Event Timeline

MaskRay created this revision.May 15 2021, 11:02 PM
MaskRay requested review of this revision.May 15 2021, 11:02 PM
Herald added a project: Restricted Project. · View Herald TranscriptMay 15 2021, 11:02 PM
MaskRay edited the summary of this revision. (Show Details)May 15 2021, 11:03 PM
MaskRay edited the summary of this revision. (Show Details)May 15 2021, 11:07 PM

No objections from me, but we don't use symbol preemption in our downstream codebase, so I haven't got much experience with the implications one way or another, so probably best others give their points of view.

I don't have any strong objections. Just want to make sure that weak symbols are the right subset to use. My understanding is that this is coming from vague linkage https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague the only part that I could find in there about weak definitions is in static data: (https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-static)

Some objects with static storage duration have associated guard variables used to ensure that they are initialized only once (see 3.3.3). If the object is emitted using a COMDAT group, the guard variable must be too. It is suggested that it be emitted in the same COMDAT group as the associated data object, but it may be emitted in its own COMDAT group, identified by its name. In either case, it must be weak.

The part about out of line copies of inline functions https://itanium-cxx-abi.github.io/cxx-abi/abi.html#vague-inline says to define them in a comdat group but nothing about weak definitions. I think that both clang and GCC use weak definitions for functions in COMDAT groups but is that guaranteed of all compilers that might use LLD?

Some suggestions:

  • Is -Bsymbolic-non-vague-functions more descriptive, or perhaps -Bsymbolic-non-weak-functions? When I see global-functions as a programmer, not a linker person, I think of non-static functions rather than non-weak functions.
  • Would "Not defined in a COMDAT group" be a better criteria for the subset than non-weak? This would correctly find global symbols defined in COMDAT groups? Certainly more work to implement.
MaskRay updated this revision to Diff 345972.May 17 2021, 12:37 PM
MaskRay retitled this revision from RFC: [ELF] -Bsymbolic-global-functions to RFC: [ELF] -Bsymbolic-non-weak-functions.
MaskRay edited the summary of this revision. (Show Details)

-> -Bsymbolic-non-weak-functions

peter.smith accepted this revision.May 18 2021, 6:32 AM

Thanks for the update. I think it is a well defined useful subset of -Bsymbolic-functions so I'm happy to approve.

This revision is now accepted and ready to land.May 18 2021, 6:32 AM