[llvm] 6861658 - [llvm-objcopy][ELF] Avoid reordering section headers

Igor Kudrin via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 12 03:22:15 PDT 2021


Author: Igor Kudrin
Date: 2021-08-12T17:12:09+07:00
New Revision: 68616584c3a5ce9352b37d24e408b171928f6840

URL: https://github.com/llvm/llvm-project/commit/68616584c3a5ce9352b37d24e408b171928f6840
DIFF: https://github.com/llvm/llvm-project/commit/68616584c3a5ce9352b37d24e408b171928f6840.diff

LOG: [llvm-objcopy][ELF] Avoid reordering section headers

As for now, llvm-objcopy sorts section headers according to the offsets
of the sections in the input file. That can corrupt section references
in the dynamic symbol table because it is a loadable section and as such
is not updated by the tool. Even though the section references are not
required for loading the binary correctly, they are still handy for a
user who analyzes the file.

While the patch removes global reordering of section headers, it layouts
the sections in the same way as before, i.e. according to their original
offsets. All that helps the output file to resemble the input better.

Note that the patch removes sorting SHT_GROUP sections to the start of
the list, which was introduced in D62620 in order to ensure that they
come before the group members, along with the corresponding test. The
original issue was caused by the sorting of section headers, so dropping
the sorting also resolves the issue.

Differential Revision: https://reviews.llvm.org/D107653

Added: 
    llvm/test/tools/llvm-objcopy/ELF/dwarf-fission.test
    llvm/test/tools/llvm-objcopy/ELF/dynsym-valid-refs.test
    llvm/test/tools/llvm-objcopy/ELF/layout-sections-by-original-offsets.test

Modified: 
    llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test
    llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
    llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s
    llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test
    llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test
    llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
    llvm/tools/llvm-objcopy/ELF/Object.cpp
    llvm/tools/llvm-objcopy/ELF/Object.h

Removed: 
    llvm/test/tools/llvm-objcopy/ELF/drawf-fission.test
    llvm/test/tools/llvm-objcopy/ELF/group-reorder.test


################################################################################
diff  --git a/llvm/test/tools/llvm-objcopy/ELF/drawf-fission.test b/llvm/test/tools/llvm-objcopy/ELF/dwarf-fission.test
similarity index 100%
rename from llvm/test/tools/llvm-objcopy/ELF/drawf-fission.test
rename to llvm/test/tools/llvm-objcopy/ELF/dwarf-fission.test
index 76f74ce7e8051..4e8f1605f96a2 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/drawf-fission.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/dwarf-fission.test
@@ -8,36 +8,36 @@
 
 #DWARF:     SectionHeaderCount: 8
 
+#DWARF:     Name: .strtab
 #DWARF:     Name: .debug_loc.dwo
 #DWARF:     Name: .debug_str.dwo
 #DWARF:     Name: .debug_str_offsets.dwo
 #DWARF:     Name: .debug_info.dwo
 #DWARF:     Name: .debug_abbrev.dwo
 #DWARF:     Name: .debug_line.dwo
-#DWARF:     Name: .strtab
 
 #STRIP:     SectionHeaderCount: 24
 
+#STRIP:    Name: .strtab
 #STRIP:    Name: .text
+#STRIP:    Name: .rela.text
 #STRIP:    Name: .rodata.str1.1
 #STRIP:    Name: .debug_str
 #STRIP:    Name: .debug_abbrev
 #STRIP:    Name: .debug_info
+#STRIP:    Name: .rela.debug_info
 #STRIP:    Name: .debug_ranges
 #STRIP:    Name: .debug_macinfo
 #STRIP:    Name: .debug_addr
+#STRIP:    Name: .rela.debug_addr
 #STRIP:    Name: .debug_pubnames
+#STRIP:    Name: .rela.debug_pubnames
 #STRIP:    Name: .debug_pubtypes
+#STRIP:    Name: .rela.debug_pubtypes
 #STRIP:    Name: .comment
 #STRIP:    Name: .note.GNU-stack
 #STRIP:    Name: .debug_frame
-#STRIP:    Name: .debug_line
-#STRIP:    Name: .symtab
-#STRIP:    Name: .rela.text
-#STRIP:    Name: .rela.debug_info
-#STRIP:    Name: .rela.debug_addr
-#STRIP:    Name: .rela.debug_pubnames
-#STRIP:    Name: .rela.debug_pubtypes
 #STRIP:    Name: .rela.debug_frame
+#STRIP:    Name: .debug_line
 #STRIP:    Name: .rela.debug_line
