[PATCH] D88635: [llvm-readobj] Add --coff-tls-directory flag to print TLS Directory & test.

[Luqman Aden via Phabricator via llvm-commits]

luqmana added a comment.

In D88635#2305377 <https://reviews.llvm.org/D88635#2305377>, @jhenderson wrote:

> Thanks for the patch. It looks to me like this patch is intended to add a new option to llvm-readobj. This should be tested in the llvm-readobj tests, and not in the LLD tests. If you want a new test for LLD, you'll probably want a separate patch for using this new option. You'll want to use a different method to create your input for llvm-readobj, since the LLVM tools should be testable without it. This might require you to use something like yaml2obj to create your test input (I'm assuming you need a  "linked" image, rather than a relocatable COFF object - I'm not particularly familiar with COFF).
>
> You also need to update the documentation located at `llvm\docs\CommandGuide\llvm-readobj.rst` with the new option.
>
> I can't comment on the functional logic of the patch, due to lack of expertise with COFF. Please make sure to add some reviewers who have touched/reviewed the COFF dumping code recently.

Yea, I'll just split out that LLD test into a separate revision. For the llvm-readobj specifc test, I looked around the existing tests and kinda copied the same setup by using a locally generated .exe (with instructions how in the test).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88635