Skip to content

[lld][ELF] Introduce an option to keep data section prefix. #148985

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

[lld][ELF] Introduce an option to keep data section prefix. #148985

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
24c1c62
lld keep data section prefix
mingmingl-llvm Mar 30, 2025
6328b71
simplify implementation
mingmingl-llvm Jul 21, 2025
25c6149
update code comment, and add a regression test
mingmingl-llvm Jul 21, 2025
67e44ed
resolve comments
mingmingl-llvm Jul 22, 2025
ed70327
resolve comments
mingmingl-llvm Jul 23, 2025
17639b1
run 'git merge main'
mingmingl-llvm Jul 23, 2025
b66ecee
use {{.*}} to omit irrelevant columns in the test case
mingmingl-llvm Jul 23, 2025
5dbe4df
resolve comment
mingmingl-llvm Jul 27, 2025
28b26f8
drop braces for single-line if
mingmingl-llvm Aug 4, 2025
a451287
incorporate review feedback
mingmingl-llvm Aug 5, 2025
ed99ca5
undo a test change
mingmingl-llvm Aug 7, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 42 additions & 7 deletions lld/ELF/LinkerScript.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,12 @@ static bool isSectionPrefix(StringRef prefix, StringRef name) {
return name.consume_front(prefix) && (name.empty() || name[0] == '.');
}

// If name starts with prefix, trim the prefix and return true.
// Otherwise, leave the name unchanged and return false.
static bool trimSectionPrefix(StringRef prefix, StringRef &name) {
return name.consume_front(prefix) && (name.empty() || name[0] == '.');
}

