[PATCH] D151353: [DebugInfo] Add error-handling to DWARFAbbreviationDeclarationSet

[Felipe de Azevedo Piovezan via Phabricator via llvm-commits]

fdeazeve added inline comments.


================
Comment at: llvm/unittests/DebugInfo/DWARF/DWARFDebugAbbrevTest.cpp:24
+void writeValidAbbreviationDeclarations(raw_ostream &OS, uint32_t FirstCode,
+                                        OrderKind Order) {
   encodeULEB128(FirstCode, OS);
----------------
This is likely irrelevant for the patch itself, but I missed the boat on the original review: aren't we missing the compile unit terminator for this in most tests?


We have this structure:
```
First code
  DW_TAG_compile_unit
    Has Children = true
    Name, strp
    0, 0   // terminates list of attributes

    DW_TAG_subprogram   // children of compile unit
      Has children = false
      Name, strp
      0, 0   // terminates list of attributes
```

This is all the function writes. Sometimes, the callees write an extra zero, which is terminating the `DW_TAG_compile_unit` children list. (Maybe we should write that one here instead? The extra entries we write manually write in each test would simply become siblings of the TAG_compile_unit, which is fine)


But even with that extra zero, we're missing an extra zero terminating the abbreviation table itself (when multiple abbreviation tables are stitched together, they are separated by this terminator, see page 207 line 15 of the D5 spec)

Anyhow, I am pointing all of this out because it's curious that the `DWARFAbbreviationDeclarationSet::extract` method seems to happily ignore the absence of this?



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D151353