[PATCH] D38237: [dwarfdump] Add support for -debug-loc=OFFSET

Adrian Prantl via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Sep 26 15:31:21 PDT 2017


aprantl added a comment.

In https://reviews.llvm.org/D38237#881042, @JDevlieghere wrote:

> I have some code that uses `parseOneLocationList(DataExtractor Data, unsigned *Offset)` to do this. This is a good solution for valid offsets, but not so much for invalid offsets as it is incremented based on the previously parsed amount of bytes. I have a few concerns about this though:
>
> - We currently prints errors when we can't parse the section. Would we display them conditionally? (e.g. during regular parsing but not during parsing for dumping)
> - I don't know how likely this scenario is, but what if an invalid offset causes the data to be accidentally parsable?
>
>   If we move forward with this approach, I'd recommend doing it in in a separate differential as it will mostly build on top of the changes in this patch. (I'm thinking about the tests, the changes in dump method, and we could even keep the current approach as fallback)


I support doing the lazy API in a follow-up commit. More fine-grained commits is always preferable, it makes reviews easier and usually yields better test coverage.
As for errors:

- When it is easily possible to determine that no entry exists at the give offset, we should not print anything.
- Otherwise, we probably should suppress errors when dumping with `DumpOffsets[<section>] != None`. Why? Because someone might call `llvm-dwarfdump --debug-info=0x000abcd fat_macho.o` and I think it is reasonable to dump DIEs in each slice that has something at the given offset and be silent in all others.


Repository:
  rL LLVM

https://reviews.llvm.org/D38237





More information about the llvm-commits mailing list