-#STRIP:    Name: .strtab
+#STRIP:    Name: .symtab

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/dynsym-valid-refs.test b/llvm/test/tools/llvm-objcopy/ELF/dynsym-valid-refs.test
new file mode 100644
index 0000000000000..96ede5a970bb6
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/dynsym-valid-refs.test
@@ -0,0 +1,69 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj --dyn-syms %t2 | FileCheck %s
+
+## This checks that section references in the dynamic symbol table are not
+## corrupted after processing a file where the sequence of section headers does
+## not follow the order of their offsets. The tool avoids updating loadable
+## sections, so, the content of the dynamic symbol table stays intact and the
+## used section indices are preserved. In this test, the section 'dummy' comes
+## first while having the offset after all other sections. If the section
+## headers were sorted by their offsets by the tool, what it did before, the
+## index of the '.text' section would change from '2' to '1', which resulted
+## in an incorrect reference in the corresponding dynamic symbol 'foo'.
+
+# CHECK:      Name: foo
+# CHECK-NEXT: Value:
+# CHECK-NEXT: Size:
+# CHECK-NEXT: Binding:
+# CHECK-NEXT: Type:
+# CHECK-NEXT: Other:
+# CHECK-NEXT: Section: .text
+
+!ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+ProgramHeaders:
+  - Type:            PT_LOAD
+    Flags:           [ PF_X, PF_R ]
+    FirstSec:        .text
+    LastSec:         .text
+    Align:           0x1000
+Sections:
+  - Name:            .text
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
+    AddressAlign:    0x4
+    Offset:          0x1000
+    Content:         0000000000000000
+  - Name:            .dynsym
+    Type:            SHT_DYNSYM
+    Flags:           [ SHF_ALLOC ]
+    Link:            .dynstr
+  - Name:            .dynstr
+    Type:            SHT_STRTAB
+    Flags:           [ SHF_ALLOC ]
+  - Name:            dummy
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Offset:          0x1100
+  - Type:            SectionHeaderTable
+    Sections:
+## 'dummy' comes before '.text' in the section header table despite its offset
+## is larger.
+      - Name:            dummy
+      - Name:            .text
+      - Name:            .dynsym
+      - Name:            .dynstr
+      - Name:            .shstrtab
+      - Name:            .strtab
+DynamicSymbols:
+  - Name:            foo
+    Type:            STT_FUNC
+    Section:         .text
+    Binding:         STB_GLOBAL
+    Value:           0x0
+    Size:            0x8

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test b/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test
deleted file mode 100644
index db2f733fbf8a1..0000000000000
--- a/llvm/test/tools/llvm-objcopy/ELF/group-reorder.test
+++ /dev/null
@@ -1,65 +0,0 @@
-# RUN: yaml2obj %s -o %t.o
-# RUN: llvm-objcopy %t.o %t.2.o
-# RUN: llvm-readelf --section-groups --sections %t.o | FileCheck %s --check-prefix=IN
-# RUN: llvm-readelf --section-groups --sections %t.2.o | FileCheck %s --check-prefix=OUT
-
-# In this test, .group gets moved to the beginning. Run readelf -gS on input as
-# well as output to make sure it really moved, as well as to verify that we
-# aren't purely sorting based on offsets (it gets moved to the beginning
-# despite having a larger offset).
-
-# IN:      There are 7 section headers, starting at offset 0x160:
-# IN:        [Nr] Name              Type            Address          Off    Size
-# IN-NEXT:   [ 0]                   NULL            0000000000000000 000000 000000
-# IN-NEXT:   [ 1] .foo              PROGBITS        0000000000000000 000040 000040
-# IN-NEXT:   [ 2] .group            GROUP           0000000000000000 000080 000008
-# IN-NEXT:   [ 3] .bar              PROGBITS        0000000000000000 000088 000040
-
-# IN:      COMDAT group section [    2] `.group' [bar] contains 1 sections:
-# IN-NEXT:    [Index]    Name
-# IN-NEXT:    [    3]   .bar
-
-# OUT:      There are 7 section headers, starting at offset 0x160:
-# OUT:        [Nr] Name              Type            Address          Off    Size
-# OUT-NEXT:   [ 0]                   NULL            0000000000000000 000000 000000
-# OUT-NEXT:   [ 1] .group            GROUP           0000000000000000 000040 000008
-# OUT-NEXT:   [ 2] .foo              PROGBITS        0000000000000000 000048 000040
-# OUT-NEXT:   [ 3] .bar              PROGBITS        0000000000000000 000088 000040
-
-# OUT:      COMDAT group section [    1] `.group' [bar] contains 1 sections:
-# OUT-NEXT:    [Index]    Name
-# OUT-NEXT:    [    3]   .bar
-
---- !ELF
-FileHeader:
-  Class:           ELFCLASS64
-  Data:            ELFDATA2LSB
-  Type:            ET_REL
-  Machine:         EM_X86_64
-Sections:
-  - Name:            .foo
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC, SHF_EXECINSTR ]
-    Size:            64
-  - Name:            .group
-    Type:            SHT_GROUP
-    Link:            .symtab
-    Info:            bar
-    Members:
-      - SectionOrType:   GRP_COMDAT
-      - SectionOrType:   .bar
-  - Name:            .bar
-    Type:            SHT_PROGBITS
-    Flags:           [ SHF_ALLOC, SHF_EXECINSTR, SHF_GROUP ]
-    Size:            64
-Symbols:
-  - Name:            .foo
-    Type:            STT_SECTION
-    Section:         .foo
-  - Name:            .bar
-    Type:            STT_SECTION
-    Section:         .bar
-  - Name:            bar
-    Type:            STT_FUNC
-    Binding:         STB_GLOBAL
-    Section:         .foo

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test b/llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test
index 885ba13d1b3ca..eeccbed9cc9db 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/ihex-reader.test
@@ -4,66 +4,62 @@
 # RUN: llvm-objcopy -I ihex -O elf32-i386 %t.hex %t2
 # RUN: llvm-readobj --section-headers %t2 | FileCheck %s
 
