[PATCH] D63752: Fix an issue that common symbols are not internalized under some condition.

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added a comment.

`compileBitcodeFiles<ELFT>();` is executed before `replaceCommonSymbols()`.
When `includeInDynsym()` is called to compute `VisibileToRegularObj`, it unnecessarily returned true for a common symbol.

The description can be generalized a bit:

The issue occurred under the following condition:

- There exists a common symbol.
- At least one DSO is given to lld, or -pie (i.e. ld.lld t.o t1.so or ld.lld t.o -pie)



================
Comment at: lld/test/ELF/lto/common4.ll:2
+; REQUIRES: x86
+; RUN: echo | llvm-mc -filetype=obj -triple=x86_64-unknown-linux -o %t.so.o
+; RUN: ld.lld -shared -o %t.so %t.so.o
----------------
`llvm-mc -filetype=obj -triple=x86_64-unknown-linux /dev/null -o %t.so.o`

/dev/null has been used in many other tests.


================
Comment at: lld/test/ELF/lto/common4.ll:6
+; RUN: llvm-as %s -o %t.o
+; RUN: ld.lld %t.o -o %t.exe -save-temps %t.so
+; RUN: llvm-dis < %t.exe.0.2.internalize.bc | FileCheck %s
----------------
Nit: put input files together, `%t.o %t.so`


================
Comment at: lld/test/ELF/lto/common4.ll:8
+; RUN: llvm-dis < %t.exe.0.2.internalize.bc | FileCheck %s
+
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
----------------
You may add another RUN+CHECK lines: `ld.lld -pie %t.o -o %t.exe -save-temps`

The purpose of the input DSO is to make HasDynSymTab true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63752