Skip to content

[ELF] Initialize Symbol fields in the constructor instead of via memset#198129

Merged
MaskRay merged 1 commit into
llvm:mainfrom
MaskRay:users/MaskRay/maskray/elf-symbol-init-1
May 16, 2026
Merged

[ELF] Initialize Symbol fields in the constructor instead of via memset#198129
MaskRay merged 1 commit into
llvm:mainfrom
MaskRay:users/MaskRay/maskray/elf-symbol-init-1

Conversation

@MaskRay

@MaskRay MaskRay commented May 16, 2026

Copy link
Copy Markdown
Member

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.

`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.
@MaskRay MaskRay added the skip-precommit-approval PR for CI feedback, not intended for review label May 16, 2026
@MaskRay MaskRay enabled auto-merge (squash) May 16, 2026 21:30
@MaskRay MaskRay merged commit 905a88b into llvm:main May 16, 2026
14 checks passed
@nathanchance

Copy link
Copy Markdown
Member

Thanks for the fix! Should this be applied to release/22.x?

pedroMVicente pushed a commit to pedroMVicente/llvm-project that referenced this pull request 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.
@thesamesam

Copy link
Copy Markdown
Member

/cherry-pick 905a88b

@llvmbot

llvmbot commented May 19, 2026

Copy link
Copy Markdown
Member

/cherry-pick 905a88b

Error: Command failed due to missing milestone.

@thesamesam

Copy link
Copy Markdown
Member

/cherry-pick 905a88b

@llvmbot

llvmbot commented May 19, 2026

Copy link
Copy Markdown
Member

/pull-request #198665

c-rhodes pushed a commit to llvmbot/llvm-project that referenced this pull request May 26, 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)
daunabomba pushed a commit to daunabomba/llvm-project that referenced this pull request Jun 2, 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)
nga888 added a commit to nga888/llvm-project that referenced this pull request Jun 9, 2026
On the "main" branch, Symbol no longer has the `used` member which was
moved in PR llvm#190117 (commit 6a87416). Therefore, when the cherry pick
from PR llvm#198129 (commit 905a88b) was merged to the "release/22.x"
branch, the initialization of `used` was lost.
dyung pushed a commit to nga888/llvm-project that referenced this pull request Jun 10, 2026
On the "main" branch, Symbol no longer has the `used` member which was
moved in PR llvm#190117 (commit 6a87416). Therefore, when the cherry pick
from PR llvm#198129 (commit 905a88b) was merged to the "release/22.x"
branch, the initialization of `used` was lost.
daunabomba pushed a commit to daunabomba/llvm-project that referenced this pull request Jun 17, 2026
On the "main" branch, Symbol no longer has the `used` member which was
moved in PR llvm#190117 (commit 6a87416). Therefore, when the cherry pick
from PR llvm#198129 (commit 905a88b) was merged to the "release/22.x"
branch, the initialization of `used` was lost.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lld:ELF lld skip-precommit-approval PR for CI feedback, not intended for review

Projects

Development

Successfully merging this pull request may close these issues.

4 participants