-# CHECK:         Index: 1
-# CHECK-NEXT:    Name: .sec1
+# CHECK:         Name: .sec1
 # CHECK-NEXT:    Type: SHT_PROGBITS (0x1)
 # CHECK-NEXT:    Flags [ (0x3)
 # CHECK-NEXT:      SHF_ALLOC (0x2)
 # CHECK-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 # CHECK-NEXT:    Address: 0x0
-# CHECK-NEXT:    Offset: 0x34
+# CHECK-NEXT:    Offset:
 # CHECK-NEXT:    Size: 21
 # CHECK-NEXT:    Link: 0
 # CHECK-NEXT:    Info: 0
 # CHECK-NEXT:    AddressAlignment: 1
 # CHECK-NEXT:    EntrySize: 0
 
-# CHECK:         Index: 2
-# CHECK-NEXT:    Name: .sec2
+# CHECK:         Name: .sec2
 # CHECK-NEXT:    Type: SHT_PROGBITS (0x1)
 # CHECK-NEXT:    Flags [ (0x3)
 # CHECK-NEXT:      SHF_ALLOC (0x2)
 # CHECK-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 # CHECK-NEXT:    Address: 0xFFF8
-# CHECK-NEXT:    Offset: 0x49
+# CHECK-NEXT:    Offset:
 # CHECK-NEXT:    Size: 11
 # CHECK-NEXT:    Link: 0
 # CHECK-NEXT:    Info: 0
 # CHECK-NEXT:    AddressAlignment: 1
 # CHECK-NEXT:    EntrySize: 0
 
-# CHECK:         Index: 3
-# CHECK-NEXT:    Name: .sec3
+# CHECK:         Name: .sec3
 # CHECK-NEXT:    Type: SHT_PROGBITS (0x1)
 # CHECK-NEXT:    Flags [ (0x3)
 # CHECK-NEXT:      SHF_ALLOC (0x2)
 # CHECK-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 # CHECK-NEXT:    Address: 0x10100
-# CHECK-NEXT:    Offset: 0x54
+# CHECK-NEXT:    Offset:
 # CHECK-NEXT:    Size: 4
 # CHECK-NEXT:    Link: 0
 # CHECK-NEXT:    Info: 0
 # CHECK-NEXT:    AddressAlignment: 1
 # CHECK-NEXT:    EntrySize: 0
 
-# CHECK:         Index: 4
-# CHECK-NEXT:    Name: .sec4
+# CHECK:         Name: .sec4
 # CHECK-NEXT:    Type: SHT_PROGBITS (0x1)
 # CHECK-NEXT:    Flags [ (0x3)
 # CHECK-NEXT:      SHF_ALLOC (0x2)
 # CHECK-NEXT:      SHF_WRITE (0x1)
 # CHECK-NEXT:    ]
 # CHECK-NEXT:    Address: 0x10FFF8
-# CHECK-NEXT:    Offset: 0x58
+# CHECK-NEXT:    Offset:
 # CHECK-NEXT:    Size: 11
 # CHECK-NEXT:    Link: 0
 # CHECK-NEXT:    Info: 0
 # CHECK-NEXT:    AddressAlignment: 1
 # CHECK-NEXT:    EntrySize: 0
-  
+
 ## Check section contents.
 # RUN: llvm-objcopy -O binary --only-section=.text %t %t.text
 # RUN: llvm-objcopy -O binary --only-section=.sec1 %t2 %t2.sec1
@@ -82,37 +78,34 @@
 # RUN: llvm-objcopy -I ihex -O elf32-i386 %p/Inputs/sections.hex %t-raw
 # RUN: llvm-readobj --section-headers %t-raw | FileCheck %s --check-prefix=RAW
 
-# RAW:          Index: 1
-# RAW-NEXT:     Name: .sec1
+# RAW:          Name: .sec1
 # RAW-NEXT:     Type: SHT_PROGBITS (0x1)
 # RAW-NEXT:     Flags [ (0x3)
 # RAW-NEXT:       SHF_ALLOC (0x2)
 # RAW-NEXT:       SHF_WRITE (0x1)
 # RAW-NEXT:     ]
 # RAW-NEXT:     Address: 0x1FFF8
-# RAW-NEXT:     Offset: 0x34
+# RAW-NEXT:     Offset:
 # RAW-NEXT:     Size: 11
 
-# RAW:          Index: 2
-# RAW-NEXT:     Name: .sec2
+# RAW:          Name: .sec2
 # RAW-NEXT:     Type: SHT_PROGBITS (0x1)
 # RAW-NEXT:     Flags [ (0x3)
 # RAW-NEXT:       SHF_ALLOC (0x2)
 # RAW-NEXT:       SHF_WRITE (0x1)
 # RAW-NEXT:     ]
 # RAW-NEXT:     Address: 0xFFFF8
-# RAW-NEXT:     Offset: 0x3F
+# RAW-NEXT:     Offset:
 # RAW-NEXT:     Size: 11
 
-# RAW:          Index: 3
-# RAW-NEXT:     Name: .sec3
+# RAW:          Name: .sec3
 # RAW-NEXT:     Type: SHT_PROGBITS (0x1)
 # RAW-NEXT:     Flags [ (0x3)
 # RAW-NEXT:       SHF_ALLOC (0x2)
 # RAW-NEXT:       SHF_WRITE (0x1)
 # RAW-NEXT:     ]
 # RAW-NEXT:     Address: 0x1FFFF8
