[PATCH] D83116: [DWARFYAML] Add support for emitting multiple abbrev tables.

Fri Jul 3 02:40:03 PDT 2020

jhenderson added a comment.

In D83116#2130019 <https://reviews.llvm.org/D83116#2130019>, @labath wrote:

> 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.

================
Comment at: lldb/unittests/Symbol/Inputs/inlined-functions.yaml:348
+      Version:          4
+      AbbrevTableIndex: 1
+      AbbrOffset:       0
----------------
`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.

================
Comment at: llvm/test/tools/yaml2obj/ELF/DWARF/debug-info.yaml:658
+
+# NOTABLE: yaml2obj: error: abbrev table index must be less than or equal to the number of abbrev tables
+
----------------
Perhaps worth including the index and table count values in this error message to make it easier for people to identify the problem.

================
Comment at: llvm/tools/obj2yaml/dwarf2yaml.cpp:19
 #include <algorithm>
+#include <utility>

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

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D83116/new/

https://reviews.llvm.org/D83116