[PATCH] D85519: [llvm-readobj/elf] - Refine the code for broken PT_DYNAMIC segment diagnostic.

Mon Aug 10 07:37:26 PDT 2020

grimar planned changes to this revision.
grimar marked an inline comment as done.
grimar added a comment.

In D85519#2206037 <https://reviews.llvm.org/D85519#2206037>, @jhenderson wrote:

> Maybe it would be a good idea to split this change in two - one to update the test, and the other to change the behaviour - to ensure we don't accidentally break something in the test and behaviour at the same time. What do you think?

Well, OK. It might take more than 2 splits then I think. Currently we report "PT_DYNAMIC segment offset + size exceeds the size of the file" for two slightly different cases
and it does not provide enough information about the difference. So I'd start from improving this message first. Then partially rewriting the test to a new one, simplified, which will use the new messages.
And then a third patch will change the logic and add the corresponding testing.

================
Comment at: llvm/test/tools/llvm-readobj/ELF/malformed-pt-dynamic.test:68-69
       - Section: .dynamic
+SectionHeaderTable:
+  NoHeaders: true
----------------
jhenderson wrote:
> Do we actually need to strip the section headers any more? I'm not sure the original issue really applies any more, as it was just to do with yaml2obj's previous limitations.
I'll answer on this later. I am observing a different behavior without stripping headers - llvm-readobj complains about broken PT_DYNAMIC and says it going to use some information from the sections header instead. But I had no chance to investigate it better yet. 

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

https://reviews.llvm.org/D85519