-# RAW-NEXT:     Offset: 0x4A
+# RAW-NEXT:     Offset:
 # RAW-NEXT:     Size: 11
 
 ## Check section contents.

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/layout-sections-by-original-offsets.test b/llvm/test/tools/llvm-objcopy/ELF/layout-sections-by-original-offsets.test
new file mode 100644
index 0000000000000..1a22f62e97c74
--- /dev/null
+++ b/llvm/test/tools/llvm-objcopy/ELF/layout-sections-by-original-offsets.test
@@ -0,0 +1,65 @@
+# RUN: yaml2obj %s -o %t
+# RUN: llvm-objcopy %t %t2
+# RUN: llvm-readobj --sections %t2 | FileCheck %s
+
+## This checks that llvm-objcopy layouts sections in the order of their offsets
+## in the input file in spite of the order of section headers is 
diff erent.
+
+# CHECK:      Index: 1
+# CHECK-NEXT: Name: foo
+# CHECK-NEXT: Type:
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Offset: 0x50
+# CHECK:      Index: 2
+# CHECK-NEXT: Name: bar
+# CHECK-NEXT: Type: SHT_PROGBITS
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Offset: 0x40
+# CHECK:      Index: 3
+# CHECK-NEXT: Name: baz
+# CHECK-NEXT: Type: SHT_PROGBITS
+# CHECK-NEXT: Flags [
+# CHECK-NEXT:   SHF_ALLOC
+# CHECK-NEXT: ]
+# CHECK-NEXT: Address:
+# CHECK-NEXT: Offset: 0x48
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_REL
+  Machine:         EM_X86_64
+Sections:
+  - Name:            bar
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x4
+    Offset:          0x40
+    Content:         0000000000000000
+  - Name:            baz
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x4
+    Offset:          0x48
+    Content:         0000000000000000
+  - Name:            foo
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    AddressAlign:    0x4
+    Offset:          0x50
+    Content:         0000000000000000
+  - Type:            SectionHeaderTable
+    Sections:
+## Note: the order of section headers 
diff ers from their layout.
+      - Name:            foo
+      - Name:            bar
+      - Name:            baz
+      - Name:            .shstrtab
+      - Name:            .strtab

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test b/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
index 3c41afefb6440..88b937120ace1 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/only-keep-debug.test
@@ -318,3 +318,77 @@ ProgramHeaders:
     Flags:    []
     FileSize: 0x01000
     MemSize:  0x01000
+
+## Check that sections are placed correctly in a case when their order in the
+## section header table is 
diff erent from their layout.
+# RUN: yaml2obj --docnum=6 %s -o %t6
+# RUN: llvm-objcopy --only-keep-debug %t6 %t6.dbg
+# RUN: llvm-readelf -S -l %t6.dbg | FileCheck --check-prefix=CHECK6 %s
+
+# CHECK6:      [Nr] Name        Type     Address          Off    Size   ES Flg Lk Inf Al
+# CHECK6:      [ 1] foo         NOBITS   0000000000000008 001000 000008 00   A  0   0  4
+# CHECK6-NEXT: [ 2] bar         NOBITS   0000000000000000 001000 000008 00   A  0   0  4
+# CHECK6-NEXT: [ 3] baz         NOTE     0000000000000018 001008 000008 00   A  0   0  0
+# CHECK6-NEXT: [ 4] qux         NOTE     0000000000000010 001000 000008 00   A  0   0  0
+
+# CHECK6:      Type Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
+# CHECK6-NEXT: LOAD 0x001000 0x0000000000000000 0x0000000000000000 0x000000 0x000010 R   0x1000
+# CHECK6-NEXT: LOAD 0x001000 0x0000000000000010 0x0000000000000010 0x000010 0x000010 R   0x1
+# CHECK6-EMPTY:
+
+--- !ELF
+FileHeader:
+  Class:           ELFCLASS64
+  Data:            ELFDATA2LSB
+  Type:            ET_DYN
+  Machine:         EM_X86_64
+ProgramHeaders:
+  - Type:     PT_LOAD
+    Flags:    [ PF_R ]
+    Offset:   0x1000
+    VAddr:    0x0
+    Align:    0x1000
+    FileSize: 0x10
+    MemSize:  0x10
+  - Type:     PT_LOAD
+    Flags:    [ PF_R ]
+    Offset:   0x1010
+    VAddr:    0x10
+    FileSize: 0x10
+    MemSize:  0x10
+Sections:
+  - Name:            bar
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x0
+    AddressAlign:    0x4
+    Offset:          0x1000
+    Content:         0000000000000000
+  - Name:            foo
+    Type:            SHT_PROGBITS
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x8
+    AddressAlign:    0x4
+    Offset:          0x1008
+    Content:         0000000000000000
+  - Name:            qux
+    Type:            SHT_NOTE
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x10
+    Offset:          0x1010
+    Content:         0000000000000000
+  - Name:            baz
+    Type:            SHT_NOTE
+    Flags:           [ SHF_ALLOC ]
+    Address:         0x18
+    Offset:          0x1018
+    Content:         0000000000000000
+  - Type:            SectionHeaderTable
+    Sections:
+## Note: the order of section headers 
diff ers from their layout.
+      - Name:            foo
+      - Name:            bar
+      - Name:            baz
+      - Name:            qux
+      - Name:            .shstrtab
+      - Name:            .strtab

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s b/llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s
index b692d7691772f..e46da5483a6a3 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s
+++ b/llvm/test/tools/llvm-objcopy/ELF/shared-strtab-shstrtab.s
@@ -20,8 +20,8 @@
 # RUN:   | FileCheck %s --check-prefix=BASIC --implicit-check-not=.shstrtab
 
 # BASIC: Sections [
