[llvm-readobj][ELF] Alter JSON/LLVM output on note sections to allow for multiple notes per section in JSON #96813
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…for multiple notes per section in JSON
It turns out that the notes section for corefiles (or really any elf
file with multiple notes) is set up in such a way for LLVM formatted
output that the JSON equivalent only has the last note since the notes
are held in a dictionary with every key being Note. This pr alters the
layout for the notes to a list of dictionaries to sidestep this issue
for JSON output. Prior to this pr a note section in the output looked
like (for LLVM output):
```
Notes [
NoteSection {
Name: <?>
Offset: 0x2148
Size: 0x1F864
Note {
Owner: CORE
Data size: 0x150
Type: NT_PRSTATUS (prstatus structure)
Description data (
0000: 06000000 00000000 00000000 06000000 |................|
...
)
}
Note {
Owner: CORE
Data size: 0x88
Type: NT_PRPSINFO (prpsinfo structure)
Description data (
0000: 02440000 00000000 04054040 00000000 |.D........@@....|
....
```
But is now:
```
Notes [
NoteSection {
Name: <?>
Offset: 0x2148
Size: 0x1F864
Notes [
{
Owner: CORE
Data size: 0x150
Type: NT_PRSTATUS (prstatus structure)
Description data (
0000: 06000000 00000000 00000000 06000000 |................|
...
)
}
{
Owner: CORE
Data size: 0x88
Type: NT_PRPSINFO (prpsinfo structure)
Description data (
0000: 02440000 00000000 04054040 00000000 |.D........@@....|
...
```
|
@llvm/pr-subscribers-lld @llvm/pr-subscribers-lld-elf Author: Fred Grim (feg208) ChangesIt turns out that the notes section for corefiles (or really any elf file with multiple notes) is set up in such a way for LLVM formatted output that the JSON equivalent only has the last note since the notes are held in a dictionary with every key being Note. This pr alters the layout for the notes to a list of dictionaries to sidestep this issue for JSON output. Prior to this pr a note section in the output looked like (for LLVM output): But is now: Patch is 54.91 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/96813.diff 24 Files Affected:
diff --git a/lld/test/ELF/gnu-property-align-32.s b/lld/test/ELF/gnu-property-align-32.s
index 8022a49d34c6c..a6f2bdb5518ad 100644
--- a/lld/test/ELF/gnu-property-align-32.s
+++ b/lld/test/ELF/gnu-property-align-32.s
@@ -17,7 +17,9 @@
# CHECK-NEXT: Info: 0
# CHECK-NEXT: AddressAlignment: 4
-# CHECK: Note {
+# CHECK: Size: 0x1C
+# CHECK-NEXT: Notes [
+# CHECK-NEXT: {
# CHECK-NEXT: Owner: GNU
# CHECK-NEXT: Data size: 0xC
# CHECK-NEXT: Type: NT_GNU_PROPERTY_TYPE_0 (property note)
diff --git a/lld/test/ELF/gnu-property-align.s b/lld/test/ELF/gnu-property-align.s
index b109c09039a2c..8c89f44f37f3a 100644
--- a/lld/test/ELF/gnu-property-align.s
+++ b/lld/test/ELF/gnu-property-align.s
@@ -17,7 +17,9 @@
# CHECK-NEXT: Info: 0
# CHECK-NEXT: AddressAlignment: 8
-# CHECK: Note {
+# CHECK: Size: 0x20
+# CHECK-NEXT: Notes [
+# CHECK-NEXT: {
# CHECK-NEXT: Owner: GNU
# CHECK-NEXT: Data size: 0x10
# CHECK-NEXT: Type: NT_GNU_PROPERTY_TYPE_0 (property note)
diff --git a/lld/test/ELF/partition-notes.s b/lld/test/ELF/partition-notes.s
index c5ade3a47e052..37b6cde62539d 100644
--- a/lld/test/ELF/partition-notes.s
+++ b/lld/test/ELF/partition-notes.s
@@ -20,7 +20,8 @@
// CHECK-NEXT: Name: .note.obj
// CHECK-NEXT: Offset: 0x{{0*}}[[NOTE_OFFSET]]
// CHECK-NEXT: Size:
-// CHECK-NEXT: Note {
+// CHECK-NEXT: Notes [
+// CHECK-NEXT: {
// CHECK-NEXT: Owner: foo
// CHECK-NEXT: Data size: 0x4
// CHECK-NEXT: Type: NT_VERSION (version)
@@ -28,17 +29,20 @@
// CHECK-NEXT: 0000: 62617200 |bar.|
// CHECK-NEXT: )
// CHECK-NEXT: }
+// CHECK-NEXT: ]
// CHECK-NEXT: }
// CHECK-NEXT: NoteSection {
// CHECK-NEXT: Name: .note.gnu.build-id
// CHECK-NEXT: Offset:
// CHECK-NEXT: Size:
-// CHECK-NEXT: Note {
+// CHECK-NEXT: Notes [
+// CHECK-NEXT: {
// CHECK-NEXT: Owner: GNU
// CHECK-NEXT: Data size:
// CHECK-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
// CHECK-NEXT: Build ID: d5101cb9d03b7e836ba9b64f5768a0b31980920f{{$}}
// CHECK-NEXT: }
+// CHECK-NEXT: ]
// CHECK-NEXT: }
// CHECK-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test b/llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test
index 2f131a6cb347b..a78b8b318ec17 100644
--- a/llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test
+++ b/llvm/test/tools/llvm-readobj/ELF/gnu-note-size.test
@@ -16,7 +16,8 @@
# LLVM-NEXT: Name: .note.ABI-tag
# LLVM-NEXT: Offset:
# LLVM-NEXT: Size: 0x14
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: GNU
# LLVM-NEXT: Data size: 0x4
# LLVM-NEXT: Type: NT_GNU_ABI_TAG (ABI version tag)
@@ -25,6 +26,7 @@
# LLVM-NEXT: 0000: 00000000 |....|
# LLVM-NEXT: )
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/gnu-notes.test b/llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
index e73238e6093a8..739cdd1da76c4 100644
--- a/llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
+++ b/llvm/test/tools/llvm-readobj/ELF/gnu-notes.test
@@ -33,41 +33,48 @@
# LLVM-NEXT: Name: .note.ABI-tag
# LLVM-NEXT: Offset: 0x78
# LLVM-NEXT: Size: 0x20
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: GNU
# LLVM-NEXT: Data size: 0x10
# LLVM-NEXT: Type: NT_GNU_ABI_TAG (ABI version tag)
# LLVM-NEXT: OS: Linux
# LLVM-NEXT: ABI: 2.6.32
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.gnu.build-id
# LLVM-NEXT: Offset: 0x98
# LLVM-NEXT: Size: 0x20
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: GNU
# LLVM-NEXT: Data size: 0x10
# LLVM-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
# LLVM-NEXT: Build ID: 4fcb712aa6387724a9f465a32cd8c14b
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.gnu.gold-version
# LLVM-NEXT: Offset: 0xB8
# LLVM-NEXT: Size: 0x1C
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: GNU
# LLVM-NEXT: Data size: 0x9
# LLVM-NEXT: Type: NT_GNU_GOLD_VERSION (gold version)
# LLVM-NEXT: Version: gold 1.11
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.gnu.unknown
# LLVM-NEXT: Offset: 0xD4
# LLVM-NEXT: Size: 0x14
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: GNU
# LLVM-NEXT: Data size: 0x4
# LLVM-NEXT: Type: Unknown (0x0000abcd)
@@ -75,6 +82,7 @@
# LLVM-NEXT: 0000: 61626364 |abcd|
# LLVM-NEXT: )
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: ]
@@ -84,12 +92,14 @@
# LLVM-STRIPPED-NEXT: Name: <?>
# LLVM-STRIPPED-NEXT: Offset: 0x78
# LLVM-STRIPPED-NEXT: Size: 0x20
-# LLVM-STRIPPED-NEXT: Note {
+# LLVM-STRIPPED-NEXT: Notes [
+# LLVM-STRIPPED-NEXT: {
# LLVM-STRIPPED-NEXT: Owner: GNU
# LLVM-STRIPPED-NEXT: Data size: 0x10
# LLVM-STRIPPED-NEXT: Type: NT_GNU_BUILD_ID (unique build ID bitstring)
# LLVM-STRIPPED-NEXT: Build ID: 4fcb712aa6387724a9f465a32cd8c14b
# LLVM-STRIPPED-NEXT: }
+# LLVM-STRIPPED-NEXT: ]
# LLVM-STRIPPED-NEXT: }
# LLVM-STRIPPED-NEXT: ]
@@ -149,7 +159,9 @@ ProgramHeaders:
# ERR1-LLVM-NEXT: Name: .note
# ERR1-LLVM-NEXT: Offset: 0xFFFF0000
# ERR1-LLVM-NEXT: Size: 0x0
+# ERR1-LLVM-NEXT: Notes [
# ERR1-LLVM-NEXT: warning: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: invalid offset (0xffff0000) or size (0x0)
+# ERR1-LLVM-NEXT: ]
# ERR1-LLVM-NEXT: }
# ERR1-LLVM-NEXT: ]
@@ -182,7 +194,9 @@ Sections:
# ERR2-LLVM-NEXT: Name: .note
# ERR2-LLVM-NEXT: Offset: 0x40
# ERR2-LLVM-NEXT: Size: 0xFFFF0000
+# ERR2-LLVM-NEXT: Notes [
# ERR2-LLVM-NEXT: warning: '[[FILE]]': unable to read notes from the SHT_NOTE section with index 1: invalid offset (0x40) or size (0xffff0000)
+# ERR2-LLVM-NEXT: ]
# ERR2-LLVM-NEXT: }
# ERR2-LLVM-NEXT: ]
@@ -203,7 +217,9 @@ Sections:
# ERR3-LLVM-NEXT: Name: <?>
# ERR3-LLVM-NEXT: Offset: 0xFFFF0000
# ERR3-LLVM-NEXT: Size: 0x0
+# ERR3-LLVM-NEXT: Notes [
# ERR3-LLVM-NEXT: warning: '[[FILE]]': unable to read notes from the PT_NOTE segment with index 0: invalid offset (0xffff0000) or size (0x0)
+# ERR3-LLVM-NEXT: ]
# ERR3-LLVM-NEXT: }
# ERR3-LLVM-NEXT: ]
@@ -234,7 +250,9 @@ ProgramHeaders:
# ERR4-LLVM-NEXT: Name: <?>
# ERR4-LLVM-NEXT: Offset: 0x0
# ERR4-LLVM-NEXT: Size: 0xFFFF0000
+# ERR4-LLVM-NEXT: Notes [
# ERR4-LLVM-NEXT: warning: '[[FILE]]': unable to read notes from the PT_NOTE segment with index 0: invalid offset (0x0) or size (0xffff0000)
+# ERR4-LLVM-NEXT: ]
# ERR4-LLVM-NEXT: }
# ERR4-LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v2.test b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v2.test
index 778724b8ab6ce..2f281ad509bb9 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v2.test
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v2.test
@@ -10,100 +10,118 @@
# LLVM-NEXT: Name: .note.nt_amd_hsa_code_object_version_0
# LLVM-NEXT: Offset: 0x40
# LLVM-NEXT: Size: 0x14
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x4
# LLVM-NEXT: Type: NT_AMD_HSA_CODE_OBJECT_VERSION (AMD HSA Code Object Version)
# LLVM-NEXT: AMD HSA Code Object Version: Invalid AMD HSA Code Object Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_code_object_version_1
# LLVM-NEXT: Offset: 0x54
# LLVM-NEXT: Size: 0x1C
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0xC
# LLVM-NEXT: Type: NT_AMD_HSA_CODE_OBJECT_VERSION (AMD HSA Code Object Version)
# LLVM-NEXT: AMD HSA Code Object Version: Invalid AMD HSA Code Object Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_hsail_0
# LLVM-NEXT: Offset: 0x70
# LLVM-NEXT: Size: 0x1C
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0xA
# LLVM-NEXT: Type: NT_AMD_HSA_HSAIL (AMD HSA HSAIL Properties)
# LLVM-NEXT: AMD HSA HSAIL Properties: Invalid AMD HSA HSAIL Properties
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_hsail_1
# LLVM-NEXT: Offset: 0x8C
# LLVM-NEXT: Size: 0x24
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x14
# LLVM-NEXT: Type: NT_AMD_HSA_HSAIL (AMD HSA HSAIL Properties)
# LLVM-NEXT: AMD HSA HSAIL Properties: Invalid AMD HSA HSAIL Properties
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_version_0
# LLVM-NEXT: Offset: 0xB0
# LLVM-NEXT: Size: 0x18
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x8
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_VERSION (AMD HSA ISA Version)
# LLVM-NEXT: AMD HSA ISA Version: Invalid AMD HSA ISA Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_version_1
# LLVM-NEXT: Offset: 0xC8
# LLVM-NEXT: Size: 0x28
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x17
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_VERSION (AMD HSA ISA Version)
# LLVM-NEXT: AMD HSA ISA Version: Invalid AMD HSA ISA Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_version_2
# LLVM-NEXT: Offset: 0xF0
# LLVM-NEXT: Size: 0x28
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x17
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_VERSION (AMD HSA ISA Version)
# LLVM-NEXT: AMD HSA ISA Version: Invalid AMD HSA ISA Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_version_3
# LLVM-NEXT: Offset: 0x118
# LLVM-NEXT: Size: 0x28
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x17
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_VERSION (AMD HSA ISA Version)
# LLVM-NEXT: AMD HSA ISA Version: Invalid AMD HSA ISA Version
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_pal_metadata
# LLVM-NEXT: Offset: 0x140
# LLVM-NEXT: Size: 0x14
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x4
# LLVM-NEXT: Type: NT_AMD_PAL_METADATA (AMD PAL Metadata)
# LLVM-NEXT: AMD PAL Metadata: Invalid AMD PAL Metadata
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
index dd090b9483e29..52348b6559e2d 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amd-invalid-v3.test
@@ -10,7 +10,8 @@
# LLVM-NEXT: Name: .note.nt_amdgpu_metadata
# LLVM-NEXT: Offset: 0x40
# LLVM-NEXT: Size: 0x38
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMDGPU
# LLVM-NEXT: Data size: 0x24
# LLVM-NEXT: Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
@@ -21,6 +22,7 @@
# LLVM-NEXT: ...
# LLVM-EMPTY:
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/note-amd-valid-v2.test b/llvm/test/tools/llvm-readobj/ELF/note-amd-valid-v2.test
index 3af1bb2acafdf..adfd61f2b4923 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amd-valid-v2.test
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amd-valid-v2.test
@@ -10,89 +10,105 @@
# LLVM-NEXT: Name: .note.nt_amd_hsa_code_object_version
# LLVM-NEXT: Offset: 0x40
# LLVM-NEXT: Size: 0x18
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x8
# LLVM-NEXT: Type: NT_AMD_HSA_CODE_OBJECT_VERSION (AMD HSA Code Object Version)
# LLVM-NEXT: AMD HSA Code Object Version: [Major: 2, Minor: 1]
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_hsail
# LLVM-NEXT: Offset: 0x58
# LLVM-NEXT: Size: 0x1C
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0xC
# LLVM-NEXT: Type: NT_AMD_HSA_HSAIL (AMD HSA HSAIL Properties)
# LLVM-NEXT: AMD HSA HSAIL Properties: [HSAIL Major: 2, HSAIL Minor: 1, Profile: 1, Machine Model: 2, Default Float Round: 3]
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_version
# LLVM-NEXT: Offset: 0x74
# LLVM-NEXT: Size: 0x2C
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x1B
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_VERSION (AMD HSA ISA Version)
# LLVM-NEXT: AMD HSA ISA Version: [Vendor: AMD, Architecture: AMDGPU, Major: 8, Minor: 0, Stepping: 2]
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_metadata_0
# LLVM-NEXT: Offset: 0xA0
# LLVM-NEXT: Size: 0x10
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x0
# LLVM-NEXT: Type: NT_AMD_HSA_METADATA (AMD HSA Metadata)
# LLVM-NEXT: AMD HSA Metadata:
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_metadata_1
# LLVM-NEXT: Offset: 0xB0
# LLVM-NEXT: Size: 0x18
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x6
# LLVM-NEXT: Type: NT_AMD_HSA_METADATA (AMD HSA Metadata)
# LLVM-NEXT: AMD HSA Metadata: abcde
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_name_0
# LLVM-NEXT: Offset: 0xC8
# LLVM-NEXT: Size: 0x10
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x0
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_NAME (AMD HSA ISA Name)
# LLVM-NEXT: AMD HSA ISA Name:
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_hsa_isa_name_1
# LLVM-NEXT: Offset: 0xD8
# LLVM-NEXT: Size: 0x18
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x6
# LLVM-NEXT: Type: NT_AMD_HSA_ISA_NAME (AMD HSA ISA Name)
# LLVM-NEXT: AMD HSA ISA Name: abcdef
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.nt_amd_pal_metadata
# LLVM-NEXT: Offset: 0xF0
# LLVM-NEXT: Size: 0x28
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMD
# LLVM-NEXT: Data size: 0x18
# LLVM-NEXT: Type: NT_AMD_PAL_METADATA (AMD PAL Metadata)
# LLVM-NEXT: AMD PAL Metadata: [2: 1][4: 2][8: 4]
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/note-amd.s b/llvm/test/tools/llvm-readobj/ELF/note-amd.s
index 260be3a725af7..153b224578e9b 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amd.s
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amd.s
@@ -39,52 +39,59 @@
// LLVM-NEXT: Name: .note.no.desc
// LLVM-NEXT: Offset:
// LLVM-NEXT: Size:
-// LLVM-NEXT: Note {
+// LLVM-NEXT: Notes [
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0x0
// LLVM-NEXT: Type: NT_AMD_HSA_METADATA (AMD HSA Metadata)
// LLVM-NEXT: AMD HSA Metadata:
// LLVM-NEXT: }
-// LLVM-NEXT: Note {
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0x0
// LLVM-NEXT: Type: NT_AMD_HSA_ISA_NAME (AMD HSA ISA Name)
// LLVM-NEXT: AMD HSA ISA Name:
// LLVM-NEXT: }
+// LLVM-NEXT: ]
// LLVM-NEXT: }
// LLVM-NEXT: NoteSection {
// LLVM-NEXT: Name: .note.desc
// LLVM-NEXT: Offset:
// LLVM-NEXT: Size:
-// LLVM-NEXT: Note {
+// LLVM-NEXT: Notes [
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0xA
// LLVM-NEXT: Type: NT_AMD_HSA_METADATA (AMD HSA Metadata)
// LLVM-NEXT: AMD HSA Metadata: meta_blah
// LLVM-NEXT: }
-// LLVM-NEXT: Note {
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0x9
// LLVM-NEXT: Type: NT_AMD_HSA_ISA_NAME (AMD HSA ISA Name)
// LLVM-NEXT: AMD HSA ISA Name: isa_blah
// LLVM-NEXT: }
+// LLVM-NEXT: ]
// LLVM-NEXT: }
// LLVM-NEXT: NoteSection {
// LLVM-NEXT: Name: .note.other
// LLVM-NEXT: Offset:
// LLVM-NEXT: Size:
-// LLVM-NEXT: Note {
+// LLVM-NEXT: Notes [
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0x0
// LLVM-NEXT: Type: NT_AMD_PAL_METADATA (AMD PAL Metadata)
// LLVM-NEXT: AMD PAL Metadata:
// LLVM-NEXT: }
+// LLVM-NEXT: ]
// LLVM-NEXT: }
// LLVM-NEXT: NoteSection {
// LLVM-NEXT: Name: .note.unknown
// LLVM-NEXT: Offset:
// LLVM-NEXT: Size:
-// LLVM-NEXT: Note {
+// LLVM-NEXT: Notes [
+// LLVM-NEXT: {
// LLVM-NEXT: Owner: AMD
// LLVM-NEXT: Data size: 0x7
// LLVM-NEXT: Type: Unknown (0x000004d2)
@@ -92,6 +99,7 @@
// LLVM-NEXT: 0000: 61626364 656600 |abcdef.|
// LLVM-NEXT: )
// LLVM-NEXT: }
+// LLVM-NEXT: ]
// LLVM-NEXT: }
// LLVM-NEXT: ]
diff --git a/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s b/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
index 0ed791c30954e..97815ef356a8e 100644
--- a/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
+++ b/llvm/test/tools/llvm-readobj/ELF/note-amdgpu-invalid.s
@@ -35,7 +35,8 @@
# LLVM-NEXT: Name: .note.foo
# LLVM-NEXT: Offset: 0x40
# LLVM-NEXT: Size: 0xE8
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMDGPU
# LLVM-NEXT: Data size: 0xD4
# LLVM-NEXT: Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
@@ -55,12 +56,14 @@
# LLVM-NEXT: ...
# LLVM-EMPTY:
# LLVM-NEXT: }
+# LLVM-NEXT: ]
# LLVM-NEXT: }
# LLVM-NEXT: NoteSection {
# LLVM-NEXT: Name: .note.bar
# LLVM-NEXT: Offset: 0x128
# LLVM-NEXT: Size: 0x30
-# LLVM-NEXT: Note {
+# LLVM-NEXT: Notes [
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMDGPU
# LLVM-NEXT: Data size: 0x3
# LLVM-NEXT: Type: NT_AMDGPU_METADATA (AMDGPU Metadata)
@@ -68,7 +71,7 @@
# LLVM-NEXT: 0000: 123456 |.4V|
# LLVM-NEXT: )
# LLVM-NEXT: }
-# LLVM-NEXT: Note {
+# LLVM-NEXT: {
# LLVM-NEXT: Owner: AMDGPU
# LLVM-NEXT: Data size: 0x...
[truncated]
|
|
No objection, but the downside is that we now have both a top-level Personally I prefer not to use the LLVM style. I only use the LLVM style for COFF/Mach-O output where the llvm-objdump tabular output lacks some details... |
Yeah. I am building some tooling that just makes use of the JSON output for elf files mostly because it is persistently machine parseable. I do sort of wonder if we would be ok with formatted NT_PRPSINFO and NT_PRSTATUS notes similar to NT_FILE. Thoughts @jh7370? |
|
I can't see a way of avoiding the extra indentation, due to the structure of the stuff being dumped. I'd ideally like to come up with a different title, so we don't have an inner and an outer Notes list title, because I have a feeling that this could make things somewhat tricky to parse, especially in lit tests. Perhaps the inner one could be something like "NoteEntries"? I'll come to reviewing the code later. |
ilovepi
reviewed
Jun 27, 2024
Comment on lines
7915
to
7916
| template <class ELFT> void LLVMELFDumper<ELFT>::printNotes() { | ||
| ListScope L(W, "Notes"); |
There was a problem hiding this comment.
Maybe this should be NoteSections or NoteSectionList to disambigutate? @jh7370's suggestion also seems fine, but this seems to be the convention we normally use: to name the list as the plural of the type being listed. I'm fine either way, as long as we're outputting something reasonable, though.
There was a problem hiding this comment.
I changed it to NodeSectionList. That seems like a pretty reasonable tag
There was a problem hiding this comment.
The existing ListScope variables use *s instead of *List. I wonder whether NoteSections would be better.
Comment on lines
34
to
44
| // LLVM-NEXT: Notes [ | ||
| // LLVM-NEXT: { | ||
| // LLVM-NEXT: Owner: XYZ | ||
| // LLVM-NEXT: Data size: 0x1C | ||
| // LLVM-NEXT: Type: Unknown (0x00000003) | ||
| // LLVM-NEXT: Description data ( | ||
| // LLVM-NEXT: 0000: 4C6F7265 6D206970 73756D20 646F6C6F |Lorem ipsum dolo| | ||
| // LLVM-NEXT: 0010: 72207369 7420616D 65740000 |r sit amet..| | ||
| // LLVM-NEXT: ) | ||
| // LLVM-NEXT: } | ||
| // LLVM-NEXT: ] |
There was a problem hiding this comment.
Is this indentation correct? I thought we used 2 space indentation for the LLVM style, but these appear to use tabs now... that seems like a regression, IMO. This also only seems to happen w/ the LLVM output, though I'm not sure why, based on the code changes I see.
There was a problem hiding this comment.
I think my vim settings are off. I'll fix this
jh7370
reviewed
Jun 28, 2024
llvm/test/tools/llvm-readobj/ELF/AArch64/aarch64-feature-pauth.s
Outdated
Show resolved
Hide resolved
llvm/test/tools/llvm-readobj/ELF/note-core-multiple-sections.test
Outdated
Show resolved
Hide resolved
Thanks for the update. LGTM but give @jh7370 a chance. The example below The description will be used as the default commit message when "Squash and merge" is clicked. |
MaskRay
approved these changes
Jul 2, 2024
Will do.
Yeah I am planning on cleaning up the commit message before merging |
Taking a look now.
Just to be abundantly clear, the only thing you need to "clean up" for the final merged-in commit to have the right commit message is the description of the PR: LLVM's GitHub uses the "Use PR description by default when squashing and merging". |
kbluck
pushed a commit
to kbluck/llvm-project
that referenced
this pull request
Jul 6, 2024
…for multiple notes per section in JSON (llvm#96813) It turns out that the notes section for corefiles (or really any elf file with multiple notes) is set up in such a way for LLVM formatted output that the JSON equivalent only has the last note since the notes are held in a dictionary with every key being Note. This pr alters the layout for the notes to a list of dictionaries to sidestep this issue for JSON output. Prior to this pr a note section in the output looked like (for LLVM output): ``` Notes [ NoteSection { Name: <?> Offset: 0x2148 Size: 0x1F864 Note { Owner: CORE Data size: 0x150 Type: NT_PRSTATUS (prstatus structure) Description data ( 0000: 06000000 00000000 00000000 06000000 |................| ... ) } Note { Owner: CORE Data size: 0x88 Type: NT_PRPSINFO (prpsinfo structure) Description data ( 0000: 02440000 00000000 04054040 00000000 |.D........@@....| .... ``` But is now: ``` NoteSections [ NoteSection { Name: <?> Offset: 0x2148 Size: 0x1F864 Notes [ { Owner: CORE Data size: 0x150 Type: NT_PRSTATUS (prstatus structure) Description data ( 0000: 06000000 00000000 00000000 06000000 |................| ... ) } { Owner: CORE Data size: 0x88 Type: NT_PRPSINFO (prpsinfo structure) Description data ( 0000: 02440000 00000000 04054040 00000000 |.D........@@....| ... ```
|
It's unfortunate that this left the useless |
Could you give an example of what you'd prefer the output to be? In the presence of potentially multiple note sections, each with multiple notes, I can't see any redundant layering. |
Here's an example object file: Here's the current output for that: What purpose is served by having a dictionary in each element of the "Notes" array, each having only one entry with one key of "NoteSection"? Here's one possible alternate schema that avoids single-entry dictionaries, and also avoids dictionaries with multiple identical keys used in place of lists (which is awkward for many JSON-ingesting programming situations, where dictionaries are usually accessed with the expectation of only a single match for each key): I don't have strong opinions about the exact schema. But layers of dictionary that add no information seems like an anti-pattern, and multiple identical keys in a dictionary also seems like an anti-pattern. |
|
Thanks, that does look much cleaner and I'm not sure how I missed that! @feg208, are you okay to rework this as suggested above, to resolve this issue, please? If it can be got done fairly quickly, we would probably want to cherry-pick it into the ongoing release, so that we don't have two releases with different formats unnecessarily. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It turns out that the notes section for corefiles (or really any elf file with multiple notes) is set up in such a way for LLVM formatted output that the JSON equivalent only has the last note since the notes are held in a dictionary with every key being Note. This pr alters the layout for the notes to a list of dictionaries to sidestep this issue for JSON output. Prior to this pr a note section in the output looked like (for LLVM output):
But is now: