[PATCH] D60784: [Object][XCOFF] Add intial support for parsing/dumping section header table.

Hubert Tong via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Apr 19 20:22:10 PDT 2019


hubert.reinterpretcast accepted this revision.
hubert.reinterpretcast added a comment.
This revision is now accepted and ready to land.

LGTM with minor changes (no need to post a new revision).



================
Comment at: llvm/include/llvm/Object/XCOFFObjectFile.h:24
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/SymbolicFile.h"
 #include "llvm/Support/Casting.h"
----------------
sfertile wrote:
> sfertile wrote:
> > sfertile wrote:
> > > DiggerLin wrote:
> > > > ObjectFile.h already include SymbolicFile.h , I think there do need SymbolicFile.h here. and also 
> > > > #include "llvm/Object/Binary.h"
> > > > #include "llvm/Object/Error.h"
> > > > #include "llvm/BinaryFormat/Magic.h"
> > > > #include "llvm/ADT/iterator_range.h"
> > > > 
> > > Good point, most of these are already included from ObjectFile.h. I'll clean up our header includes but I'll land that in a separate NFC patch.
> > Didn't intend to mark this done yet.
> Posted: https://reviews.llvm.org/D60885. I plan to commit that Monday or Tuesday and then I'll rebase this patch to pick up the changes.
 I'd be happy with seeing this committed and having D60885 adjusted instead.


================
Comment at: llvm/test/tools/llvm-objdump/xcoff-section-headers.test:23
+# xcoff-section-headers.o Compiled with IBM XL C/C++ for AIX, V16.1.0
+# source:
+# int a;
----------------
Some indication of whether the source is C or C++ (e.g., the base name of the source file) would be helpful.


================
Comment at: llvm/test/tools/llvm-objdump/xcoff-section-headers.test:29
+#
+#int func(void)  {
+#  return a;
----------------
Add a space after the `#` on this line (and the next few lines) to match the above lines.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60784





More information about the llvm-commits mailing list