-# BASIC:   Name: .foo (
 # BASIC:   Name: .strtab (
+# BASIC:   Name: .foo (
 # BASIC: Symbols [
 # BASIC:   Name: foo (
 
@@ -31,8 +31,8 @@
 # RUN:   | FileCheck %s --check-prefix=SECTION-RENAME --implicit-check-not=.shstrtab
 
 # SECTION-RENAME: Sections [
-# SECTION-RENAME:   Name: .oof (
 # SECTION-RENAME:   Name: .strtab (
+# SECTION-RENAME:   Name: .oof (
 # SECTION-RENAME: Symbols [
 # SECTION-RENAME:   Name: foo (
 
@@ -42,8 +42,8 @@
 # RUN:   | FileCheck %s --check-prefix=SYMBOL-RENAME --implicit-check-not=.shstrtab
 
 # SYMBOL-RENAME: Sections [
-# SYMBOL-RENAME:   Name: .foo (
 # SYMBOL-RENAME:   Name: .strtab (
+# SYMBOL-RENAME:   Name: .foo (
 # SYMBOL-RENAME: Symbols [
 # SYMBOL-RENAME:   Name: oof (
 
@@ -63,8 +63,8 @@
 # RUN:   | FileCheck %s --check-prefix=SYMBOL-REMOVE --implicit-check-not=.shstrtab --implicit-check-not="Name: foo"
 
 # SYMBOL-REMOVE: Sections [
-# SYMBOL-REMOVE:   Name: .foo (
 # SYMBOL-REMOVE:   Name: .strtab (
+# SYMBOL-REMOVE:   Name: .foo (
 # SYMBOL-REMOVE: Symbols [
 
 ## Case 6: adding a section.
@@ -73,8 +73,8 @@
 # RUN:   | FileCheck %s --check-prefix=SECTION-ADD --implicit-check-not=.shstrtab
 
 # SECTION-ADD: Sections [
-# SECTION-ADD:   Name: .foo (
 # SECTION-ADD:   Name: .strtab (
+# SECTION-ADD:   Name: .foo (
 # SECTION-ADD:   Name: .bar (
 # SECTION-ADD: Symbols [
 # SECTION-ADD:   Name: foo (
@@ -85,8 +85,8 @@
 # RUN:   | FileCheck %s --check-prefix=SYMBOL-ADD --implicit-check-not=.shstrtab
 
 # SYMBOL-ADD: Sections [
-# SYMBOL-ADD:   Name: .foo (
 # SYMBOL-ADD:   Name: .strtab (
+# SYMBOL-ADD:   Name: .foo (
 # SYMBOL-ADD: Symbols [
 # SYMBOL-ADD:   Name: foo (
 # SYMBOL-ADD:   Name: bar (
@@ -97,8 +97,8 @@
 # RUN:   | FileCheck %s --check-prefix=STRIP-ALL --implicit-check-not=.shstrtab
 
 # STRIP-ALL:      Sections [
-# STRIP-ALL:        Name: .foo (
 # STRIP-ALL:        Name: .strtab (
+# STRIP-ALL:        Name: .foo (
 # STRIP-ALL:      Symbols [
 # STRIP-ALL-NEXT: ]
 

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test
index 93c35117fa8f8..a4a867ad80374 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-groups.test
@@ -1,40 +1,38 @@
 # RUN: yaml2obj %s -o %t
 # RUN: llvm-objcopy --strip-dwo %t
-# RUN: llvm-readobj --symbols -S --section-groups %t | FileCheck %s
+# RUN: llvm-readobj --symbols -S --section-groups %t | \
+# RUN:   FileCheck %s --implicit-check-not=.debug_
 
-## `llvm-objcopy --strip-dwo` strips out dwo sections, as a result, the index of 
+## `llvm-objcopy --strip-dwo` strips out dwo sections, as a result, the index of
 ## the symbol table, the indices of the symbols and the indices of the sections
-## which go after the removed ones will change. Consequently, the fields 
-## Link, Info and the content of .group need to be updated.
+## which go after the removed ones will change. Consequently, the fields Link,
+## Info and the content of .group need to be updated.
 
-# CHECK-NOT: .debug_
+## Note. 'Signature' is generated using 'Link' and 'Info', so checking it
+## validates updating these fields.
 
 # CHECK:      Groups {
 # CHECK-NEXT:  Group {
-# CHECK-NEXT:   Name: .group (1)
-# CHECK-NEXT:   Index: 1{{$}}
-# CHECK-NEXT:   Link: 6
-# CHECK-NEXT:   Info: 2
-# CHECK-NEXT:   Type: COMDAT (0x1)
+# CHECK-NEXT:   Name: .group
+# CHECK-NEXT:   Index:
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   Type:
 # CHECK-NEXT:   Signature: group1
 # CHECK-NEXT:   Section(s) in group [
