Skip to content

release/22.x: [ELF] Initialize Symbol fields in the constructor instead of via memset (#198129)#198665

Merged
c-rhodes merged 1 commit into
llvm:release/22.xfrom
llvmbot:issue198129
May 26, 2026
Merged

release/22.x: [ELF] Initialize Symbol fields in the constructor instead of via memset (#198129)#198665
c-rhodes merged 1 commit into
llvm:release/22.xfrom
llvmbot:issue198129

Conversation

@llvmbot

@llvmbot llvmbot commented May 19, 2026

Copy link
Copy Markdown
Member

Backport 905a88b

Requested by: @thesamesam

@llvmorg-github-actions

llvmorg-github-actions Bot commented May 19, 2026

Copy link
Copy Markdown

@llvm/pr-subscribers-lld-elf

@llvm/pr-subscribers-lld

Author: llvmbot

Changes

Backport 905a88b

Requested by: @thesamesam


Full diff: https://github.com/llvm/llvm-project/pull/198665.diff

2 Files Affected:

  • (modified) lld/ELF/InputFiles.cpp (-2)
  • (modified) lld/ELF/Symbols.h (+12-8)
diff --git a/lld/ELF/InputFiles.cpp b/lld/ELF/InputFiles.cpp
index 176f6937a8d66..7ba2be5d1608f 100644
--- a/lld/ELF/InputFiles.cpp
+++ b/lld/ELF/InputFiles.cpp
@@ -1261,7 +1261,6 @@ void ObjFile<ELFT>::initSectionsAndLocalSyms(bool ignoreComdats) {
   if (!firstGlobal)
     return;
   SymbolUnion *locals = makeThreadLocalN<SymbolUnion>(firstGlobal);
-  memset(locals, 0, sizeof(SymbolUnion) * firstGlobal);
 
   ArrayRef<Elf_Sym> eSyms = this->getELFSyms<ELFT>();
   for (size_t i = 0, end = firstGlobal; i != end; ++i) {
@@ -1297,7 +1296,6 @@ void ObjFile<ELFT>::initSectionsAndLocalSyms(bool ignoreComdats) {
     else
       new (symbols[i]) Defined(ctx, this, name, STB_LOCAL, eSym.st_other, type,
                                eSym.st_value, eSym.st_size, sec);
-    symbols[i]->partition = 1;
     symbols[i]->isUsedInRegularObj = true;
   }
 }
diff --git a/lld/ELF/Symbols.h b/lld/ELF/Symbols.h
index 034c8734addb8..23994ccea1ef0 100644
--- a/lld/ELF/Symbols.h
+++ b/lld/ELF/Symbols.h
@@ -102,7 +102,7 @@ class Symbol {
   uint8_t symbolKind;
 
   // The partition whose dynamic symbol table contains this symbol's definition.
-  uint8_t partition;
+  uint8_t partition = 1;
 
   // True if this symbol is preemptible at load time.
   //
@@ -242,8 +242,13 @@ class Symbol {
   Symbol(Kind k, InputFile *file, StringRef name, uint8_t binding,
          uint8_t stOther, uint8_t type)
       : file(file), nameData(name.data()), nameSize(name.size()), type(type),
-        binding(binding), stOther(stOther), symbolKind(k), ltoCanOmit(false),
-        archSpecificBit(false) {}
+        binding(binding), stOther(stOther), symbolKind(k), isPreemptible(false),
+        isUsedInRegularObj(false), isExported(false), ltoCanOmit(false),
+        traced(false), hasVersionSuffix(false), isInIplt(false),
+        gotInIgot(false), folded(false), archSpecificBit(false),
+        scriptDefined(false), dsoDefined(false), dsoProtected(false),
+        versionScriptAssigned(false), thunkAccessed(false),
+        inDynamicList(false), referenced(false), referencedAfterWrap(false) {}
 
   void overwrite(Symbol &sym, Kind k) const {
     if (sym.traced)
@@ -302,12 +307,12 @@ class Symbol {
 
   // Temporary flags used to communicate which symbol entries need PLT and GOT
   // entries during postScanRelocations();
-  std::atomic<uint16_t> flags;
+  std::atomic<uint16_t> flags = 0;
 
   // A ctx.symAux index used to access GOT/PLT entry indexes. This is allocated
   // in postScanRelocations().
-  uint32_t auxIdx;
-  uint32_t dynsymIndex;
+  uint32_t auxIdx = 0;
+  uint32_t dynsymIndex = 0;
 
   // If `file` is SharedFile (for SharedSymbol or copy-relocated Defined), this
   // represents the Verdef index within the input DSO, which will be converted
@@ -315,7 +320,7 @@ class Symbol {
   // index (VER_NDX_LOCAL, VER_NDX_GLOBAL, or a named version).
   // VER_NDX_LOCAL indicates a defined symbol that has been localized by a
   // version script's local: directive or --exclude-libs.
-  uint16_t versionId;
+  uint16_t versionId = 0;
   LLVM_PREFERRED_TYPE(bool)
   uint8_t versionScriptAssigned : 1;
 
@@ -526,7 +531,6 @@ union SymbolUnion {
 
 template <typename... T> Defined *makeDefined(T &&...args) {
   auto *sym = getSpecificAllocSingleton<SymbolUnion>().Allocate();
-  memset(sym, 0, sizeof(Symbol));
   auto &s = *new (reinterpret_cast<Defined *>(sym)) Defined(std::forward<T>(args)...);
   return &s;
 }

@thesamesam

Copy link
Copy Markdown
Member

cc @nickdesaulniers @MaskRay

@nickdesaulniers

Copy link
Copy Markdown
Member

LGTM

@github-project-automation github-project-automation Bot moved this from Needs Triage to Needs Merge in LLVM Release Status May 19, 2026
…et (llvm#198129)

`initSectionsAndLocalSyms` and `makeDefined` memset the storage to zero
and then placement-new a Symbol-derived object into it. Placement new
begins a new object's lifetime. The standard does not seem to guarantee
the memset bytes carry into members the constructor leaves
uninitialized.

lld built by GCC 16 can make Valgrind report reads of Symbol::flags
(via getSymSectionIndex during finalizeSections) as uses of
uninitialized values (ClangBuiltLinux/linux#2162).

This patch reinstates the per-field initialization that commit
7787427 ("[ELF] Avoid redundant assignment to Symbol fields. NFC")
had replaced with a bulk memset.

(cherry picked from commit 905a88b)
@c-rhodes c-rhodes merged commit 4d5dffb into llvm:release/22.x May 26, 2026
@github-project-automation github-project-automation Bot moved this from Needs Merge to Done in LLVM Release Status May 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Development

Successfully merging this pull request may close these issues.

5 participants