[PATCH] D144872: [AIX] Align the content of an xcoff object file which has auxiliary header in big archive.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

I've started reviewing, but have run out of time for this for now. One high-level question: regular Unix archive member alignment is done at the end of a member (I believe), rather than the start of the next, meaning that the final member will have tail padding, if needed. I take it that this isn't the case here, since the alignment is purely to ensure aligned data in the object?



================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:442-444
+// In AIX OS, if the member file is an XCOFF object file and has an auxiliary
+// header, the content of the member file need to be aligned at the
+// MAX(maximum alignment of .text , maximum alignment of .data).
----------------
This should be referring to the Big Archive format, right, not the OS?

Other suggestions in the inline edit.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:445
+// MAX(maximum alignment of .text , maximum alignment of .data).
+static uint32_t getMemberAlignment(const StringRef &ObjStringRef) {
+  LLVMContext Context;
----------------
Two nits, and one more significant point.
1) No need for `const &` for `StringRef`, which is intended to be copied.
2) `ObjStringRef` -> `Obj`.
3) This function appears to be reading in the file and parsing it just to get the alignments. However, presumably this isn't the only place where we have the fully parsed object, since at some point you have to know what to write in the object file in the first place, right? Wouldn't it make more sense to identify and record this alignment then?


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:452
+  if (!ObjOrErr) {
+    consumeError(ObjOrErr.takeError());
+    return 1;
----------------
Seems like this should report the error, not just ignore it...


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:468
+      return GetAuxMaxAlignment(XCOFFObj->auxiliaryHeader64());
+    else
+      return GetAuxMaxAlignment(XCOFFObj->auxiliaryHeader32());
----------------
No need for `else` after `return`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144872