-# CHECK-NEXT:     .text.group1 (3)
+# CHECK-NEXT:     .text.group1
 # CHECK-NEXT:   ]
 
-# CHECK-NOT: .debug_
-
-# CHECK:      Name: .group (1)
-# CHECK-NEXT: Index: 2{{$}}
-# CHECK-NEXT: Link: 6
-# CHECK-NEXT: Info: 1
-# CHECK-NEXT: Type: COMDAT (0x1)
-# CHECK-NEXT: Signature: group2
-# CHECK-NEXT: Section(s) in group [
-# CHECK-NEXT:   .text.group2 (4)
-# CHECK-NEXT:   .rela.text.group2 (5)
-# CHECK-NEXT: ]
-
-# CHECK-NOT: .debug_
+# CHECK:        Name: .group
+# CHECK-NEXT:   Index:
+# CHECK-NEXT:   Link:
+# CHECK-NEXT:   Info:
+# CHECK-NEXT:   Type:
+# CHECK-NEXT:   Signature: group2
+# CHECK-NEXT:   Section(s) in group [
+# CHECK-NEXT:     .text.group2
+# CHECK-NEXT:     .rela.text.group2
+# CHECK-NEXT:   ]
 
 --- !ELF
 FileHeader:

diff  --git a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test
index fc2d6c34280af..34708e6350d88 100644
--- a/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test
+++ b/llvm/test/tools/llvm-objcopy/ELF/strip-dwo-inplace.test
@@ -4,26 +4,26 @@
 
 CHECK:     SectionHeaderCount: 24
 
+CHECK:    Name: .strtab
 CHECK:    Name: .text
+CHECK:    Name: .rela.text
 CHECK:    Name: .rodata.str1.1
 CHECK:    Name: .debug_str
 CHECK:    Name: .debug_abbrev
 CHECK:    Name: .debug_info
+CHECK:    Name: .rela.debug_info
 CHECK:    Name: .debug_ranges
 CHECK:    Name: .debug_macinfo
 CHECK:    Name: .debug_addr
+CHECK:    Name: .rela.debug_addr
 CHECK:    Name: .debug_pubnames
+CHECK:    Name: .rela.debug_pubnames
 CHECK:    Name: .debug_pubtypes
+CHECK:    Name: .rela.debug_pubtypes
 CHECK:    Name: .comment
 CHECK:    Name: .note.GNU-stack
 CHECK:    Name: .debug_frame
-CHECK:    Name: .debug_line
-CHECK:    Name: .symtab
-CHECK:    Name: .rela.text
-CHECK:    Name: .rela.debug_info
-CHECK:    Name: .rela.debug_addr
-CHECK:    Name: .rela.debug_pubnames
-CHECK:    Name: .rela.debug_pubtypes
 CHECK:    Name: .rela.debug_frame
+CHECK:    Name: .debug_line
 CHECK:    Name: .rela.debug_line
-CHECK:    Name: .strtab
+CHECK:    Name: .symtab

diff  --git a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
index 986eeca6256c3..946191f8764d9 100644
--- a/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/ELFObjcopy.cpp
@@ -204,8 +204,7 @@ static bool isCompressable(const SectionBase &Sec) {
 }
 
 static Error replaceDebugSections(
-    Object &Obj, SectionPred &RemovePred,
-    function_ref<bool(const SectionBase &)> ShouldReplace,
+    Object &Obj, function_ref<bool(const SectionBase &)> ShouldReplace,
     function_ref<Expected<SectionBase *>(const SectionBase *)> AddSection) {
   // Build a list of the debug sections we are going to replace.
   // We can't call `AddSection` while iterating over sections,
@@ -225,17 +224,7 @@ static Error replaceDebugSections(
     FromTo[S] = *NewSection;
   }
 
-  // Now we want to update the target sections of relocation
-  // sections. Also we will update the relocations themselves
-  // to update the symbol references.
-  for (auto &Sec : Obj.sections())
-    Sec.replaceSectionReferences(FromTo);
-
-  RemovePred = [ShouldReplace, RemovePred](const SectionBase &Sec) {
-    return ShouldReplace(Sec) || RemovePred(Sec);
-  };
-
-  return Error::success();
+  return Obj.replaceSections(FromTo);
 }
 
 static bool isUnneededSymbol(const Symbol &Sym) {
@@ -474,9 +463,12 @@ static Error replaceAndRemoveSections(const CommonConfig &Config, Object &Obj) {
     };
   }
 
