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

[Luqman Aden via Phabricator via llvm-commits]

luqmana marked 11 inline comments as done.
luqmana added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/tls-directory.test:12-13
+
+## .rdata
+## 004040000840400000204000000000000000000000003000
+
----------------
jhenderson wrote:
> I'm not sure I understand the purpose of this comment.
That's the binary form of the TLSDirectory struct we're printing out. I had the comment there before so it'd be closer to the actual CHECK lines. But the file is yaml has only one section now and is small enough anyways so I'll just remove that.


================
Comment at: llvm/test/tools/llvm-readobj/COFF/tls-directory.test:111
+# RUN: yaml2obj %s --docnum=2 -o %t.no-tls1.exe -DTLSRVA=0 -DTLSSIZE=0
+# RUN: llvm-readobj --coff-tls-directory %t.no-tls1.exe | FileCheck %s --check-prefix NO-TLS1
+
----------------
grimar wrote:
> I wonder if in this case an empty scope should be emitted. I.e:
> 
> ```
> TLSDirectory {
> }
> ```
> 
> This is what we do for ELF sometimes, e.g:
> 
> 
> ```
> template <typename ELFT> void ELFDumper<ELFT>::printHashTable() {
>   DictScope D(W, "HashTable");
>   if (!HashTable)
>     return;
> ```
> 
> but I am not familar with llvm-readelf COFF to say what is the preferred style.
> Could you check?
Looked at a couple methods and they seem to do the same. Updated to follow that style.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D88635