[PATCH] D133030: [AIX] llvm-readobj support a new option --exception-section for xcoff object file.

[Digger Lin via Phabricator via llvm-commits]

DiggerLin added inline comments.


================
Comment at: llvm/lib/Object/XCOFFObjectFile.cpp:1008-1011
+  if (is64Bit())
+    assert(sizeof(ExceptEnt) == sizeof(ExceptionSectionEntry64));
+  else
+    assert(sizeof(ExceptEnt) == sizeof(ExceptionSectionEntry32));
----------------
jhenderson wrote:
> I recommend folding this `if` into the assertions, possibly like the inline suggestion. Otherwise you have an empty if without assertions, which seems messy (although the optimizer should get rid of it in this case).
thanks for explain. fixed


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/exception-section.test:9
+# CHECK:       Exception section {
+# CHECK32-NEXT:   Symbol Index: 12 (.bar)
+# CHECK64-NEXT:   Symbol Index: 6 (.bar)
----------------
jhenderson wrote:
> I think it would be more similar to other dumping formats to print this row as:
> `Symbol: .bar (12)`.
according to https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__iua3i23ajbau

e_addr.e_symndx+
Symbol table index for function
 
the value of the field is for symbol index, so putting symbol index at first and putting symbol name in brackets are reasonable.


================
Comment at: llvm/tools/llvm-readobj/llvm-readobj.cpp:508-509
+
+  if (Obj.isXCOFF() && opts::XCOFFExceptionSection)
+    Dumper->printExceptionSection();
+
----------------
jhenderson wrote:
> Is there a reasson you're not putting this up with the other XCOFF-specific dumping option?
If there are several options in the command line at the same time. I  want to keep the output  as order of content xcoff object file as much as possible. 
If you do not agree with this, I can put the all the XCOFF-specific dumping option together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133030