StringRef LinkerScript::getOutputSectionName(const InputSectionBase *s) const {
// This is for --emit-relocs and -r. If .text.foo is emitted as .text.bar, we
// want to emit .rela.text.foo as .rela.text.bar for consistency (this is not
Expand Down Expand Up @@ -103,13 +109,42 @@ StringRef LinkerScript::getOutputSectionName(const InputSectionBase *s) const {
return ".text";
}

for (StringRef v : {".data.rel.ro", ".data", ".rodata",
".bss.rel.ro", ".bss", ".ldata",
".lrodata", ".lbss", ".gcc_except_table",
".init_array", ".fini_array", ".tbss",
".tdata", ".ARM.exidx", ".ARM.extab",
".ctors", ".dtors", ".sbss",
".sdata", ".srodata"})
// When zKeepDataSectionPrefix is true, keep .hot and .unlikely suffixes
// in data sections.
static constexpr StringRef dataSectionPrefixes[] = {
".data.rel.ro", ".data", ".rodata", ".bss.rel.ro", ".bss",
};

for (auto [index, v] : llvm::enumerate(dataSectionPrefixes)) {
StringRef secName = s->name;
if (trimSectionPrefix(v, secName)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can use early return for trimSectionPrefix. The helper is only called once. Inlining it is more readable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

inlined the helper function and used 'continue;' to reduce nested IFs inside this loop.

if (!ctx.arg.zKeepDataSectionPrefix)
return v;
if (isSectionPrefix(".hot", secName))
return s->name.substr(0, v.size() + 4);
if (isSectionPrefix(".unlikely", secName))
return s->name.substr(0, v.size() + 9);
// For .rodata, a section could be`.rodata.cst<N>.hot.` for constant
// pool or `rodata.str<N>.hot.` for string literals.
if (index == 2) {
// The reason to specialize this path is to spell out .rodata.hot and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is too verbose. I would specify:

// Place input .rodata.cst<N>.hot. into .rodata.hot.

and delete the previous comment

// For .rodata, a section could be.rodata.cst<N>.hot. for constant
// pool or rodata.str<N>.hot. for string literals.

The code block has fewer than 20 lines, making it easy for users to understand its semantics without adding comments that redundantly explain the code.

Now I am unsure why we want to support .rodata.hot while we don't support .rodata.cst4.hot.

(The keep-text-data-sections was added long ago when we supported both .text.hot and .text.hot. In clang, we now always emit .text.hot.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is too verbose ... making it easy for users to understand its semantics without adding comments that redundantly explain the code

done.

Now I am unsure why we want to support .rodata.hot while we don't support .rodata.cst4.hot.

If I understand correctly, the question is about why having [1] when .rodata.cst4.hot doesn't end with a trailing dot.

Basically, [1] is meant to to group both string literals (with name pattern .rodata.str1.1.unlikely. [2]) and constant pools (with name pattern .rodata.cst<N>.hot [3]). As the test case examples show, string literal section names have a trailing dot but the constant pool section names don't.

Based on #148985 (comment), we need trailing dot to disambiguate between .rodata.<var-name> and .rodata.<hotness> when <var-name> is hot (something like https://gcc.godbolt.org/z/95f4oafcn). I prepared #150859 to add trailing dot for constant pools when there is .hot or .unlikely in the section names. Without PR 150859, only string literals will be grouped with [1].

[1]

if (index == 2) {
      // Place input .rodata.str<N>.hot. or .rodata.cst<N>.hot. into the
      // .rodata.hot section.
      if (s->name.ends_with(".hot."))
        return ".rodata.hot";
      // Place input .rodata.str<N>.hot. or .rodata.cst<N>.unlikely. into the
      // .rodata.unlikely section.
      if (s->name.ends_with(".unlikely."))
        return ".rodata.unlikely";
    }
}

[2]

llvm-project/llvm/test/CodeGen/X86/global-variable-partition.ll

Lines 60 to 66 in 314e22b

; For @.str.2
; COMMON: .type .L.str.2,@object
; SYM-NEXT: .section .rodata.str1.1.unlikely.,"aMS",@progbits,1
; UNIQ-NEXT: .section .rodata.str1.1.unlikely.,"aMS",@progbits,1
; AGG-NEXT: .section .rodata.str1.1.unlikely.,"aMS",@progbits,1
; COMMON-NEXT: .L.str.2:
; COMMON-NEXT: "cold%d\t%d\t%d\n"

[3]

llvm-project/llvm/test/CodeGen/X86/constant-pool-partition.ll

Lines 27 to 31 in 314e22b

; CHECK: .section .rodata.cst8.hot,"aM",@progbits,8
; CHECK-NEXT: .p2align
; CHECK-NEXT: .LCPI0_0:
; CHECK-NEXT: .quad 0x3fe5c28f5c28f5c3 # double 0.68000000000000005
; CHECK-NEXT: .section .rodata.cst8.unlikely,"aM",@progbits,8

// .rodata.unlikely as string literals for StringRef lifetime
// considerations. We cannot use the pattern above to get substr from
// section names.
if (s->name.ends_with(".hot."))
return ".rodata.hot";

if (s->name.ends_with(".unlikely."))
return ".rodata.unlikely";
}
return v;
}
}

for (StringRef v :
{".ldata", ".lrodata", ".lbss", ".gcc_except_table", ".init_array",
".fini_array", ".tbss", ".tdata", ".ARM.exidx", ".ARM.extab", ".ctors",
".dtors", ".sbss", ".sdata", ".srodata"})
if (isSectionPrefix(v, s->name))
return v;

Expand Down
174 changes: 147 additions & 27 deletions lld/test/ELF/keep-data-section-prefix.s
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,32 +1,49 @@
# REQUIRES: x86
## -z keep-data-section-prefix separates static data sections with prefix
## .<section>.hot, .<section>.unlikely in the absence of a SECTIONS command.

# RUN: rm -rf %t && split-file %s %t && cd %t

## Test that lld knows .data.rel.ro.unlikely and .data.rel.ro.hot are relocatable
## read-only data sections.
# RUN: llvm-mc -filetype=obj -triple=x86_64 a.s -o a.o

# RUN: ld.lld -z keep-data-section-prefix -T x.lds a.o -o out1
# RUN: ld.lld -z keep-data-section-prefix -T x1.lds a.o -o out1
# RUN: llvm-readelf -l out1 | FileCheck --check-prefixes=SEG,LS %s
# RUN: llvm-readelf -S out1 | FileCheck %s --check-prefix=CHECK-LS

# RUN: ld.lld -z keep-data-section-prefix a.o -o out2
# RUN: llvm-readelf -l out2 | FileCheck --check-prefixes=SEG,PRE %s
# RUN: llvm-readelf -S out2 | FileCheck %s --check-prefix=CHECK-PRE

# RUN: ld.lld a.o -o out3
# RUN: llvm-readelf -l out3 | FileCheck --check-prefixes=SEG,PRE %s
# RUN: llvm-readelf -S out3 | FileCheck %s --check-prefix=CHECK-PRE

# RUN: not ld.lld -T x.lds a.o 2>&1 | FileCheck %s
# RUN: not ld.lld -T x1.lds a.o 2>&1 | FileCheck %s
# CHECK: error: section: .relro_padding is not contiguous with other relro sections

## Test that lld can group data sections based on its hotness prefix.

# RUN: llvm-mc -filetype=obj -triple=x86_64 b.s -o b.o

# RUN: ld.lld b.o -o out1
# RUN: llvm-readelf -S out1 | FileCheck --check-prefix=BASIC %s
# RUN: ld.lld -z nokeep-text-section-prefix b.o -o out2
# RUN: cmp out1 out2

# RUN: ld.lld -z keep-data-section-prefix b.o -o out3
# RUN: llvm-readelf -S out3 | FileCheck --check-prefix=KEEP %s

## With a SECTIONS command, orphan sections are created verbatim.
## No grouping is performed for them.
# RUN: ld.lld -T x2.lds -z keep-data-section-prefix b.o -o out4
# RUN: llvm-readelf -S out4 | FileCheck --check-prefix=SCRIPT %s

## The first RW PT_LOAD segment has FileSiz 0x126f (0x1000 + 0x200 + 0x60 + 0xf),
## and its p_offset p_vaddr p_paddr p_filesz should match PT_GNU_RELRO.
# Type Offset VirtAddr PhysAddr FileSiz MemSiz Flg Align
# SEG: LOAD 0x0001c8 0x00000000002011c8 0x00000000002011c8 0x000001 0x000001 R E 0x1000
# SEG-NEXT: LOAD 0x0001c9 0x00000000002021c9 0x00000000002021c9 0x00126f 0x001e37 RW 0x1000
# SEG-NEXT: LOAD 0x001438 0x0000000000204438 0x0000000000204438 0x000001 0x000002 RW 0x1000
# SEG-NEXT: GNU_RELRO 0x0001c9 0x00000000002021c9 0x00000000002021c9 0x00126f 0x001e37 R 0x1
# SEG-NEXT: GNU_STACK 0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW 0x0
# Type {{.*}} FileSiz MemSiz Flg
# SEG: LOAD {{.*}} 0x000001 0x000001 R E
# SEG-NEXT: LOAD {{.*}} 0x00126f 0x001e37 RW
# SEG-NEXT: LOAD {{.*}} 0x000001 0x000002 RW
# SEG-NEXT: GNU_RELRO {{.*}} 0x00126f 0x001e37 R
# SEG-NEXT: GNU_STACK {{.*}} 0x000000 0x000000 RW

## Input to output mapping per linker script
## .data.rel.ro.split -> .data.rel.ro
Expand All @@ -37,34 +54,80 @@
# LS-NEXT: .data.rel.ro.hot .data.rel.ro .data.rel.ro.unlikely .relro_padding
# LS-NEXT: .data .bss

# [Nr] Name Type Address Off Size
# CHECK-LS: .data.rel.ro.hot PROGBITS 00000000002021c9 0001c9 00000f
# CHECK-LS-NEXT: .data.rel.ro PROGBITS 00000000002021d8 0001d8 000260
# CHECK-LS-NEXT: .data.rel.ro.unlikely PROGBITS 0000000000202438 000438 001000
# CHECK-LS-NEXT: .relro_padding NOBITS 0000000000203438 001438 000bc8
# CHECK-LS-NEXT: .data PROGBITS 0000000000204438 001438 000001
# CHECK-LS-NEXT: .bss NOBITS 0000000000204439 001439 000001
# [Nr] Name Type {{.*}} Size
# CHECK-LS: .data.rel.ro.hot PROGBITS {{.*}} 00000f
# CHECK-LS-NEXT: .data.rel.ro PROGBITS {{.*}} 000260
# CHECK-LS-NEXT: .data.rel.ro.unlikely PROGBITS {{.*}} 001000
# CHECK-LS-NEXT: .relro_padding NOBITS {{.*}} 000bc8
# CHECK-LS-NEXT: .data PROGBITS {{.*}} 000001
# CHECK-LS-NEXT: .bss NOBITS {{.*}} 000001

## Linker script is not provided to map data sections.
## So all input sections with prefix .data.rel.ro will map to .data.rel.ro in the output.
# PRE: .text
# PRE-NEXT: .data.rel.ro .relro_padding
# PRE-NEXT: .data .bss

# [Nr] Name Type Address Off Size
# CHECK-PRE: .data.rel.ro PROGBITS 00000000002021c9 0001c9 00126f
# CHECK-PRE-NEXT: .relro_padding NOBITS 0000000000203438 001438 000bc8
# CHECK-PRE-NEXT: .data PROGBITS 0000000000204438 001438 000001
# CHECK-PRE-NEXT: .bss NOBITS 0000000000204439 001439 000001

#--- x.lds
# [Nr] Name Type {{.*}} Size
# CHECK-PRE: .data.rel.ro PROGBITS {{.*}} 00126f
# CHECK-PRE-NEXT: .relro_padding NOBITS {{.*}} 000bc8
# CHECK-PRE-NEXT: .data PROGBITS {{.*}} 000001
# CHECK-PRE-NEXT: .bss NOBITS {{.*}} 000001

# BASIC: [Nr] Name Type {{.*}} Size
# BASIC: [ 1] .text
# BASIC-NEXT: [ 2] .data.rel.ro PROGBITS {{.*}} 00000f
# BASIC-NEXT: [ 3] .bss.rel.ro NOBITS {{.*}} 000008
# BASIC-NEXT: [ 4] .relro_padding NOBITS {{.*}} 000e20
# BASIC-NEXT: [ 5] .rodata PROGBITS {{.*}} 000006
# BASIC-NEXT: [ 6] .data PROGBITS {{.*}} 000004
# BASIC-NEXT: [ 7] .bss NOBITS {{.*}} 000004

# KEEP: [Nr] Name Type {{.*}} Size
# KEEP: [ 1] .text
# KEEP-NEXT: [ 2] .data.rel.ro PROGBITS {{.*}} 000009
# KEEP-NEXT: [ 3] .data.rel.ro.hot PROGBITS {{.*}} 000004
# KEEP-NEXT: [ 4] .data.rel.ro.unlikely PROGBITS {{.*}} 000002
# KEEP-NEXT: [ 5] .bss.rel.ro NOBITS {{.*}} 000008
# KEEP-NEXT: [ 6] .relro_padding NOBITS {{.*}} 000e20
# KEEP-NEXT: [ 7] .rodata PROGBITS {{.*}} 000002
# KEEP-NEXT: [ 8] .rodata.hot PROGBITS {{.*}} 000002
# KEEP-NEXT: [ 9] .rodata.unlikely PROGBITS {{.*}} 000002
# KEEP-NEXT: [10] .data PROGBITS {{.*}} 000002
# KEEP-NEXT: [11] .data.hot PROGBITS {{.*}} 000001
# KEEP-NEXT: [12] .data.unlikely PROGBITS {{.*}} 000001
# KEEP-NEXT: [13] .bss NOBITS {{.*}} 000002
# KEEP-NEXT: [14] .bss.hot NOBITS {{.*}} 000001
# KEEP-NEXT: [15] .bss.unlikely NOBITS {{.*}} 000001

# SCRIPT: .text
# SCRIPT-NEXT: .bss.rel.ro
# SCRIPT-NEXT: .rodata.i
# SCRIPT-NEXT: .rodata.hot.
# SCRIPT-NEXT: .rodata.unlikely.k
# SCRIPT-NEXT: .rodata.split.l
# SCRIPT-NEXT: .rodata.cst32.hot.
# SCRIPT-NEXT: .rodata.str1.1.unlikely.
# SCRIPT-NEXT: .data.m
# SCRIPT-NEXT: .data.hot.n
# SCRIPT-NEXT: .data.unlikely.o
# SCRIPT-NEXT: .data.split.p
# SCRIPT-NEXT: .data.rel.ro.q
# SCRIPT-NEXT: .data.rel.ro.hot.r
# SCRIPT-NEXT: .data.rel.ro.unlikely.s
# SCRIPT-NEXT: .data.rel.ro.split.t
# SCRIPT-NEXT: .bss.a
# SCRIPT-NEXT: .bss.hot.b
# SCRIPT-NEXT: .bss.unlikely.c
# SCRIPT-NEXT: .bss.split.d

#--- x1.lds
SECTIONS {
.data.rel.ro.hot : { *(.data.rel.ro.hot) }
.data.rel.ro : { .data.rel.ro }
.data.rel.ro.unlikely : { *(.data.rel.ro.unlikely) }
} INSERT AFTER .text


#--- a.s
.globl _start
_start:
Expand All @@ -87,3 +150,60 @@ _start:

.section .bss, "aw"
.space 1

#--- b.s
.globl _start
_start:
ret

.section .bss.a,"aw"
.byte 0
.section .bss.hot.b,"aw"
.byte 0
.section .bss.unlikely.c,"aw"
.byte 0
.section .bss.split.d,"aw"
.byte 0

.section .bss.rel.ro, "aw"
.space 2
.section .bss.rel.ro, "aw"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the 4 .bss.rel.ro sections of the same name intended?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah the 4 .bss.rel.ro sections used to have different names. Given the discussion in #148985 (comment), only one section without any prefix should be kept.

.space 2
.section .bss.rel.ro, "aw"
.space 2
.section .bss.rel.ro, "aw"
.space 2

.section .rodata.i,"aw"
.byte 1
.section .rodata.hot.,"aw"
.byte 2
.section .rodata.unlikely.k,"aw"
.byte 3
.section .rodata.split.l,"aw"
.byte 4
.section .rodata.cst32.hot.,"aw"
.byte 5
.section .rodata.str1.1.unlikely.,"aw"
.byte 6

.section .data.m,"aw"
.byte 5
.section .data.hot.n,"aw"
.byte 6
.section .data.unlikely.o,"aw"
.byte 7
.section .data.split.p,"aw"
.byte 8

.section .data.rel.ro.q,"aw"
.quad 0
.section .data.rel.ro.hot.r,"aw"
.long 255
.section .data.rel.ro.unlikely.s,"aw"
.word 1
.section .data.rel.ro.split.t,"aw"
.byte 0

#--- x2.lds
SECTIONS {}