[PATCH] D82173: [DWARFYAML][debug_info] Use 'AbbrCode' to index the abbreviation.

[James Henderson via Phabricator via llvm-commits]

jhenderson accepted this revision.
jhenderson added a comment.
This revision is now accepted and ready to land.

LGTM.



================
Comment at: llvm/lib/ObjectYAML/DWARFVisitor.cpp:59
+      // TODO: Add error handling and test this error.
+      assert(AbbrCode <= DebugInfo.AbbrevDecls.size());
+      auto &Abbrev = DebugInfo.AbbrevDecls[AbbrCode - 1];
----------------
Higuoxing wrote:
> grimar wrote:
> > Doesn't seem this assert is helpful. `AbbrevDecls` is `std::vector<Abbrev>`,
> > Most (or all) of STL implementations check the out of bounds access internally I think.
> > 
> > Probably you should add a test case and do the following for now:
> > 
> > ```
> > if (AbbrCode > DebugInfo.AbbrevDecls.size())
> >   report_fatal_error(...)
> > ```
> Sorry, I haven't tested a `report_fatal_error()` before. What will be checked? The dumped stack trace? or something else? Can I leave a "TODO" here and let this function return `Error` in a follow-up patch?
Typically, you wouldn't test a `report_fatal_error` as it shouldn't be possible to reach that bit of code. It's like an assert, but works without asserts being enabled too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D82173