+  if (Error E = Obj.removeSections(Config.AllowBrokenLinks, RemovePred))
+    return E;
+
   if (Config.CompressionType != DebugCompressionType::None) {
     if (Error Err = replaceDebugSections(
-            Obj, RemovePred, isCompressable,
+            Obj, isCompressable,
             [&Config, &Obj](const SectionBase *S) -> Expected<SectionBase *> {
               Expected<CompressedSection> NewSection =
                   CompressedSection::create(*S, Config.CompressionType);
@@ -488,7 +480,7 @@ static Error replaceAndRemoveSections(const CommonConfig &Config, Object &Obj) {
       return Err;
   } else if (Config.DecompressDebugSections) {
     if (Error Err = replaceDebugSections(
-            Obj, RemovePred,
+            Obj,
             [](const SectionBase &S) { return isa<CompressedSection>(&S); },
             [&Obj](const SectionBase *S) {
               const CompressedSection *CS = cast<CompressedSection>(S);
@@ -497,7 +489,7 @@ static Error replaceAndRemoveSections(const CommonConfig &Config, Object &Obj) {
       return Err;
   }
 
-  return Obj.removeSections(Config.AllowBrokenLinks, RemovePred);
+  return Error::success();
 }
 
 // Add symbol to the Object symbol table with the specified properties.

diff  --git a/llvm/tools/llvm-objcopy/ELF/Object.cpp b/llvm/tools/llvm-objcopy/ELF/Object.cpp
index 6b632ae929f00..fce6a1365b9d4 100644
--- a/llvm/tools/llvm-objcopy/ELF/Object.cpp
+++ b/llvm/tools/llvm-objcopy/ELF/Object.cpp
@@ -1343,10 +1343,10 @@ void IHexELFBuilder::addDataSections() {
         continue;
       RecAddr = R.Addr + SegmentAddr + BaseAddr;
       if (!Section || Section->Addr + Section->Size != RecAddr) {
-        // OriginalOffset field is only used to sort section properly, so
-        // instead of keeping track of real offset in IHEX file, and as
-        // Object::sortSections() uses llvm::stable_sort(), we can just set to a
-        // constant (zero).
+        // OriginalOffset field is only used to sort sections before layout, so
+        // instead of keeping track of real offsets in IHEX file, and as
+        // layoutSections() and layoutSectionsForOnlyKeepDebug() use
+        // llvm::stable_sort(), we can just set it to a constant (zero).
         Section = &Obj->addSection<OwnedDataSection>(
             ".sec" + std::to_string(SecNo), RecAddr,
             ELF::SHF_ALLOC | ELF::SHF_WRITE, 0);
@@ -2165,6 +2165,30 @@ Error Object::removeSections(
   return Error::success();
 }
 
+Error Object::replaceSections(
+    const DenseMap<SectionBase *, SectionBase *> &FromTo) {
+  auto SectionIndexLess = [](const SecPtr &Lhs, const SecPtr &Rhs) {
+    return Lhs->Index < Rhs->Index;
+  };
+  assert(llvm::is_sorted(Sections, SectionIndexLess) &&
+         "Sections are expected to be sorted by Index");
+  // Set indices of new sections so that they can be later sorted into positions
+  // of removed ones.
+  for (auto &I : FromTo)
+    I.second->Index = I.first->Index;
+
+  // Notify all sections about the replacement.
+  for (auto &Sec : Sections)
+    Sec->replaceSectionReferences(FromTo);
+
+  if (Error E = removeSections(
+          /*AllowBrokenLinks=*/false,
+          [=](const SectionBase &Sec) { return FromTo.count(&Sec) > 0; }))
+    return E;
+  llvm::sort(Sections, SectionIndexLess);
+  return Error::success();
+}
+
 Error Object::removeSymbols(function_ref<bool(const Symbol &)> ToRemove) {
   if (SymbolTable)
     for (const SecPtr &Sec : Sections)
@@ -2203,20 +2227,6 @@ Error Object::addNewSymbolTable() {
   return Error::success();
 }
 
-void Object::sortSections() {
-  // Use stable_sort to maintain the original ordering as closely as possible.
-  llvm::stable_sort(Sections, [](const SecPtr &A, const SecPtr &B) {
-    // Put SHT_GROUP sections first, since group section headers must come
-    // before the sections they contain. This also matches what GNU objcopy
-    // does.
-    if (A->Type != B->Type &&
-        (A->Type == ELF::SHT_GROUP || B->Type == ELF::SHT_GROUP))
-      return A->Type == ELF::SHT_GROUP;
-    // For all other sections, sort by offset order.
-    return A->OriginalOffset < B->OriginalOffset;
-  });
-}
-
 // Orders segments such that if x = y->ParentSegment then y comes before x.
 static void orderSegments(std::vector<Segment *> &Segments) {
   llvm::stable_sort(Segments, compareSegmentsByOffset);
@@ -2265,6 +2275,9 @@ static uint64_t layoutSections(Range Sections, uint64_t Offset) {
   // the offset from the start of the segment. Using the offset from the start
   // of the segment we can assign a new offset to the section. For sections not
   // covered by segments we can just bump Offset to the next valid location.
+  // While it is not necessary, layout the sections in the order based on their
+  // original offsets to resemble the input file as close as possible.
+  std::vector<SectionBase *> OutOfSegmentSections;
   uint32_t Index = 1;
   for (auto &Sec : Sections) {
     Sec.Index = Index++;
@@ -2272,12 +2285,19 @@ static uint64_t layoutSections(Range Sections, uint64_t Offset) {
       auto Segment = *Sec.ParentSegment;
       Sec.Offset =
           Segment.Offset + (Sec.OriginalOffset - Segment.OriginalOffset);
-    } else {
-      Offset = alignTo(Offset, Sec.Align == 0 ? 1 : Sec.Align);
-      Sec.Offset = Offset;
-      if (Sec.Type != SHT_NOBITS)
-        Offset += Sec.Size;
-    }
+    } else
+      OutOfSegmentSections.push_back(&Sec);
+  }
+
+  llvm::stable_sort(OutOfSegmentSections,
+                    [](const SectionBase *Lhs, const SectionBase *Rhs) {
+                      return Lhs->OriginalOffset < Rhs->OriginalOffset;
+                    });
+  for (auto *Sec : OutOfSegmentSections) {
+    Offset = alignTo(Offset, Sec->Align == 0 ? 1 : Sec->Align);
+    Sec->Offset = Offset;
+    if (Sec->Type != SHT_NOBITS)
+      Offset += Sec->Size;
   }
   return Offset;
 }
@@ -2285,38 +2305,49 @@ static uint64_t layoutSections(Range Sections, uint64_t Offset) {
 // Rewrite sh_offset after some sections are changed to SHT_NOBITS and thus
 // occupy no space in the file.
 static uint64_t layoutSectionsForOnlyKeepDebug(Object &Obj, uint64_t Off) {
+  // The layout algorithm requires the sections to be handled in the order of
+  // their offsets in the input file, at least inside segments.
+  std::vector<SectionBase *> Sections;
+  Sections.reserve(Obj.sections().size());
   uint32_t Index = 1;
   for (auto &Sec : Obj.sections()) {
     Sec.Index = Index++;
-
-    auto *FirstSec = Sec.ParentSegment && Sec.ParentSegment->Type == PT_LOAD
-                         ? Sec.ParentSegment->firstSection()
+    Sections.push_back(&Sec);
+  }
+  llvm::stable_sort(Sections,
+                    [](const SectionBase *Lhs, const SectionBase *Rhs) {
+                      return Lhs->OriginalOffset < Rhs->OriginalOffset;
+                    });
+
+  for (auto *Sec : Sections) {
+    auto *FirstSec = Sec->ParentSegment && Sec->ParentSegment->Type == PT_LOAD
+                         ? Sec->ParentSegment->firstSection()
                          : nullptr;
 
     // The first section in a PT_LOAD has to have congruent offset and address
     // modulo the alignment, which usually equals the maximum page size.
-    if (FirstSec && FirstSec == &Sec)
-      Off = alignTo(Off, Sec.ParentSegment->Align, Sec.Addr);
+    if (FirstSec && FirstSec == Sec)
+      Off = alignTo(Off, Sec->ParentSegment->Align, Sec->Addr);
 
     // sh_offset is not significant for SHT_NOBITS sections, but the congruence
     // rule must be followed if it is the first section in a PT_LOAD. Do not
     // advance Off.
-    if (Sec.Type == SHT_NOBITS) {
-      Sec.Offset = Off;
+    if (Sec->Type == SHT_NOBITS) {
+      Sec->Offset = Off;
       continue;
     }
 
     if (!FirstSec) {
       // FirstSec being nullptr generally means that Sec does not have the
       // SHF_ALLOC flag.
-      Off = Sec.Align ? alignTo(Off, Sec.Align) : Off;
-    } else if (FirstSec != &Sec) {
+      Off = Sec->Align ? alignTo(Off, Sec->Align) : Off;
+    } else if (FirstSec != Sec) {
       // The offset is relative to the first section in the PT_LOAD segment. Use
       // sh_offset for non-SHF_ALLOC sections.
-      Off = Sec.OriginalOffset - FirstSec->OriginalOffset + FirstSec->Offset;
+      Off = Sec->OriginalOffset - FirstSec->OriginalOffset + FirstSec->Offset;
     }
-    Sec.Offset = Off;
-    Off += Sec.Size;
+    Sec->Offset = Off;
+    Off += Sec->Size;
   }
   return Off;
 }
@@ -2463,7 +2494,6 @@ template <class ELFT> Error ELFWriter<ELFT>::finalize() {
 
   if (Error E = removeUnneededSections(Obj))
     return E;
-  Obj.sortSections();
 
   // We need to assign indexes before we perform layout because we need to know
   // if we need large indexes or not. We can assign indexes first and check as

diff  --git a/llvm/tools/llvm-objcopy/ELF/Object.h b/llvm/tools/llvm-objcopy/ELF/Object.h
index d03eb4d682873..95a2ccd817255 100644
--- a/llvm/tools/llvm-objcopy/ELF/Object.h
+++ b/llvm/tools/llvm-objcopy/ELF/Object.h
@@ -1050,7 +1050,6 @@ class Object {
   SymbolTableSection *SymbolTable = nullptr;
   SectionIndexSection *SectionIndexTable = nullptr;
 
-  void sortSections();
   SectionTableRef sections() const { return SectionTableRef(Sections); }
   iterator_range<
       filter_iterator<pointee_iterator<std::vector<SecPtr>::const_iterator>,
@@ -1070,6 +1069,7 @@ class Object {
 
   Error removeSections(bool AllowBrokenLinks,
                        std::function<bool(const SectionBase &)> ToRemove);
+  Error replaceSections(const DenseMap<SectionBase *, SectionBase *> &FromTo);
   Error removeSymbols(function_ref<bool(const Symbol &)> ToRemove);
   template <class T, class... Ts> T &addSection(Ts &&... Args) {
     auto Sec = std::make_unique<T>(std::forward<Ts>(Args)...);


        


More information about the llvm-commits mailing list