[XCOFF] write the aux header when the visibility is specified in XCOFF32.

[Esme]

Summary

The n_type field in the symbol table entry has two interpretations in XCOFF32, and a single interpretation in XCOFF64.
The new interpretation is used in XCOFF32 if the value of the o_vstamp field in the auxiliary header is 2.
In XCOFF64 and the new XCOFF32 interpretation, the n_type field is used for the symbol type and visibility.

The patch writes the aux header with an o_vstamp field value of 2 when the visibility is specified in XCOFF32 to make the new XCOFF32 interpretation used.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

34

not sure how the size 28 come from ?

llvm/lib/MC/XCOFFObjectWriter.cpp

315

according to "The auxiliary header contains system-dependent and implementation-dependent information, in which is used for loading and executing a module. Information in the auxiliary header minimizes how much of the file must be processed by the system loader at execution time." in https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__fyovh386shar

the auxiliaryHeaderSize should not depend on the HasVisibility

785

in the xcoff.h , there is
enum XCOFFInterpret : uint16_t {

OLD_XCOFF_INTERPRET = 1,
NEW_XCOFF_INTERPRET = 2

};

maybe better to use NEW_XCOFF_INTERPRET than 2 .

[Esme]

llvm/include/llvm/BinaryFormat/XCOFF.h

34

Some AIX programs generate auxiliary headers for 32-bit object files that end after the data_start field.  The macro _AOUTHSZ_SHORT defines the length of such a header.

As I replied in the next comment, the value of _AOUTHSZ_SHORT here is 28.
Well, changing AuxFileHeaderSizeObj to AuxFileHeaderSizeShort might be more appropriate?

llvm/lib/MC/XCOFFObjectWriter.cpp

315

Thank you for your comments!
Generally, the auxiliary header should not be required by an un-executable object file. However, it may be needed in some specific cases, like the visibility case here.
Here is a related description in /usr/include/aouthdr.h of AIX.

In general, the auxiliary header can be any size, and the length of the auxiliary in any particular object file is specified in the f_opthdr field of the XCOFF header.
Some AIX programs generate auxiliary headers for 32-bit object files that end after the data_start field.  The macro _AOUTHSZ_SHORT defines the length of such a header.  No auxiliary header is required for an object file that is not an executable.

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

468

maybe more readable to change to
if (XSym->getVisibilityType() !=XCOFF::SYM_V_UNSPECIFIED)

771

since in function auxiliaryHeaderSize() , 64bit return size zero, can we use auxiliaryHeaderSize() here ?

794

since we can know the address and Size from stuct SectionEntry , do you want to put the value here instead of zero?

[Esme]

Address comments.

llvm/lib/MC/XCOFFObjectWriter.cpp

794

I may prefer to post another patch for calculating the start addresses and the total sizes for Text/Data/BSS sections since this patch comes from the visibility requirement and these fields are not required.

[Esme]

Addressed comments.
Hi @DiggerLin, please ignore my previous comments.
Since those values of size and address are easy to obtain, I've filled them for the aux header in this revision.

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

315

add some comment here :
// TODO: not support 64-bit auxiliary header.

784

since we only implement the 32-bit auxiliary header here
it is better to add a new function

writeAuxFileHeader32() {
    W.write<uint16_t>(0); // Magic
   ... 
}

and copy the code into the function.

 void XCOFFObjectWriter::writeAuxFileHeader() {
   if (!auxiliaryHeaderSize())
    return;
   if(!is64Bit()) 
     writeAuxFileHeader32();
  //TODO : 64-bit auxiliary header not implemented.   
}

[DiggerLin]

llvm/test/CodeGen/PowerPC/aix-xcoff-visibility.ll

7

change to --check-prefixes=SYM,AUX32 ?

10

change to --check-prefixes=SYM,AUX64 ?

[Esme]

Addressed.

llvm/lib/MC/XCOFFObjectWriter.cpp

315

As the definition in aouthdr.h, 64-bit object files will not have the auxiliary header. So this should not be a TODO?

[jhenderson]

llvm/test/CodeGen/PowerPC/aix-aux-header.ll

2

This test is failing in the pre-merge checks. Please make sure to fix it.

[Esme]

@jhenderson Thanks for finding it.

[DiggerLin]

llvm/lib/MC/XCOFFObjectWriter.cpp

315

thanks for the clarify. do you want to add some comment for it (64-bit object files will not have the auxiliary header)?

[Esme]

Add a comment about 64-bit object has no aux header.

[DiggerLin]

Look good to me. Maybe James Henderson has further comment.

llvm/lib/MC/XCOFFObjectWriter.cpp

315

nit: move comment in side the function

// 64-bit object files have no auxiliary header.
 return HasVisibility && !is64Bit() ? XCOFF::AuxFileHeaderSizeShort : 0;

