[LTO] Fix the logic for dropping unnamed_addr

[davide]

Summary

https://llvm.org/bugs/show_bug.cgi?id=29155
We should still get this right when we'll switch to the new API (soon'ish)

[• rafael]

testing email

[• rafael]

The change to a test looks wrong.

test/ELF/lto/unnamed-addr-comdat.ll

11 ↗

(On Diff #69713)

Why is it being dropped in this case?

[davide]

Found the thinko, it should be

// Merge in the new unnamed_addr attribute.
if (WasInserted)
  S->HasUnnamedAddr = HasUnnamedAddr;
else
  S->HasUnnamedAddr &= HasUnnamedAddr;

Otherwise HasUnnamedAddr is not set correctly in the case you pointed out. Updating

[• rafael]

ELF/SymbolTable.cpp

222 ↗

(On Diff #69735)

Maybe change this to true.

245 ↗

(On Diff #69735)

And then you don't need this if, no?

[davide]

sure, nice simplification.

lld/trunk/ELF/InputFiles.cpp

SymbolBody *elf::ObjectFile<ELFT>::createSymbolBody(const Elf_Sym *Sym) {
   }
 
   StringRef Name = check(Sym->getName(this->StringTable));
 
   switch (Sym->st_shndx) {
   case SHN_UNDEF:
     return elf::Symtab<ELFT>::X
         ->addUndefined(Name, Binding, Sym->st_other, Sym->getType(),
-                       /*CanOmitFromDynSym*/ false, this)
+                       /*CanOmitFromDynSym*/ false, /*HasUnnamedAddr*/ false,
+                       this)
         ->body();
   case SHN_COMMON:
     return elf::Symtab<ELFT>::X
         ->addCommon(Name, Sym->st_size, Sym->st_value, Binding, Sym->st_other,
-                    Sym->getType(), this)
+                    Sym->getType(), /*HasUnnamedAddr*/ false, this)
         ->body();
   }
 
   switch (Binding) {
   default:
     fatal(getFilename(this) + ": unexpected binding: " + Twine(Binding));
   case STB_GLOBAL:
   case STB_WEAK:
   case STB_GNU_UNIQUE:
     if (Sec == &InputSection<ELFT>::Discarded)
       return elf::Symtab<ELFT>::X
           ->addUndefined(Name, Binding, Sym->st_other, Sym->getType(),
-                         /*CanOmitFromDynSym*/ false, this)
+                         /*CanOmitFromDynSym*/ false,
+                         /*HasUnnamedAddr*/ false, this)
           ->body();
     return elf::Symtab<ELFT>::X->addRegular(Name, *Sym, Sec)->body();
   }
 }
 
 template <class ELFT> void ArchiveFile::parse() {
   File = check(Archive::create(MB), "failed to parse archive");
 
Symbol *BitcodeFile::createSymbol(const DenseSet<const Comdat *> &KeptComdats,
   // FIXME: Expose a thread-local flag for module asm symbols.
   if (GV) {
     if (GV->isThreadLocal())
       Type = STT_TLS;
     CanOmitFromDynSym = canBeOmittedFromSymbolTable(GV);
   }
 
   uint8_t Visibility;
-  if (GV)
+  bool HasUnnamedAddr = false;
+  if (GV) {
     Visibility = getGvVisibility(GV);
-  else
+    HasUnnamedAddr =
+        GV->getUnnamedAddr() == llvm::GlobalValue::UnnamedAddr::Global;
+  } else {
     // FIXME: Set SF_Hidden flag correctly for module asm symbols, and expose
     // protected visibility.
     Visibility = STV_DEFAULT;
+  }
 
   if (GV)
     if (const Comdat *C = GV->getComdat())
       if (!KeptComdats.count(C))
         return Symtab<ELFT>::X->addUndefined(NameRef, Binding, Visibility, Type,
-                                             CanOmitFromDynSym, this);
+                                             CanOmitFromDynSym, HasUnnamedAddr,
+                                             this);
 
   const Module &M = Obj.getModule();
   if (Flags & BasicSymbolRef::SF_Undefined)
     return Symtab<ELFT>::X->addUndefined(NameRef, Binding, Visibility, Type,
-                                         CanOmitFromDynSym, this);
+                                         CanOmitFromDynSym, HasUnnamedAddr,
+                                         this);
   if (Flags & BasicSymbolRef::SF_Common) {
     // FIXME: Set SF_Common flag correctly for module asm symbols, and expose
     // size and alignment.
     assert(GV);
     const DataLayout &DL = M.getDataLayout();
     uint64_t Size = DL.getTypeAllocSize(GV->getValueType());
     return Symtab<ELFT>::X->addCommon(NameRef, Size, GV->getAlignment(),
-                                      Binding, Visibility, STT_OBJECT, this);
+                                      Binding, Visibility, STT_OBJECT,
+                                      HasUnnamedAddr, this);
   }
   return Symtab<ELFT>::X->addBitcode(NameRef, IsWeak, Visibility, Type,
-                                     CanOmitFromDynSym, this);
+                                     CanOmitFromDynSym, HasUnnamedAddr, this);
 }
 
 bool BitcodeFile::shouldSkip(uint32_t Flags) {
   return !(Flags & BasicSymbolRef::SF_Global) ||
          (Flags & BasicSymbolRef::SF_FormatSpecific);
 }
 
 template <class ELFT>

