[PATCH] D98686: [lld:elf] Weaken the requirement for a computed binding to be STB_LOCAL

[Nathan Lanza via Phabricator via llvm-commits]

lanza added a comment.



> The `isDefined()` requirement is to make `version-script-weak.s` work: a STB_WEAK lazy symbol, if not fetched, should be treated similar to an undefined weak symbol. An undefined symbol should not have non-`VER_NDX_GLOBAL` `versionId` values.



> Can you elaborate what happens? b.o has only one symbol `_Z14doThingWithCatP3Cat`. There is no vtable symbol...

Yup, sorry, my minimized case was too minimized. Here's one that performs devirtualization on

`a.cpp`

  struct Animal {
    virtual void makeNoise() const = 0;
  };
  
  struct Cat : public Animal {
    void makeNoise() const override;
  };
  
  extern "C" int puts(char const *);
  
  void Cat::makeNoise() const { puts("Meow"); }
  
  void doThingWithCat(Animal *a) {
    if (auto d = reinterpret_cast<Cat *>(a))
      d->makeNoise();
  }

``b.cpp`

  struct Animal {
    virtual void makeNoise() const = 0;
  };
  
  struct Cat : public Animal {
    void makeNoise() const override;
  };
  
  void doThingWithCat(Animal *a);
  
  void useDoThingWithCat() {
    Cat *d = new Cat;
    doThingWithCat(d);
  }

`version.exp`

  {
    global:
      _Z17useDoThingWithCatv;
    local:
    *;
  };

clang++-13 invocation

  clang++ Cat.cpp CatUser.cpp -o libA.so -shared -std=c++20 -O3 -fuse-ld=lld \
                        -fwhole-program-vtables -flto=thin \
                        -Wl,--lto-whole-program-visibility \
                        -Wl,--version-script,version.exp \
                        -Wl,--opt-remarks-format,yaml -Wl,--opt-remarks-filename,libA.yaml -fpic

`_ZTV3Cat` occurs 4 times here -- twice for each file due to split LTO. Two instances have `isDefined() == true` and two have `isDefined() == false`. My change instead tests `!isLazy()` which they are both non-lazy.

> Only canBeVersioned symbols can have a versionId value not equal to VER_NDX_GLOBAL. How does the isDefine) -> !isLazy() change affect behaviors?

I'm guessing that since the symbol is seen and defined during the initial symbol collection from `Cat.cpp.o` that the vtbl is `isDefined()` by the time the parsing of the version-script happens. Thus the version is marked as local but it is marked as `UndefinedKind`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D98686/new/

https://reviews.llvm.org/D98686