[PATCH] D65716: [ELF] Consistently priority non-* wildcards overs "*" in version scripts

Mon Aug 5 04:57:20 PDT 2019

MaskRay marked 6 inline comments as done.
MaskRay added a comment.

In D65716#1614431 <https://reviews.llvm.org/D65716#1614431>, @peter.smith wrote:

> Overall looks good to me. I've got a few suggestions on the refactoring. One other thing that might be worth doing to make this easier to review is to split it into two patches:
>
> - A refactoring change that doesn't change the wildcard behaviour.
> - The change to the wildcard behaviour. Not got a strong opinion on that now I've gone through it.

I starts from two patches :/ 1) merge `config->versionScript{Locals,Globals}` into `config->versionDefinitions` 2) delete `defaultSymbolVersion`. Then I realized just 1) would fix the ld.bfd/gold incompatibility though the code was a big uglier in `readAnonymousDeclaration` and `readVersionDeclaration`. So I think probably I should just merge 2) into 1)...

================
Comment at: ELF/SymbolTable.cpp:207
+    if (sym->verdefIndex == UINT32_C(-1)) {
+      sym->verdefIndex = 0;
       sym->versionId = versionId;
----------------
peter.smith wrote:
> Would prefer verdefIndex = VER_NDX_LOCAL;
Here 0 (any number other than -1 works) is an arbitrary number to indicate the version has been assigned. Added a comment about this.

This change is why we can detect `{ global: foo; };` and `V1 { global: foo};` duplicate assignment now.

Repository:
  rLLD LLVM Linker

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

https://reviews.llvm.org/D65716