[jhenderson]

I've got nothing else to add. LGTM.

llvm/include/llvm/BinaryFormat/XCOFF.h

 // Constants used in the XCOFF definition.
 
 constexpr size_t FileNamePadSize = 6;
 constexpr size_t NameSize = 8;
 constexpr size_t FileHeaderSize32 = 20;
 constexpr size_t FileHeaderSize64 = 24;
 constexpr size_t AuxFileHeaderSize32 = 72;
 constexpr size_t AuxFileHeaderSize64 = 110;
+constexpr size_t AuxFileHeaderSizeShort = 28;
 constexpr size_t SectionHeaderSize32 = 40;
 constexpr size_t SectionHeaderSize64 = 72;
 constexpr size_t SymbolTableEntrySize = 18;
 constexpr size_t RelocationSerializationSize32 = 10;
 constexpr size_t RelocationSerializationSize64 = 14;
 constexpr uint16_t RelocOverflow = 65535;
 constexpr uint8_t AllocRegNo = 31;
 

llvm/lib/MC/XCOFFObjectWriter.cpp

 
 class XCOFFObjectWriter : public MCObjectWriter {
 
   uint32_t SymbolTableEntryCount = 0;
   uint64_t SymbolTableOffset = 0;
   uint16_t SectionCount = 0;
   uint64_t RelocationEntryOffset = 0;
   std::vector<std::pair<std::string, size_t>> FileNames;
+  bool HasVisibility = false;
 
   support::endian::Writer W;
   std::unique_ptr<MCXCOFFObjectTargetWriter> TargetObjectWriter;
   StringTableBuilder Strings;
 
   const uint64_t MaxRawDataSize =
       TargetObjectWriter->is64Bit() ? UINT64_MAX : UINT32_MAX;
 
void writeSymbolEntryForCsectMemberLabel(const Symbol &SymbolRef,
                                            int16_t SectionIndex,
                                            uint64_t SymbolOffset);
   void writeSymbolEntryForControlSection(const XCOFFSection &CSectionRef,
                                          int16_t SectionIndex,
                                          XCOFF::StorageClass StorageClass);
   void writeSymbolEntryForDwarfSection(const XCOFFSection &DwarfSectionRef,
                                        int16_t SectionIndex);
   void writeFileHeader();
+  void writeAuxFileHeader();
   void writeSectionHeaderTable();
   void writeSections(const MCAssembler &Asm, const MCAsmLayout &Layout);
   void writeSectionForControlSectionEntry(const MCAssembler &Asm,
                                           const MCAsmLayout &Layout,
                                           const CsectSectionEntry &CsectEntry,
                                           uint64_t &CurrentAddressLocation);
   void writeSectionForDwarfSectionEntry(const MCAssembler &Asm,
                                         const MCAsmLayout &Layout,
class XCOFFObjectWriter : public MCObjectWriter {
   // *) Calculates physical/virtual addresses, raw-pointer offsets, and section
   //    sizes.
   // *) Assigns symbol table indices.
   // *) Builds up the section header table by adding any non-empty sections to
   //    `Sections`.
   void assignAddressesAndIndices(const MCAsmLayout &);
   void finalizeSectionInfo();
 
-  // TODO aux header support not implemented.
-  bool needsAuxiliaryHeader() const { return false; }
-
-  // Returns the size of the auxiliary header to be written to the object file.
+  // 64-bit object files have no auxiliary header.
   size_t auxiliaryHeaderSize() const {
-    assert(!needsAuxiliaryHeader() &&
-           "Auxiliary header support not implemented.");
-    return 0;
+    return HasVisibility && !is64Bit() ? XCOFF::AuxFileHeaderSizeShort : 0;
   }
 
 public:
   XCOFFObjectWriter(std::unique_ptr<MCXCOFFObjectTargetWriter> MOTW,
                     raw_pwrite_stream &OS);
 
   void writeWord(uint64_t Word) {
     is64Bit() ? W.write<uint64_t>(Word) : W.write<uint32_t>(Word);
void XCOFFObjectWriter::executePostLayoutBinding(MCAssembler &Asm,
   for (const MCSymbol &S : Asm.symbols()) {
     // Nothing to do for temporary symbols.
     if (S.isTemporary())
       continue;
 
     const MCSymbolXCOFF *XSym = cast<MCSymbolXCOFF>(&S);
     const MCSectionXCOFF *ContainingCsect = getContainingCsect(XSym);
 
+    if (XSym->getVisibilityType() != XCOFF::SYM_V_UNSPECIFIED)
+      HasVisibility = true;
+
     if (ContainingCsect->getCSectType() == XCOFF::XTY_ER) {
       // Handle undefined symbol.
       UndefinedCsects.emplace_back(ContainingCsect);
       SectionMap[ContainingCsect] = &UndefinedCsects.back();
       if (nameShouldBeInStringTable(ContainingCsect->getSymbolTableName()))
         Strings.add(ContainingCsect->getSymbolTableName());
       continue;
     }
uint64_t XCOFFObjectWriter::writeObject(MCAssembler &Asm,
   // is incompatible with incremental linking of XCOFF.
   if (Asm.isIncrementalLinkerCompatible())
     report_fatal_error("Incremental linking not supported for XCOFF.");
 
   finalizeSectionInfo();
   uint64_t StartOffset = W.OS.tell();
 
   writeFileHeader();
+  writeAuxFileHeader();
   writeSectionHeaderTable();
   writeSections(Asm, Layout);
   writeRelocations();
   writeSymbolTable(Layout);
   // Write the string table.
   Strings.write(W.OS);
 
   return W.OS.tell() - StartOffset;
void XCOFFObjectWriter::writeSymbolEntry(StringRef SymbolName, uint64_t Value,
   if (is64Bit()) {
     W.write<uint64_t>(Value);
     W.write<uint32_t>(Strings.getOffset(SymbolName));
   } else {
     writeSymbolName(SymbolName);
     W.write<uint32_t>(Value);
   }
   W.write<int16_t>(SectionNumber);
-  // Basic/Derived type. See the description of the n_type field for symbol
-  // table entries for a detailed description. Since we don't yet support
-  // visibility, and all other bits are either optionally set or reserved, this
-  // is always zero.
-  if (SymbolType != 0)
-    report_fatal_error("Emitting non-zero visibilities is not supported yet.");
   // TODO Set the function indicator (bit 10, 0x0020) for functions
   // when debugging is enabled.
   W.write<uint16_t>(SymbolType);
   W.write<uint8_t>(StorageClass);
   W.write<uint8_t>(NumberOfAuxEntries);
 }
 
 void XCOFFObjectWriter::writeSymbolAuxCsectEntry(uint64_t SectionOrLength,

 }
 
 void XCOFFObjectWriter::writeFileHeader() {
   W.write<uint16_t>(is64Bit() ? XCOFF::XCOFF64 : XCOFF::XCOFF32);
   W.write<uint16_t>(SectionCount);
   W.write<int32_t>(0); // TimeStamp
   writeWord(SymbolTableOffset);
   if (is64Bit()) {
-    W.write<uint16_t>(0); // AuxHeaderSize. No optional header for an object
-                          // file that is not to be loaded.
+    W.write<uint16_t>(auxiliaryHeaderSize());
     W.write<uint16_t>(0); // Flags
     W.write<int32_t>(SymbolTableEntryCount);
   } else {
     W.write<int32_t>(SymbolTableEntryCount);
-    W.write<uint16_t>(0); // AuxHeaderSize. No optional header for an object
-                          // file that is not to be loaded.
+    W.write<uint16_t>(auxiliaryHeaderSize());
     W.write<uint16_t>(0); // Flags
   }
 }
 
+void XCOFFObjectWriter::writeAuxFileHeader() {
+  if (!auxiliaryHeaderSize())
+    return;
+  W.write<uint16_t>(0); // Magic
+  W.write<uint16_t>(
+      XCOFF::NEW_XCOFF_INTERPRET); // Version. The new interpretation of the
+                                   // n_type field in the symbol table entry is
+                                   // used in XCOFF32.
+  W.write<uint32_t>(Sections[0]->Size);    // TextSize
+  W.write<uint32_t>(Sections[1]->Size);    // InitDataSize
+  W.write<uint32_t>(Sections[2]->Size);    // BssDataSize
+  W.write<uint32_t>(0);                    // EntryPointAddr
+  W.write<uint32_t>(Sections[0]->Address); // TextStartAddr
+  W.write<uint32_t>(Sections[1]->Address); // DataStartAddr
+}
+
 void XCOFFObjectWriter::writeSectionHeaderTable() {
   auto writeSectionHeader = [&](const SectionEntry *Sec, bool IsDwarf) {
     // Nothing to write for this Section.
     if (Sec->Index == SectionEntry::UninitializedIndex)
       return false;
 
     // Write Name.
     ArrayRef<char> NameRef(Sec->Name, XCOFF::NameSize);

llvm/test/CodeGen/PowerPC/aix-aux-header.ll

+; RUN: llc -filetype=obj -mtriple=powerpc-ibm-aix-xcoff %s -o - | \
+; RUN:   llvm-readobj --auxiliary-header - | FileCheck %s
+
+; CHECK:      AuxiliaryHeader {
+; CHECK-NEXT:   Magic: 0x0
+; CHECK-NEXT:   Version: 0x2
+; CHECK-NEXT:   Size of .text section: 0x24
+; CHECK-NEXT:   Size of .data section: 0x14
+; CHECK-NEXT:   Size of .bss section: 0x0
+; CHECK-NEXT:   Entry point address: 0x0
+; CHECK-NEXT:   .text section start address: 0x0
+; CHECK-NEXT:   .data section start address: 0x24
+; CHECK-NEXT: }
+
+@var = hidden global i32 0, align 4
+
+define hidden i32 @fun() {
+entry:
+  %0 = load i32, i32* @var, align 4
+  ret i32 %0
+}

llvm/test/CodeGen/PowerPC/aix-xcoff-visibility.ll

 ; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 -mattr=-altivec -data-sections=false < %s | \
 ; RUN:   FileCheck %s
 ; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -mcpu=pwr4 -mattr=-altivec -data-sections=false < %s |\
 ; RUN:   FileCheck %s
 
-; RUN: not --crash llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff \
-; RUN:     -filetype=obj -o %t.o < %s 2>&1 | \
-; RUN:   FileCheck --check-prefix=XCOFF %s
-
-; RUN: not --crash llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff \
-; RUN:     -filetype=obj -o %t.o 2>&1 < %s 2>&1 | \
-; RUN:   FileCheck --check-prefix=XCOFF %s
+; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -filetype=obj %s -o %t.o
+; RUN: llvm-readobj --syms --auxiliary-header %t.o | FileCheck %s --check-prefixes=SYM,AUX32
 
-; XCOFF: LLVM ERROR: Emitting non-zero visibilities is not supported yet.
+; RUN: llc -verify-machineinstrs -mtriple powerpc64-ibm-aix-xcoff -filetype=obj %s -o %t64.o
+; RUN: llvm-readobj --syms --auxiliary-header %t64.o | FileCheck %s --check-prefixes=SYM,AUX64
 
 @b =  global i32 0, align 4
 @b_h = hidden global i32 0, align 4
 @b_e = dllexport global i32 0, align 4
 
 define void @foo() {
 entry:
   ret void

 ; CHECK:        .weak   .zoo_weak_extern_h[PR],hidden
 ; CHECK:        .weak   zoo_weak_extern_h[DS],hidden
 ; CHECK:        .weak   .zoo_weak_extern_e[PR],exported
 ; CHECK:        .weak   zoo_weak_extern_e[DS],exported
 ; CHECK:        .extern .bar_h[PR],hidden
 ; CHECK:        .extern bar_h[DS],hidden
 ; CHECK:        .extern .bar_e[PR],exported
 ; CHECK:        .extern bar_e[DS],exported
+
+; AUX32:       AuxiliaryHeader {
+; AUX32-NEXT:    Magic: 0x0
+; AUX32-NEXT:    Version: 0x2
+; AUX32-NEXT:    Size of .text section: 0x134
+; AUX32-NEXT:    Size of .data section: 0x6C
+; AUX32-NEXT:    Size of .bss section: 0x0
+; AUX32-NEXT:    Entry point address: 0x0
+; AUX32-NEXT:    .text section start address: 0x0
+; AUX32-NEXT:    .data section start address: 0x134
+; AUX32-NEXT:  }
+
+; AUX64:       AuxiliaryHeader {
+; AUX64-NEXT:  }
+
+; SYM:         Name: .bar_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: zoo_weak_extern_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: .zoo_weak_extern_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: .zoo_weak_extern_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: zoo_weak_extern_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: bar_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .bar_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: bar_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: N_UNDEF
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .foo
+; SYM-NEXT:    Value (RelocatableAddress): 0x0
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x0
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .foo_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x1C
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .foo_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x3C
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .foo_protected
+; SYM-NEXT:    Value (RelocatableAddress): 0x5C
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x3000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: .foo_weak_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x84
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: .foo_weak_e
+; SYM-NEXT:    Value (RelocatableAddress): 0xA4
+; SYM-NEXT:    Section: .text
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: b
+; SYM-NEXT:    Value (RelocatableAddress): 0x134
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x0
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: b_h
+; SYM-NEXT:    Value (RelocatableAddress): 0x138
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: b_e
+; SYM-NEXT:    Value (RelocatableAddress): 0x13C
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: foo
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x0
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: foo_h
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: foo_e
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: foo_protected
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x3000
+; SYM-NEXT:    StorageClass: C_EXT (0x2)
+
+; SYM:         Name: foo_weak_h
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x2000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)
+
+; SYM:         Name: foo_weak_e
+; SYM-NEXT:    Value (RelocatableAddress): {{[0-9]+}}
+; SYM-NEXT:    Section: .data
+; SYM-NEXT:    Type: 0x4000
+; SYM-NEXT:    StorageClass: C_WEAKEXT (0x6F)