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

[Stephen Peckham via Phabricator via llvm-commits]

stephenpeckham added inline comments.


================
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).
----------------
jhenderson wrote:
> This should be referring to the Big Archive format, right, not the OS?
> 
> Other suggestions in the inline edit.
The alignment is not a requirement of the Big Archive format.  It's required by the OS for 64-bit members and recommended for 32-bit members. AIX allows shared objects to be archive members. When archive members are loaded, they are mapped into memory. If the members aren't aligned properly in the archive, they won't be aligned in memory.  Both .text and .data are mapped, so the required member alignment takes into account both the .text and .data alignment. Alignment is not necessary for members that are not loadable.


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:461
+        return 1;
+      return 1 << (AuxHeader->MaxAlignOfText > AuxHeader->MaxAlignOfData
+                       ? AuxHeader->MaxAlignOfText
----------------
Only loadable objects need to be aligned. Onc requirement for a loadable module is the presence of a loader section.  The o_snloader field in the auxiliary header can be checked. 


================
Comment at: llvm/lib/Object/ArchiveWriter.cpp:683
+                                       alignTo((M + 1)->MemberName.size(), 2);
+        uint64_t NextMemHeadPadSize =
+            alignToPowerOf2(OffsetToNextMemData,
----------------
It's possible to have a loadable object with a very large text or data alignment. A sanity check would be useful here.  The AIX ar command caps the alignment at 2^12 (the typical PAGESIZE on an AIX system).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144872