[PATCH] D82886: [DebugInfo] Fix a possible crash when reading a malformed .debug_*lists section.

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jul 8 21:54:11 PDT 2020


ikudrin added a comment.

In D82886#2139657 <https://reviews.llvm.org/D82886#2139657>, @dblaikie wrote:

> I'm not suggesting it needs to be fixed - but that that codepath (the one that returns zero) is untested - so when it was committed, it was committed without test coverage. It'd be good to add test coverage where it is missing like this.


Isn't adding that test coverage orthogonal to this particular patch?

> Right - but what I mean is if there's only 10 bytes, as in your example - it reads the 4 bytes of DWARF64 mark, then 6 bytes out of the desired 8 - if the length was then reported as 10 (with an error saying the length was garbled/the contents terminated earlier than expected), would that be adequate to no longer need the zero length special case?

I am OK with the current convention that if it is not possible to read the length field the code returns zero as the total length. It might be better to make the result `Optional` and return `None` in that case, but I really doubt it is worth investing time in that. Reporting something that was not read from the section (10 in your example) smells not good for me, but you may provide the patch if you feel like the code will be improved.


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

https://reviews.llvm.org/D82886





More information about the llvm-commits mailing list