[llvm-objcopy][COFF] Support --rename-section - #210573
Conversation
Implement --rename-section for COFF objects. Add tests based on the existing COFF and ELF tests. Move its documentation from the ELF-specific section to the generic options section.
|
Hello @andrew-boyarshin 👋 Thank you for submitting a Pull Request (PR) to the LLVM Project. Since this is your first PR, here are a few useful links covering our main contribution policies and review practices.
Please reply to this message to confirm that you have read these policies, especially the LLVM AI Tool Use Policy, and that any AI tool usage has been noted in the PR description. Frequently asked questionsHow do I add reviewers? This PR will be automatically labeled, and the relevant teams will be notified. For some parts of the project, reviewers may also be added automatically. You can also add reviewers manually using the Reviewers section on this page. If you cannot use that section, it is probably because you do not have write permissions for the repository. In that case, you can request a review by tagging reviewers in a comment using What if there are no comments? If you have not received any comments on your PR after a week, you can request a review by pinging the PR with a comment such as “Ping”. The common courtesy ping rate is once a week. Please remember that you are asking for volunteer time from other developers. Are any special GitHub settings required to contribute to LLVM? We only require contributors to have a public email address associated with their GitHub commits, see this section of LLVM Developer Policy for details. If you have questions, feel free to leave a comment on this PR, or ask on LLVM Discord or LLVM Discourse. Thank you, |
|
@llvm/pr-subscribers-llvm-binary-utilities Author: Andrew Boyarshin (andrew-boyarshin) ChangesImplement Assisted-by: Cursor Grok 4.5 (the tests) Full diff: https://github.com/llvm/llvm-project/pull/210573.diff 9 Files Affected:
diff --git a/llvm/docs/CommandGuide/llvm-objcopy.rst b/llvm/docs/CommandGuide/llvm-objcopy.rst
index 967253d984a35..35334db53d508 100644
--- a/llvm/docs/CommandGuide/llvm-objcopy.rst
+++ b/llvm/docs/CommandGuide/llvm-objcopy.rst
@@ -177,6 +177,13 @@ multiple file formats.
For MachO objects, ``<section>`` must be formatted as
``<segment name>,<section name>``.
+.. option:: --rename-section <old>=<new>[,<flag>,...]
+
+ Rename sections called ``<old>`` to ``<new>`` in the output, and apply any
+ specified ``<flag>`` values. See :option:`--set-section-flags` for a list of
+ supported flags. Can be specified multiple times to rename multiple sections.
+ Supported for ELF and COFF.
+
.. option:: --set-section-alignment <section>=<align>
Set the alignment of section ``<section>`` to ``<align>``. Can be specified
@@ -499,12 +506,6 @@ them.
Remove notes of integer type ``<type>`` and name ``<name>`` from SHT_NOTE
sections that are not in a segment. Can be specified multiple times.
-.. option:: --rename-section <old>=<new>[,<flag>,...]
-
- Rename sections called ``<old>`` to ``<new>`` in the output, and apply any
- specified ``<flag>`` values. See :option:`--set-section-flags` for a list of
- supported flags. Can be specified multiple times to rename multiple sections.
-
.. option:: --set-section-type <section>=<type>
Set the type of section ``<section>`` to the integer ``<type>``. Can be
diff --git a/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp b/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
index 48c488826c47d..7fc0c51e2db20 100644
--- a/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
+++ b/llvm/lib/ObjCopy/COFF/COFFObjcopy.cpp
@@ -309,6 +309,29 @@ static Error handleArgs(const CommonConfig &Config,
if (Error E = addGnuDebugLink(Obj, Config.AddGnuDebugLink))
return E;
+ if (!Config.SectionsToRename.empty()) {
+ for (Section &Sec : Obj.getMutableSections()) {
+ auto It = Config.SectionsToRename.find(Sec.Name);
+ if (It == Config.SectionsToRename.end())
+ continue;
+
+ const SectionRename &SR = It->second;
+ StringRef OldName = Sec.Name;
+ StringRef NewName = SR.NewName;
+
+ // Update section-definition symbols that still share the section's name.
+ for (Symbol &Sym : Obj.getMutableSymbols()) {
+ if (Sym.Name == OldName && Sym.TargetSectionId == Sec.UniqueId)
+ Sym.Name = NewName;
+ }
+
+ Sec.Name = NewName;
+ if (SR.NewFlags)
+ Sec.Header.Characteristics =
+ flagsToCharacteristics(*SR.NewFlags, Sec.Header.Characteristics);
+ }
+ }
+
if (COFFConfig.Subsystem || COFFConfig.MajorSubsystemVersion ||
COFFConfig.MinorSubsystemVersion) {
if (!Obj.IsPE)
diff --git a/llvm/lib/ObjCopy/ConfigManager.cpp b/llvm/lib/ObjCopy/ConfigManager.cpp
index 403b9d1bb922e..e5d0044915c59 100644
--- a/llvm/lib/ObjCopy/ConfigManager.cpp
+++ b/llvm/lib/ObjCopy/ConfigManager.cpp
@@ -26,7 +26,7 @@ Expected<const COFFConfig &> ConfigManager::getCOFFConfig() const {
!Common.AllocSectionsPrefix.empty() || !Common.KeepSection.empty() ||
!Common.SymbolsToGlobalize.empty() || !Common.SymbolsToKeep.empty() ||
!Common.SymbolsToLocalize.empty() || !Common.SymbolsToWeaken.empty() ||
- !Common.SymbolsToKeepGlobal.empty() || !Common.SectionsToRename.empty() ||
+ !Common.SymbolsToKeepGlobal.empty() ||
!Common.SetSectionAlignment.empty() || !Common.SetSectionType.empty() ||
Common.ExtractDWO || Common.StripDWO || Common.StripNonAlloc ||
Common.StripSections || Common.Weaken ||
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section-and-update.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section-and-update.test
new file mode 100644
index 0000000000000..d23fc82c967b1
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section-and-update.test
@@ -0,0 +1,56 @@
+## --update-section / --add-section / --set-section-flags match the original
+## name and run before --rename-section.
+
+# RUN: yaml2obj %s -o %t
+# RUN: echo DEADBEEF > %t.sec
+
+## --update-section then rename:
+# RUN: echo -n AAAB > %t.diff
+# RUN: llvm-objcopy --update-section=.foo=%t.diff --rename-section=.foo=.bar %t %t.1
+# RUN: llvm-readobj --sections --section-data %t.1 | FileCheck %s --check-prefix=UPDATE
+
+# UPDATE: Name: .bar
+# UPDATE: SectionData (
+# UPDATE-NEXT: 0000: 41414142
+# UPDATE-NEXT: )
+
+## --add-section then rename:
+# RUN: llvm-objcopy --add-section=.new=%t.sec --rename-section=.new=.renamed %t %t.2
+# RUN: llvm-readobj --sections --section-data %t.2 | FileCheck %s --check-prefix=ADD
+
+# ADD: Name: .foo
+# ADD: Name: .renamed
+# ADD: SectionData (
+# ADD-NEXT: 0000: {{.*}}DEADBEEF{{.*}}
+# ADD-NEXT: )
+
+## --set-section-flags then rename (flags applied under original name):
+# RUN: llvm-objcopy --set-section-flags=.foo=code --rename-section=.foo=.bar %t %t.3
+# RUN: llvm-readobj --sections %t.3 | FileCheck %s --check-prefix=FLAGS
+
+# FLAGS: Name: .bar
+# FLAGS: Characteristics [
+# FLAGS-NEXT: IMAGE_SCN_ALIGN_4BYTES
+# FLAGS-NEXT: IMAGE_SCN_CNT_CODE
+# FLAGS-NEXT: IMAGE_SCN_MEM_EXECUTE
+# FLAGS-NEXT: IMAGE_SCN_MEM_READ
+# FLAGS-NEXT: IMAGE_SCN_MEM_WRITE
+# FLAGS-NEXT: ]
+
+## Conflict: --set-section-flags on rename destination is rejected (CLI).
+# RUN: not llvm-objcopy --rename-section=.foo=.bar --set-section-flags=.bar=alloc %t %t.4 2>&1 | \
+# RUN: FileCheck %s --check-prefix=CONFLICT
+
+# CONFLICT: --set-section-flags=.bar conflicts with --rename-section=.foo=.bar
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .foo
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+ Alignment: 4
+ SectionData: '41414141'
+symbols:
+...
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section-dollar.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section-dollar.test
new file mode 100644
index 0000000000000..88107075bb912
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section-dollar.test
@@ -0,0 +1,30 @@
+## Renaming to a $ grouping name changes only the logical name.
+## LLVM does not apply GNU PE known-name characteristic forcing;
+## characteristics are not touched unless flags are given.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --rename-section=.foo=.rdata$1 %t %t2
+# RUN: llvm-readobj --sections --section-data %t2 | FileCheck %s
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .foo
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ, IMAGE_SCN_MEM_WRITE ]
+ Alignment: 4
+ SectionData: '01020304'
+symbols:
+...
+
+# CHECK: Name: .rdata$1
+# CHECK: Characteristics [
+# CHECK-NEXT: IMAGE_SCN_ALIGN_4BYTES
+# CHECK-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+# CHECK-NEXT: IMAGE_SCN_MEM_READ
+# CHECK-NEXT: IMAGE_SCN_MEM_WRITE
+# CHECK-NEXT: ]
+# CHECK: SectionData (
+# CHECK-NEXT: 0000: 01020304
+# CHECK-NEXT: )
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section-flags.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section-flags.test
new file mode 100644
index 0000000000000..0b6d993e0d451
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section-flags.test
@@ -0,0 +1,72 @@
+# RUN: yaml2obj %s -o %t
+
+## Single flags via --rename-section (same mapping as --set-section-flags).
+# RUN: llvm-objcopy --rename-section=.foo=.bar,alloc %t %t.alloc
+# RUN: llvm-readobj --sections %t.alloc | FileCheck %s --check-prefixes=CHECK,UNINITDATA,READ,WRITE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,load %t %t.load
+# RUN: llvm-readobj --sections %t.load | FileCheck %s --check-prefixes=CHECK,READ,WRITE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,noload %t %t.noload
+# RUN: llvm-readobj --sections %t.noload | FileCheck %s --check-prefixes=CHECK,READ,WRITE,REMOVE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,readonly %t %t.readonly
+# RUN: llvm-readobj --sections %t.readonly | FileCheck %s --check-prefixes=CHECK,READ
+# RUN: llvm-objcopy --rename-section=.foo=.bar,exclude %t %t.exclude
+# RUN: llvm-readobj --sections %t.exclude | FileCheck %s --check-prefixes=CHECK,READ,WRITE,REMOVE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,debug %t %t.debug
+# RUN: llvm-readobj --sections %t.debug | FileCheck %s --check-prefixes=CHECK,INITDATA,DISCARDABLE,READ,WRITE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,code %t %t.code
+# RUN: llvm-readobj --sections %t.code | FileCheck %s --check-prefixes=CHECK,CODE,EXECUTE,READ,WRITE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,data %t %t.data
+# RUN: llvm-readobj --sections %t.data | FileCheck %s --check-prefixes=CHECK,INITDATA,READ,WRITE
+# RUN: llvm-objcopy --rename-section=.foo=.bar,share %t %t.share
+# RUN: llvm-readobj --sections %t.share | FileCheck %s --check-prefixes=CHECK,READ,SHARED,WRITE
+
+## Multiple flags:
+# RUN: llvm-objcopy --rename-section=.foo=.bar,alloc,readonly,share %t %t.alloc_ro_share
+# RUN: llvm-readobj --sections %t.alloc_ro_share | FileCheck %s --check-prefixes=CHECK,UNINITDATA,READ,SHARED
+# RUN: llvm-objcopy --rename-section=.foo=.bar,alloc,code %t %t.alloc_code
+# RUN: llvm-readobj --sections %t.alloc_code | FileCheck %s --check-prefixes=CHECK,CODE,UNINITDATA,EXECUTE,READ,WRITE
+
+## No flags: preserve original characteristics (contents are never dropped).
+# RUN: llvm-objcopy --rename-section=.foo=.bar %t %t.none
+# RUN: llvm-readobj --sections --section-data %t.none | FileCheck %s --check-prefixes=PRESERVE,PRESERVE-DATA
+
+## Invalid flag:
+# RUN: not llvm-objcopy --rename-section=.foo=.bar,xyzzy %t %t.xyzzy 2>&1 | FileCheck %s --check-prefix=BAD-FLAG
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .foo
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+ Alignment: 4
+ SectionData: 'DEADBEEF'
+symbols:
+...
+
+# CHECK: Name: .bar
+# CHECK: Characteristics [
+# CHECK-NEXT: IMAGE_SCN_ALIGN_4BYTES
+# CODE-NEXT: IMAGE_SCN_CNT_CODE
+# INITDATA-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+# UNINITDATA-NEXT: IMAGE_SCN_CNT_UNINITIALIZED_DATA
+# REMOVE-NEXT: IMAGE_SCN_LNK_REMOVE
+# DISCARDABLE-NEXT: IMAGE_SCN_MEM_DISCARDABLE
+# EXECUTE-NEXT: IMAGE_SCN_MEM_EXECUTE
+# READ-NEXT: IMAGE_SCN_MEM_READ
+# SHARED-NEXT: IMAGE_SCN_MEM_SHARED
+# WRITE-NEXT: IMAGE_SCN_MEM_WRITE
+# CHECK-NEXT: ]
+
+# PRESERVE: Name: .bar
+# PRESERVE: Characteristics [
+# PRESERVE-NEXT: IMAGE_SCN_ALIGN_4BYTES
+# PRESERVE-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+# PRESERVE-NEXT: IMAGE_SCN_MEM_READ
+# PRESERVE-NEXT: ]
+# PRESERVE-DATA: SectionData (
+# PRESERVE-DATA-NEXT: 0000: DEADBEEF
+# PRESERVE-DATA-NEXT: )
+
+# BAD-FLAG: unrecognized section flag 'xyzzy'. Flags supported for GNU compatibility: alloc, load, noload, readonly, exclude, debug, code, data, rom, share, contents, merge, strings
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section-long-name.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section-long-name.test
new file mode 100644
index 0000000000000..c2a4370e45598
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section-long-name.test
@@ -0,0 +1,44 @@
+## Long section names (>8 bytes) go into the COFF string table.
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --rename-section=.text=.longsectionname %t %t2
+# RUN: llvm-readobj --sections --symbols --string-table %t2 | FileCheck %s
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .text
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+ Alignment: 16
+ SectionData: '90'
+symbols:
+ - Name: .text
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 1
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 1
+...
+
+# CHECK: Name: .longsectionname
+# CHECK: Characteristics [
+# CHECK-NEXT: IMAGE_SCN_ALIGN_16BYTES
+# CHECK-NEXT: IMAGE_SCN_CNT_CODE
+# CHECK-NEXT: IMAGE_SCN_MEM_EXECUTE
+# CHECK-NEXT: IMAGE_SCN_MEM_READ
+# CHECK-NEXT: ]
+
+# CHECK: Name: .longsectionname
+# CHECK-NEXT: Value: 0
+# CHECK-NEXT: Section: .longsectionname
+
+# CHECK: StringTable {
+# CHECK: .longsectionname
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section-reloc.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section-reloc.test
new file mode 100644
index 0000000000000..2490519501986
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section-reloc.test
@@ -0,0 +1,64 @@
+## Renaming a section also renames its section-definition symbol. Relocations
+## that target that symbol stay valid (they use symbol indices, not names).
+
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy --rename-section=.text=.text2 %t %t2
+# RUN: llvm-readobj --sections --relocations --symbols %t2 | FileCheck %s
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .text
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+ Alignment: 16
+ SectionData: '9090'
+ - Name: .debug_info
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_DISCARDABLE, IMAGE_SCN_MEM_READ ]
+ Alignment: 1
+ SectionData: '0000000000000000'
+ Relocations:
+ - VirtualAddress: 0
+ SymbolName: .text
+ Type: IMAGE_REL_AMD64_ADDR64
+symbols:
+ - Name: .text
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 2
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 1
+ - Name: .debug_info
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 8
+ NumberOfRelocations: 1
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 2
+...
+
+# CHECK: Name: .text2
+# CHECK: Name: .debug_info
+
+# CHECK: Relocations [
+# CHECK-NEXT: Section (2) .debug_info {
+# CHECK-NEXT: 0x0 IMAGE_REL_AMD64_ADDR64 .text2 (0)
+# CHECK-NEXT: }
+# CHECK-NEXT: ]
+
+# CHECK: Name: .text2
+# CHECK-NEXT: Value: 0
+# CHECK-NEXT: Section: .text2
+# CHECK: Name: .debug_info
diff --git a/llvm/test/tools/llvm-objcopy/COFF/rename-section.test b/llvm/test/tools/llvm-objcopy/COFF/rename-section.test
new file mode 100644
index 0000000000000..7960ea284bc4c
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/COFF/rename-section.test
@@ -0,0 +1,101 @@
+# RUN: yaml2obj %s -o %t
+
+## Basic rename: section header, contents, alignment, and section symbol.
+# RUN: llvm-objcopy --rename-section=.foo=.bar %t %t2
+# RUN: llvm-readobj --sections --section-data --symbols %t2 | FileCheck %s
+
+## Bad format / multiple renames of the same source (CLI, format-agnostic).
+# RUN: not llvm-objcopy --rename-section=.foo.bar --rename-section=.foo=.other %t %t2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=BAD-FORMAT
+# RUN: not llvm-objcopy --rename-section=.foo=.bar --rename-section=.foo=.other %t %t2 2>&1 | \
+# RUN: FileCheck %s --check-prefix=MULTIPLE-RENAMES
+
+## Section renames do not chain in one invocation.
+# RUN: llvm-objcopy --rename-section=.foo=.bar --rename-section=.bar=.baz %t %t3
+# RUN: cmp %t2 %t3
+
+## Renaming a nonexistent section is a silent no-op.
+# RUN: llvm-objcopy --rename-section=.missing=.other %t %t4
+# RUN: cmp %t %t4
+
+## Empty new name is allowed (matches LLVM ELF and COFF --add-section).
+# RUN: llvm-objcopy --rename-section=.foo= %t %t5
+# RUN: llvm-readobj --sections %t5 | FileCheck %s --check-prefix=EMPTY-NAME
+
+## PE executable smoke: rename a section header when there are no symbols.
+# RUN: yaml2obj %p/Inputs/x86_64-exe.yaml -o %t.exe
+# RUN: llvm-objcopy --rename-section=.rdata=.rodata %t.exe %t.exe2
+# RUN: llvm-readobj --sections %t.exe2 | FileCheck %s --check-prefix=PE
+
+--- !COFF
+header:
+ Machine: IMAGE_FILE_MACHINE_AMD64
+ Characteristics: [ ]
+sections:
+ - Name: .foo
+ Characteristics: [ IMAGE_SCN_CNT_INITIALIZED_DATA, IMAGE_SCN_MEM_READ ]
+ Alignment: 4
+ SectionData: 'C3C3C3C3'
+ - Name: .text
+ Characteristics: [ IMAGE_SCN_CNT_CODE, IMAGE_SCN_MEM_EXECUTE, IMAGE_SCN_MEM_READ ]
+ Alignment: 16
+ SectionData: '90'
+symbols:
+ - Name: .foo
+ Value: 0
+ SectionNumber: 1
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 4
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 1
+ - Name: .text
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_NULL
+ StorageClass: IMAGE_SYM_CLASS_STATIC
+ SectionDefinition:
+ Length: 1
+ NumberOfRelocations: 0
+ NumberOfLinenumbers: 0
+ CheckSum: 0
+ Number: 2
+ - Name: main
+ Value: 0
+ SectionNumber: 2
+ SimpleType: IMAGE_SYM_TYPE_NULL
+ ComplexType: IMAGE_SYM_DTYPE_FUNCTION
+ StorageClass: IMAGE_SYM_CLASS_EXTERNAL
+...
+
+# CHECK: Name: .bar
+# CHECK: Characteristics [
+# CHECK-NEXT: IMAGE_SCN_ALIGN_4BYTES
+# CHECK-NEXT: IMAGE_SCN_CNT_INITIALIZED_DATA
+# CHECK-NEXT: IMAGE_SCN_MEM_READ
+# CHECK-NEXT: ]
+# CHECK: SectionData (
+# CHECK-NEXT: 0000: C3C3C3C3
+# CHECK-NEXT: )
+# CHECK: Name: .text
+
+# CHECK: Name: .bar
+# CHECK-NEXT: Value: 0
+# CHECK-NEXT: Section: .bar
+# CHECK: Name: .text
+# CHECK: Name: main
+
+# BAD-FORMAT: bad format for --rename-section: missing '='
+# MULTIPLE-RENAMES: multiple renames of section '.foo'
+
+# EMPTY-NAME: Name: (00 00 00 00 00 00 00 00)
+
+# PE: Name: .text
+# PE: Name: .rodata
+# PE: Name: .data
+# PE: Name: .pdata
|
|
@github-actions I confirm that I have read the policies. |
jh7370
left a comment
There was a problem hiding this comment.
I'm not a COFF expert, so there may well be intricacies that someone like @mstorsjo is better suited to reviewing. I've made a few comments on the tests though.
What's the motivation for adding this functionality? Is it something you need specifically, or is it just "I see a missing feature I could add"?
Also, you noted that you've used AI assistance for this. Could you give me an idea of how much you've used it here? I.e. how much of this PR is AI-generated versus written by yourself?
| # RUN: llvm-objcopy --rename-section=.missing=.other %t %t4 | ||
| # RUN: cmp %t %t4 | ||
|
|
||
| ## Empty new name is allowed (matches LLVM ELF and COFF --add-section). |
There was a problem hiding this comment.
Nit: I don't think we need to call out matching ELF here, since we have the pre-existing COFF behaviour. Just put "(matches --add-section)"
| # RUN: llvm-objcopy --rename-section=.foo= %t %t5 | ||
| # RUN: llvm-readobj --sections %t5 | FileCheck %s --check-prefix=EMPTY-NAME | ||
|
|
||
| ## PE executable smoke: rename a section header when there are no symbols. |
There was a problem hiding this comment.
I'm not sure I understand the significance of this specific test case. Is it that it's a PE file or that there are no symbols? If both, why is it important to test both at the same time? What does the presence (or lack thereof) of symbols impact?
| # CHECK-NEXT: Value: 0 | ||
| # CHECK-NEXT: Section: .longsectionname | ||
|
|
||
| # CHECK: StringTable { |
There was a problem hiding this comment.
Is there anything else in the string table? If not, can we check the entire contents to show there's no garbage in there?
Can we also have a test case that goes from a long to a short name and show that the old name is no longer in the table?
|
|
||
| # RUN: yaml2obj %s -o %t | ||
| # RUN: llvm-objcopy --rename-section=.text=.longsectionname %t %t2 | ||
| # RUN: llvm-readobj --sections --symbols --string-table %t2 | FileCheck %s |
There was a problem hiding this comment.
What is the purpose of dumping the symbols for this test case?
| # CHECK: Name: .text2 | ||
| # CHECK-NEXT: Value: 0 | ||
| # CHECK-NEXT: Section: .text2 | ||
| # CHECK: Name: .debug_info |
There was a problem hiding this comment.
What's the reason for checking that the .debug_info section symbol still exists?
|
|
||
| # CHECK: Name: .bar | ||
| # CHECK: Characteristics [ | ||
| # CHECK-NEXT: IMAGE_SCN_ALIGN_4BYTES |
There was a problem hiding this comment.
Might be worth a comment highlighting that the alignment flag is always preserved.
| @@ -0,0 +1,30 @@ | |||
| ## Renaming to a $ grouping name changes only the logical name. | |||
There was a problem hiding this comment.
I'm not sure I see a benefit to this test. This is testing that we don't do something that we don't ever attempt to do in the code or even something vaguely related. There are an infinite number of cases that you could test if you go down that route.
| # FLAGS-NEXT: IMAGE_SCN_MEM_WRITE | ||
| # FLAGS-NEXT: ] | ||
|
|
||
| ## Conflict: --set-section-flags on rename destination is rejected (CLI). |
There was a problem hiding this comment.
What is the point of "(CLI)" in this comment?
--rename-sectionfor COFF objects.Assisted-by: Cursor Grok 4.5 (the tests)
Apart from lit tests, I have tested it on one project that currently requires GNU ObjCopy.