[PATCH] D71795: [ELF] Delete an unused special rule from isStaticLinkTimeConstant. NFC

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

In D71795#1818468 <https://reviews.llvm.org/D71795#1818468>, @pcc wrote:

> > Weak undefined symbols are preemptible after D71794 <https://reviews.llvm.org/D71794>.
>
> What about hidden weak undefined symbols? We now reject the following input:
>
>    cat foo.cpp
>    __attribute__((weak, visibility("hidden"))) void f();
>   
>    void g() {
>      if (f) f();
>    }
>    $ clang -shared -fuse-ld=lld foo.cpp
>    ld.lld: error: relocation R_X86_64_PLT32 cannot refer to absolute symbol: f()
>   >>> defined in /tmp/foo-014d89.o
>   >>> referenced by foo.cpp
>   >>>               /tmp/foo-014d89.o:(g())
>    clang: error: linker command failed with exit code 1 (use -v to see invocation)
>   
>
> This seems like a reasonable thing to want to do. I actually hit this problem in a slightly different case (`--exclude-libs` demoted the weak undef to hidden) which we could fix like so:
>
>   diff --git a/lld/ELF/Driver.cpp b/lld/ELF/Driver.cpp
>   index 19598f52cd4..54c033303f6 100644
>   --- a/lld/ELF/Driver.cpp
>   +++ b/lld/ELF/Driver.cpp
>   @@ -1374,7 +1374,7 @@ static void excludeLibs(opt::InputArgList &args) {
>        if (!file->archiveName.empty())
>          if (all || libs.count(path::filename(file->archiveName)))
>            for (Symbol *sym : file->getSymbols())
>   -          if (!sym->isLocal() && sym->file == file)
>   +          if (!sym->isUndefined() && !sym->isLocal() && sym->file == file)
>                sym->versionId = VER_NDX_LOCAL;
>      };
>   
>
>
> But I'm somewhat more concerned about the explicit hidden case.


This patch looks good. Only defined symbols should have versions. I think the logic will also be closer to GNU ld:

  // bfd/elflink.c
  	  /* If this symbol has default visibility and the user has
  	     requested we not re-export it, then mark it as hidden.  */
  	  if (!bfd_is_und_section (sec)
  	      && !dynamic
  	      && abfd->no_export
  	      && ELF_ST_VISIBILITY (isym->st_other) != STV_INTERNAL)
  	    isym->st_other = (STV_HIDDEN
  			      | (isym->st_other & ~ELF_ST_VISIBILITY (-1)));

If you haven't started working on this yet, I can send a patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71795