[PATCH] D72337: [DebugInfo][Support] Replace DWARFDataExtractor size function

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jan 10 07:24:44 PST 2020


jhenderson added a comment.

In D72337#1813202 <https://reviews.llvm.org/D72337#1813202>, @dblaikie wrote:

> In D72337#1811669 <https://reviews.llvm.org/D72337#1811669>, @jhenderson wrote:
>
> > Added unit test case that would trigger the assertion mentioned in the description. I'm not entirely convinced that this test case is worthwhile, since with the change in how size() is calculated, it really is moot (e.g. I wouldn't have needed it if the size() function had always been the way I'm proposing).
>
>
> Ah, I was more expecting an end-to-end test (with llvm-dwarfdump or similar) when you said in the patch description "The old behaviour could cause an assertion in the debug line parser"
>
> What were the circumstances where this assertion would fail? Was the failure reachable with llvm-dwarfdump, or only via a unit test/non-production API usage?


Oh, sorry for misunderstanding. We have a local test for a local patch in LLD that makes some use of the debug information, but this patch doesn't create the data extractor with a section, which meant that it ran into this assertion. I checked, and I can't find any other usage of this function with a data extractor constructed without a section.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72337





More information about the llvm-commits mailing list