Page MenuHomePhabricator

[DWARFYAML] Add support for emitting multiple abbrev tables.
Needs ReviewPublic

Authored by Higuoxing on Jul 3 2020, 12:25 AM.

Details

Summary

This patch enables yaml2obj to emit multiple abbrev tables. This patch also introduces a new
field 'AbbrevTableIndex' in the compilation unit which records the associated abbrev table
index when emitting values in the compilation unit. Currently, we still need to specify the
'AbbrOffset' manually. In the future, we will make the 'AbbrOffset' optional.

Diff Detail

Event Timeline

Higuoxing created this revision.Jul 3 2020, 12:25 AM
Herald added a reviewer: alexshap. · View Herald Transcript
Herald added projects: Restricted Project, Restricted Project. · View Herald Transcript
Higuoxing marked 3 inline comments as done.Jul 3 2020, 12:31 AM

Newly added tests are marked by inline comments since too many tests are updated 😂.

llvm/test/ObjectYAML/MachO/DWARF-debug_info.yaml
685

Added test.

798

Added test.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
651

Added test.

labath added a subscriber: labath.Jul 3 2020, 1:19 AM

What would you say if, instead of AbbrevTableIndex, we had a field like AbbrevTableID. The main difference would be that this "ID" field can be explicitly specified on the Abbrev table, and it does not have to be a sequentially increasing number (though it could of course be that by default).

The thing I'm trying to achieve is to make the yaml more robust against modifications/simplifications. E.g., it would be nice if deleting an abbrev table does not make all compile units suddenly refer to different tables. If the ids were present explicitly, compile units would be unaffected by this, and one would get an explicit error message if there was a compile unit left which still referred to the deleted abbrev table.

(This is one of the aspects where an assembler is better than yaml, as symbolic labels in asm have these properties.)

What would you say if, instead of AbbrevTableIndex, we had a field like AbbrevTableID. The main difference would be that this "ID" field can be explicitly specified on the Abbrev table, and it does not have to be a sequentially increasing number (though it could of course be that by default).

I think it works. I'm fine with this approach, let's see what others thinking. Thanks a lot for sharing your ideas!

debug_abbrev:
  - ID: 1
    Table:
      - Code: 1
        Tag:  DW_TAG_something
        Children: DW_CHILDREN_yes
        Attributes:
          - Attribute: DW_AT_something
            Form:      DW_FORM_something
  - ID: 2
    Table:
      - Code: 1
        Tag:  DW_TAG_something
        Children: DW_CHILDREN_yes
        Attributes:
          - Attribute: DW_AT_something
            Form:      DW_FORM_something
  - ID: 3
    Table:
      - Code: 1
        Tag:  DW_TAG_something
        Children: DW_CHILDREN_yes
        Attributes:
          - Attribute: DW_AT_something
            Form:      DW_FORM_something

debug_info:
  - Length:        0x1234
    Version:       4
    AbbrevTableID: 3 ## references table 3
    ...
    Entries:
      - AbbrevCode: 1
        Values:
          - Value: 0x1234
  - Length:        0x1234
    Version:       4
    AbbrevTableID: 2 ## references table 2
    ...
    Entries:
      - AbbrevCode: 1
        Values:
          - Value: 0x1234
  - Length:        0x1234
    Version:       4
    AbbrevTableID: 1 ## references table 1
    ...
    Entries:
      - AbbrevCode: 1
        Values:
          - Value: 0x1234

What would you say if, instead of AbbrevTableIndex, we had a field like AbbrevTableID. The main difference would be that this "ID" field can be explicitly specified on the Abbrev table, and it does not have to be a sequentially increasing number (though it could of course be that by default).

The thing I'm trying to achieve is to make the yaml more robust against modifications/simplifications. E.g., it would be nice if deleting an abbrev table does not make all compile units suddenly refer to different tables. If the ids were present explicitly, compile units would be unaffected by this, and one would get an explicit error message if there was a compile unit left which still referred to the deleted abbrev table.

(This is one of the aspects where an assembler is better than yaml, as symbolic labels in asm have these properties.)

+1 to this idea. By default it could of course be an incremental index, when emitting, but in yaml2obj, I see no reason for it to be tied in that way necessarily.

lldb/unittests/Symbol/Inputs/inlined-functions.yaml
348

AbbrevTableIndex should be an optional value. I think by default it should pick the first table (or possibly should pick the Nth table, where N is the .debug_info table index). Thus in this and pretty much every other case, you should be able to omit the new value.

llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml
658

Perhaps worth including the index and table count values in this error message to make it easier for people to identify the problem.

llvm/tools/obj2yaml/dwarf2yaml.cpp
19

I'm slightly surprised you needed to add this. Did this not compile without it?

Higuoxing marked an inline comment as done.Jul 3 2020, 2:47 AM
Higuoxing added inline comments.
llvm/tools/obj2yaml/dwarf2yaml.cpp
19

I guess it was automatically added by the code completion tool.