[PATCH] D79612: [Object] Add tests for parsing invalid .dynamic section.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I've been sitting on this for a few days, and here's what I think would be my ideal situation:

1. Library code paths should be comprehensively tested via unit tests.
2. Tools using the library should just test that errors returned via the library interface are handled appropriately. If you treat the library as a black box, that just means each library call only needs a single corresponding test case in each tool that calls into it, to show how the `Error`/`Expected` return value is handled. Such lit tests would exist in the corresponding tool folder. They would inevitably slightly overlap with library unit tests, but their focus isn't the same (and appropriate test naming/commenting can probably illustrate that).

Additional test cases might need to exist at a tool level to show that a specific behaviour in the tool circumvents something that might otherwise fire from a later library call (e.g. say a tool had special handling for missing DT_NULL entries), but those tests would be testing the circumvention code path rather than the error handling. If somebody changes the behaviour of a tool to add special handling on that, they should of course ensure that the new behaviour is tested in the tool.

That's not the situation we're in at the moment, but I think we can at least ensure new tests follow that style, by preferring unit tests for the library changes (or in this case, untested library paths). If somebody wants to start porting lit tests to unittests, I'm certainly not going to stop them though.

So, my conclusion is that this new lit test shouldn't exist, but rather that the two test cases should be two unit tests in the Object library. The only issue might be in setting something like that up, but I know that, e.g. the Minidump tests use YAML as a basis to generate an object, so you might be able to follow them to create a malformed dynamic section without too much difficulty.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D79612