lld/trunk/ELF/LTO.cpp

void BitcodeCompiler::add(BitcodeFile &F) {
   for (const BasicSymbolRef &Sym : Obj->symbols()) {
     uint32_t Flags = Sym.getFlags();
     GlobalValue *GV = Obj->getSymbolGV(Sym.getRawDataRefImpl());
     if (GV && GV->hasAppendingLinkage())
       Keep.push_back(GV);
     if (BitcodeFile::shouldSkip(Flags))
       continue;
     Symbol *S = Syms[BodyIndex++];
+    if (GV) {
+      if (S->HasUnnamedAddr)
+        GV->setUnnamedAddr(GlobalValue::UnnamedAddr::Global);
+      else
+        GV->setUnnamedAddr(GlobalValue::UnnamedAddr::None);
+    }
     if (Flags & BasicSymbolRef::SF_Undefined) {
       handleUndefinedAsmRefs(Sym, GV, AsmUndefinedRefs);
       continue;
     }
     auto *B = dyn_cast<DefinedBitcode>(S->body());
     if (!B || B->file() != &F)
       continue;
 

lld/trunk/ELF/SymbolTable.h

public:
 
   DefinedRegular<ELFT> *addAbsolute(StringRef Name,
                                     uint8_t Visibility = llvm::ELF::STV_HIDDEN);
   DefinedRegular<ELFT> *addIgnored(StringRef Name,
                                    uint8_t Visibility = llvm::ELF::STV_HIDDEN);
 
   Symbol *addUndefined(StringRef Name);
   Symbol *addUndefined(StringRef Name, uint8_t Binding, uint8_t StOther,
-                       uint8_t Type, bool CanOmitFromDynSym, InputFile *File);
+                       uint8_t Type, bool CanOmitFromDynSym,
+                       bool HasUnnamedAddr, InputFile *File);
 
   Symbol *addRegular(StringRef Name, const Elf_Sym &Sym,
                      InputSectionBase<ELFT> *Section);
   Symbol *addRegular(StringRef Name, uint8_t Binding, uint8_t StOther);
   Symbol *addSynthetic(StringRef N, OutputSectionBase<ELFT> *Section,
                        uintX_t Value, uint8_t StOther);
   void addShared(SharedFile<ELFT> *F, StringRef Name, const Elf_Sym &Sym,
                  const typename ELFT::Verdef *Verdef);
 
   void addLazyArchive(ArchiveFile *F, const llvm::object::Archive::Symbol S);
   void addLazyObject(StringRef Name, LazyObjectFile &Obj);
   Symbol *addBitcode(StringRef Name, bool IsWeak, uint8_t StOther, uint8_t Type,
-                     bool CanOmitFromDynSym, BitcodeFile *File);
+                     bool CanOmitFromDynSym, bool HasUnnamedAddr,
+                     BitcodeFile *File);
 
   Symbol *addCommon(StringRef N, uint64_t Size, uint64_t Alignment,
                     uint8_t Binding, uint8_t StOther, uint8_t Type,
-                    InputFile *File);
+                    bool HasUnnamedAddr, InputFile *File);
 
   void scanUndefinedFlags();
   void scanShlibUndefined();
   void scanDynamicList();
   void scanVersionScript();
 
   SymbolBody *find(StringRef Name);
 
   void trace(StringRef Name);
   void wrap(StringRef Name);
 
 private:
   std::vector<SymbolBody *> findAll(StringRef Pattern);
   std::pair<Symbol *, bool> insert(StringRef &Name);
   std::pair<Symbol *, bool> insert(StringRef &Name, uint8_t Type,
                                    uint8_t Visibility, bool CanOmitFromDynSym,
-                                   bool IsUsedInRegularObj, InputFile *File);
+                                   bool HasUnnamedAddr, bool IsUsedInRegularObj,
+                                   InputFile *File);
 
   std::string conflictMsg(SymbolBody *Existing, InputFile *NewFile);
   void reportDuplicate(SymbolBody *Existing, InputFile *NewFile);
 
   std::map<std::string, SymbolBody *> getDemangledSyms();
 
   struct SymIndex {
     SymIndex(int Idx, bool Traced) : Idx(Idx), Traced(Traced) {}

lld/trunk/ELF/SymbolTable.cpp

std::pair<Symbol *, bool> SymbolTable<ELFT>::insert(StringRef &Name) {
   }
 
   Symbol *Sym;
   if (IsNew) {
     Sym = new (Alloc) Symbol;
     Sym->Binding = STB_WEAK;
     Sym->Visibility = STV_DEFAULT;
     Sym->IsUsedInRegularObj = false;
+    Sym->HasUnnamedAddr = true;
     Sym->ExportDynamic = false;
     Sym->Traced = V.Traced;
     std::tie(Name, Sym->VersionId) = getSymbolVersion(Name);
     SymVector.push_back(Sym);
   } else {
     Sym = SymVector[V.Idx];
   }
   return {Sym, IsNew};
 }
 
 // Find an existing symbol or create and insert a new one, then apply the given
 // attributes.
 template <class ELFT>
 std::pair<Symbol *, bool>
 SymbolTable<ELFT>::insert(StringRef &Name, uint8_t Type, uint8_t Visibility,
-                          bool CanOmitFromDynSym, bool IsUsedInRegularObj,
-                          InputFile *File) {
+                          bool CanOmitFromDynSym, bool HasUnnamedAddr,
+                          bool IsUsedInRegularObj, InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(Name);
 
+  // Merge in the new unnamed_addr attribute.
+  S->HasUnnamedAddr &= HasUnnamedAddr;
   // Merge in the new symbol's visibility.
   S->Visibility = getMinVisibility(S->Visibility, Visibility);
   if (!CanOmitFromDynSym && (Config->Shared || Config->ExportDynamic))
     S->ExportDynamic = true;
   if (IsUsedInRegularObj)
     S->IsUsedInRegularObj = true;
   if (!WasInserted && S->body()->Type != SymbolBody::UnknownType &&
       ((Type == STT_TLS) != S->body()->isTls()))
std::string SymbolTable<ELFT>::conflictMsg(SymbolBody *Existing,
   if (Config->Demangle)
     Sym = demangle(Sym);
   return Sym + " in " + getFilename(Existing->File) + " and " +
          getFilename(NewFile);
 }
 
 template <class ELFT> Symbol *SymbolTable<ELFT>::addUndefined(StringRef Name) {
   return addUndefined(Name, STB_GLOBAL, STV_DEFAULT, /*Type*/ 0,
-                      /*CanOmitFromDynSym*/ false, /*File*/ nullptr);
+                      /*CanOmitFromDynSym*/ false, /*HasUnnamedAddr*/ false,
+                      /*File*/ nullptr);
 }
 
 template <class ELFT>
 Symbol *SymbolTable<ELFT>::addUndefined(StringRef Name, uint8_t Binding,
                                         uint8_t StOther, uint8_t Type,
                                         bool CanOmitFromDynSym,
-                                        InputFile *File) {
+                                        bool HasUnnamedAddr, InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
-      insert(Name, Type, StOther & 3, CanOmitFromDynSym,
+      insert(Name, Type, StOther & 3, CanOmitFromDynSym, HasUnnamedAddr,
              /*IsUsedInRegularObj*/ !File || !isa<BitcodeFile>(File), File);
   if (WasInserted) {
     S->Binding = Binding;
     replaceBody<Undefined>(S, Name, StOther, Type, File);
     return S;
   }
   if (Binding != STB_WEAK) {
     if (S->body()->isShared() || S->body()->isLazy())
static int compareDefinedNonCommon(Symbol *S, bool WasInserted,
   }
   return 0;
 }
 
 template <class ELFT>
 Symbol *SymbolTable<ELFT>::addCommon(StringRef N, uint64_t Size,
                                      uint64_t Alignment, uint8_t Binding,
                                      uint8_t StOther, uint8_t Type,
-                                     InputFile *File) {
+                                     bool HasUnnamedAddr, InputFile *File) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
-      insert(N, Type, StOther & 3, /*CanOmitFromDynSym*/ false,
+      insert(N, Type, StOther & 3, /*CanOmitFromDynSym*/ false, HasUnnamedAddr,
              /*IsUsedInRegularObj*/ true, File);
   int Cmp = compareDefined(S, WasInserted, Binding);
   if (Cmp > 0) {
     S->Binding = Binding;
     replaceBody<DefinedCommon<ELFT>>(S, N, Size, Alignment, StOther, Type,
                                      File);
   } else if (Cmp == 0) {
     auto *C = dyn_cast<DefinedCommon<ELFT>>(S->body());
else
     error(Msg);
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, const Elf_Sym &Sym,
                                       InputSectionBase<ELFT> *Section) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) =
-      insert(Name, Sym.getType(), Sym.getVisibility(),
-             /*CanOmitFromDynSym*/ false, /*IsUsedInRegularObj*/ true,
-             Section ? Section->getFile() : nullptr);
+  std::tie(S, WasInserted) = insert(
+      Name, Sym.getType(), Sym.getVisibility(),
+      /*CanOmitFromDynSym*/ false, /*HasUnnamedAddr*/ false,
+      /*IsUsedInRegularObj*/ true, Section ? Section->getFile() : nullptr);
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Sym.getBinding());
   if (Cmp > 0)
     replaceBody<DefinedRegular<ELFT>>(S, Name, Sym, Section);
   else if (Cmp == 0)
     reportDuplicate(S->body(), Section->getFile());
   return S;
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addRegular(StringRef Name, uint8_t Binding,
                                       uint8_t StOther) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(Name, STT_NOTYPE, StOther & 3, /*CanOmitFromDynSym*/ false,
-             /*IsUsedInRegularObj*/ true, nullptr);
+             /*HasUnnamedAddr*/ false, /*IsUsedInRegularObj*/ true, nullptr);
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, Binding);
   if (Cmp > 0)
     replaceBody<DefinedRegular<ELFT>>(S, Name, StOther);
   else if (Cmp == 0)
     reportDuplicate(S->body(), nullptr);
   return S;
 }
 
 template <typename ELFT>
 Symbol *SymbolTable<ELFT>::addSynthetic(StringRef N,
                                         OutputSectionBase<ELFT> *Section,
                                         uintX_t Value, uint8_t StOther) {
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) = insert(N, STT_NOTYPE, /*Visibility*/ StOther & 0x3,
                                     /*CanOmitFromDynSym*/ false,
+                                    /*HasUnnamedAddr*/ false,
                                     /*IsUsedInRegularObj*/ true, nullptr);
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted, STB_GLOBAL);
   if (Cmp > 0)
     replaceBody<DefinedSynthetic<ELFT>>(S, N, Value, Section);
   else if (Cmp == 0)
     reportDuplicate(S->body(), nullptr);
   return S;
 }
 
 template <typename ELFT>
 void SymbolTable<ELFT>::addShared(SharedFile<ELFT> *F, StringRef Name,
                                   const Elf_Sym &Sym,
                                   const typename ELFT::Verdef *Verdef) {
   // DSO symbols do not affect visibility in the output, so we pass STV_DEFAULT
   // as the visibility, which will leave the visibility in the symbol table
   // unchanged.
   Symbol *S;
   bool WasInserted;
   std::tie(S, WasInserted) =
       insert(Name, Sym.getType(), STV_DEFAULT, /*CanOmitFromDynSym*/ true,
-             /*IsUsedInRegularObj*/ false, F);
+             /*HasUnnamedAddr*/ false, /*IsUsedInRegularObj*/ false, F);
   // Make sure we preempt DSO symbols with default visibility.
   if (Sym.getVisibility() == STV_DEFAULT)
     S->ExportDynamic = true;
   if (WasInserted || isa<Undefined>(S->body())) {
     replaceBody<SharedSymbol<ELFT>>(S, F, Name, Sym, Verdef);
     if (!S->isWeak())
       F->IsUsed = true;
   }
 }
 
 template <class ELFT>
 Symbol *SymbolTable<ELFT>::addBitcode(StringRef Name, bool IsWeak,
                                       uint8_t StOther, uint8_t Type,
-                                      bool CanOmitFromDynSym, BitcodeFile *F) {
+                                      bool CanOmitFromDynSym,
+                                      bool HasUnnamedAddr, BitcodeFile *F) {
   Symbol *S;
   bool WasInserted;
-  std::tie(S, WasInserted) = insert(Name, Type, StOther & 3, CanOmitFromDynSym,
-                                    /*IsUsedInRegularObj*/ false, F);
+  std::tie(S, WasInserted) =
+      insert(Name, Type, StOther & 3, CanOmitFromDynSym, HasUnnamedAddr,
   int Cmp = compareDefinedNonCommon<ELFT>(S, WasInserted,
                                           IsWeak ? STB_WEAK : STB_GLOBAL);
   if (Cmp > 0)
     replaceBody<DefinedBitcode>(S, Name, StOther, Type, F);
   else if (Cmp == 0)
     reportDuplicate(S->body(), F);
   return S;
 }

lld/trunk/ELF/Symbols.h

struct Symbol {
 
   // Version definition index.
   uint16_t VersionId;
 
   // Symbol visibility. This is the computed minimum visibility of all
   // observed non-DSO symbols.
   unsigned Visibility : 2;
 
+  // True if the symbol has unnamed_addr.
+  unsigned HasUnnamedAddr : 1;
+
   // True if the symbol was used for linking and thus need to be added to the
   // output file's symbol table. This is true for all symbols except for
   // unreferenced DSO symbols and bitcode symbols that are unreferenced except
   // by other bitcode objects.
   unsigned IsUsedInRegularObj : 1;
 
   // If this flag is true and the symbol has protected or default visibility, it
   // will appear in .dynsym. This flag is set by interposable DSO symbols in

lld/trunk/test/ELF/lto/Inputs/unnamed-addr-drop.ll

+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+@foo = unnamed_addr constant i32 42

lld/trunk/test/ELF/lto/unnamed-addr-drop.ll

+; RUN: llvm-as %s -o %t1.o
+; RUN: llvm-as %S/Inputs/unnamed-addr-drop.ll -o %t2.o
+; RUN: ld.lld -m elf_x86_64 %t1.o %t2.o -o %t.so -save-temps -shared
+; RUN: llvm-dis %t.so.lto.bc -o - | FileCheck %s
+
+target triple = "x86_64-unknown-linux-gnu"
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+@foo = weak constant i32 41
+
+; Check that unnamed_addr is dropped during the merge.
+; CHECK: @foo = constant i32 42

lld/trunk/test/ELF/lto/unnamed-addr-lib.ll

 ; REQUIRES: x86
 ; RUN: llvm-as %s -o %t.o
 ; RUN: llvm-mc %p/Inputs/unnamed-addr-lib.s -o %t2.o -filetype=obj -triple=x86_64-pc-linux
 ; RUN: ld.lld %t2.o -shared -o %t2.so
 ; RUN: ld.lld -m elf_x86_64 %t.o %t2.so -o %t.so -save-temps -shared
 ; RUN: llvm-dis %t.so.lto.bc -o - | FileCheck %s
 
 ; This documents a small limitation of lld's internalization logic. We decide
 ; that bar should be in the symbol table because if it is it will preempt the
 ; one in the shared library.
 ; We could add one extra bit for ODR so that we know that preemption is not
 ; necessary, but that is probably not worth it.
 
-; CHECK: @foo = internal unnamed_addr constant i8 42
-; CHECK: @bar = weak_odr unnamed_addr constant i8 42
+; CHECK: @foo = internal constant i8 42
+; CHECK: @bar = weak_odr constant i8 42
 
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
 @foo = linkonce_odr unnamed_addr constant i8 42
 @bar = linkonce_odr unnamed_addr constant i8 42