[PATCH] D87179: Fix debug_abbrev emitter to only assign table id once
Xing GUO via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 5 10:41:12 PDT 2020
Higuoxing added a comment.
I think the problem is caused by misplacing the line `Y.DebugAbbrev.back().ID = AbbrevTableID++;`. It should be placed in an outer loop.
The ID of abbrev tables should be their index and the AbbrevID of compilation units should be the distance between the associated abbrev table and the first abbrev table.
Thanks for the quick fix!
================
Comment at: llvm/test/tools/obj2yaml/MachO/debug-abbrev.yaml:1
+# RUN: yaml2obj --docnum=1 %s | obj2yaml | FileCheck %s --check-prefix=ABBREV
+
----------------
I would suggest modifying the test case (c) by duplicating entries in abbrev tables and in compilation units in llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml rather than creating a standalone test case here.
I think the following YAML is a good candidate that replaces the one in test case (c). Please ping me if it doesn't work.
```
--- !mach-o
FileHeader:
magic: 0xFEEDFACF
cputype: 0x01000007
cpusubtype: 0x00000003
filetype: 0x0000000A
ncmds: 1
sizeofcmds: 232
flags: 0x00000000
reserved: 0x00000000
LoadCommands:
- cmd: LC_SEGMENT_64
cmdsize: 232
segname: __DWARF
vmaddr: 0x00
vmsize: 0x00
fileoff: 0x00
filesize: 0x00
maxprot: 0
initprot: 0
nsects: 2
flags: 0
Sections:
- sectname: __debug_abbrev
segname: __DWARF
addr: 0x00
size: 45
offset: 528
align: 0
reloff: 0x00000000
nreloc: 0
flags: 0x00000000
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
- sectname: __debug_info
segname: __DWARF
addr: 0x00
size: 90
offset: 1070
align: 0
reloff: 0x00000000
nreloc: 0
flags: 0x00000000
reserved1: 0x00000000
reserved2: 0x00000000
reserved3: 0x00000000
DWARF:
debug_abbrev:
- Table:
- Code: 1
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_addr
- Code: 2
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_addr
- ID: 2
Table:
- Code: 1
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_data4
- Code: 2
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_data4
- ID: 1
Table:
- Code: 1
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_udata
- Code: 2
Tag: DW_TAG_compile_unit
Children: DW_CHILDREN_no
Attributes:
- Attribute: DW_AT_low_pc
Form: DW_FORM_udata
debug_info:
- Version: 4
AbbrevTableID: 2
Entries:
- AbbrCode: 1
Values:
- Value: 0x1234
- AbbrCode: 2
Values:
- Value: 0x1234
- Version: 4
AbbrevTableID: 2
Entries:
- AbbrCode: 1
Values:
- Value: 0x4321
- AbbrCode: 2
Values:
- Value: 0x4321
- Version: 4
AbbrevTableID: 0
Entries:
- AbbrCode: 1
Values:
- Value: 0x5678
- AbbrCode: 2
Values:
- Value: 0x5678
- Version: 4
AbbrevTableID: 1
Entries:
- AbbrCode: 1
Values:
- Value: 0x8765
- AbbrCode: 2
Values:
- Value: 0x8765
```
================
Comment at: llvm/test/tools/obj2yaml/MachO/debug-abbrev.yaml:4
+# ABBREV: DWARF:
+# ABBREV-NEXT: debug_abbrev:
+# ABBREV-NEXT: - ID: 0
----------------
aadsm wrote:
> This test could be better if I added debug_infos and made sure it was pointing to table 0 as well. It would be a pain for me to make all the sizes and offsets match though. Is there an easy way to achieve this? How were the other tests created?
Yeah, creating Mach-O tests are painful. I create them by manually assigning offset and size as well. When handcrafting minimal Mach-O test cases, I usually focus on the following fields.
* The ncmds and sizeofcmds in the FileHeader: For DWARF tests, ncmds should be 1 since there is only one load command (LC_SEGMENT_64). sizeofcmds should be greater than or equal to the size of load commands and less than or equal to the size of object file minus the size of mach_header_64 (actual_file_size - sizeof(mach_header_64)). In this case, sizeofcmds should be greater than or equal to 8 since we only have one load command and sizeof(load_command) is 8.
* The cmdsize field of load commands: cmdsize must be a multiple of 8 in 64-bit object files or a multiple of 4 in 32-bit object files. When there are 1 section in the load command, 152 would cover most cases. When there are 2 sections in the load command, 232 would cover most cases. I didn't investigate it very much.
* The size field of sections: Firstly, we can use yaml2elf create a ELF object file with DWARF sections and use `readelf -S` to check the debug sections' size. The section size in Mach-O is similar to the size in ELF.
* The offset field of sections: I didn't investigate it as well. But 528 is usually fine for the first section.
I think we can improve it in the future 😂
================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:42-46
+ if (!Y.DebugAbbrev.back().ID.hasValue())
+ Y.DebugAbbrev.back().ID = AbbrevTableID++;
Y.DebugAbbrev.back().Table.push_back(Abbrv);
}
}
----------------
It looks that `Y.DebugAbbrev.back().ID = AbbrevTableID++;` should be put in the outer loop.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87179/new/
https://reviews.llvm.org/D87179
More information about the llvm-commits
mailing list