[AIX][XCOFF] parsing xcoff object file auxiliary header

[DiggerLin]

Summary

the patch supports parsing the xcoff object file auxiliary header with llvm-readobj with option "auxiliary-headers"

the format of auxiliary header as
https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/filesreference/XCOFF.html#XCOFF__fyovh386shar

the object file
llvm/test/tools/llvm-readobj/XCOFF/Inputs/xcoff-32-xlc-exec
llvm/test/tools/llvm-readobj/XCOFF/Inputs/xcoff-32-xlc-obj.o
llvm/test/tools/llvm-readobj/XCOFF/Inputs/xcoff-64-xlc-exec

are build from the source code with xlc
struct Point {
double x;
double y;
double z;
};

Point add_points(Point a, Point b) {
Point p;
p.x = a.x + b.x;
p.y = a.y + b.y;
p.z = a.z + b.z;
return p;
}

int main() {
Point a = {1.0, 3.0, 4.0};
Point b = {2.0, 8.0, 5.0};
Point c = add_points(a, b);
return 0;
}

[jasonliu]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

605

Why do you need to pass in AuxSize to the macro function when all inputs are the same?

698

We should print this as hex.

llvm/tools/llvm-readobj/llvm-readobj.cpp

90

I'm assuming we need to add it somewhere in the llvm docs about this new option.

[hubert.reinterpretcast]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

595

This strikes me as extremely hazardous. What if we get a length value that is reflective of a partial field?

605

AuxSize is modified by each macro(!) invocation...

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

187

thanks

llvm/lib/Object/XCOFFObjectFile.cpp

158

thanks

llvm/tools/llvm-readobj/XCOFFDumper.cpp

109

thanks

595

thanks

598

we print out the information based on the binary sequence of auxiliary header.
the same field is on different offset between the 32bit and 64 bits. it is difficult to implement in template.

for example.
o_tsize : Offset at 4 in 32bits , but Offset at 56 at 64bits.

605

for AuxSize is modified , I just make it looks like a function.

llvm/tools/llvm-readobj/llvm-readobj.cpp

90

thanks

[DiggerLin]

address comment

[hubert.reinterpretcast]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

593

This macro does not operate within the confines of what a function can do with respect to its caller (it can cause the caller to return early). I do not believe that using a function-like naming style is appropriate. I also do not believe that using such a macro for control flow is desirable.

You can encode a table (yes, a macro is okay for that) with much the same information:
(format, description, pointer-to-member, offset in the table past-the-end of the member)

and use that table in the place where this macro is being invoked.

595

We still have to build with C++14 compilers for the time being. Assigning a large 64-bit value to a 32-bit signed type is verboten. In any case, checking the table size against the last field of the table I described above would avoid this issue.

[DiggerLin]

address comment

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

593

for the function printNumber is a overload function. using a macro, the complie will determine which version of printNumber will be used when compile. if using a table, I think how to make the code call the correct overload version of printNumber based in the parameter type when running may be complicated.

595

thanks.

[hubert.reinterpretcast]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

593

It can be solved using pointers-to-member in the table (I guess we don't really want to use those for selecting functions), but the macro is in better shape anyway now; thanks.

594

You can use XCOFFAuxiliaryHeader32::T in the sizeof.

604

Since the macro refers to it, move the macro definition into the scope of AuxiSize.

632

For symmetry, move the undef into the function body.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

594

thanks

604

thanks

632

thanks

[DiggerLin]

address comment

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

239

I believe that the name @jasonliu suggested is better:
s/XCOFF64bitsFlag/XCOFF64Flags/;

llvm/tools/llvm-readobj/XCOFFDumper.cpp

609

The use of capitalization is odd. Even for title case, "of" is not capitalized in such a position.
To avoid this issue, please use lowercase following the first word (except for abbreviations or proper nouns). Please apply for all of the strings.

633

We interpreted the packed field for the symbol alignment and type in the similar case of csect auxiliary symbol table entries. For consistency within the tool, I believe we should do it here as well.

635

We should produce some warning if the auxiliary table size indicates additional content past the last field that the code understands. This is another case where I think we should also output the raw data.

653

Please indent the body of the if. Also, if AuxiSize is greater than the offset of the field but less than the offset past-the-end of the field, then we have a partial field and should probably produce some warning about it (in case the producer of the input file has behaviour we did not expect). I would also suggest to output the raw data in that case.

684

The adjective should be "64-bit". Also, these are only the additional XCOFF64 flags (and not all of the flags for XCOFF64).

Suggestion:
Additional flags for 64-bit XCOFF

[jasonliu]

llvm/docs/CommandGuide/llvm-readobj.rst

314

nit: to match the naming convention in this file.
XCOFF SPECIFIC OPTIONS

llvm/include/llvm/Object/XCOFFObjectFile.h

239

nit: If we want to keep the bit part. Then XCOFF64BitFlag.

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

4

Should llvm-readobj --all print out the auxiliary headers?

llvm/tools/llvm-readobj/XCOFFDumper.cpp

593

nit: Address clang-format comment.

684

nit:
64 bits -> 64-bit

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

653

I agree your suggestion that "we have a partial field and should probably produce some warning about it " . If I implement the suggestion in the the patch, I also need to add test case to test it. It maybe cause the patch too big, I think it maybe better to put the suggestion in other patch.

[hubert.reinterpretcast]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

653

I would suggest implementing it in this patch and to post another patch (while this review is still up) with the specific testing for the error cases of having partial fields or trailing unrecognized fields.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

653

without the test case , I do not think we can 100% guarantee the code is correct.
if when we write a test case for it. we find an error on the code, we would put the fix and specific test case in the some patch?

[hubert.reinterpretcast]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

653

Well, if the original patch has not already landed, I do not see why we wouldn't apply the fix in the original patch. As for the testing, I think it should be fine to have it in a separate patch that is intended to land at around the same time.

[DiggerLin]

deal with partial field data

[DiggerLin]

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

4

there are several other options are not implemented, -all will be crashed.

[jasonliu]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

635

This is marked as Done. But I don't think we are able to print additional content under current revision. But anyway, I would expect this scenario would get added in the test case (which belong to another patch).

645

This field does not appear to have partial field treatment.

[jasonliu]

llvm/include/llvm/Object/XCOFFObjectFile.h

162

This block currently cut in between XCOFFSectionHeader code block.
I would suggest to move the new block to the current line 51.
Also, is it worth to consider taking the XCOFFSectionHeader template approach now? Just like XCOFFSectionHeader, we have same member functions and share the same mask between 32 and 64 bit.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

645

only more one bytes has partial field treatment, the flag only one byte.

[DiggerLin]

address comment

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

635

I think the code , print out the additional content.

if f (sizeof(XCOFFAuxiliaryHeader32) < AuxiSize) {

W.printBinary(
    "!!!Warning",
    "There are extra data beyond XCOFFAuxiliaryHeader32. Extra raw data",
    ArrayRef<uint8_t>((const uint8_t *)(AuxHeader) +
                          sizeof(XCOFFAuxiliaryHeader32),
                      AuxiSize - sizeof(XCOFFAuxiliaryHeader32)));

Yes, I will test additional content in another patch.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

635

there are already has additional content auxiliary data in the test case
xcoff-auxiliary-header.test

XLC64EXEC-NEXT: !!!Warning: There are extra data beyond XCOFFAuxiliaryHeader64. Extra raw data (00 00 00 00 00 00 00 00 00 00)

[DiggerLin]

Using crtp to implement XCOFFAuxiliaryHeader32 and XCOFFAuxiliaryHeader64

[sfertile]

llvm/docs/CommandGuide/llvm-readobj.rst

314

Can you update this to reflect Jasons comment please.

[Esme]

FYI. I am trying to add yaml2obj support for auxFileHeader to avoid using binary files as input in the tests. However, I am about to take a vacation and if I can't get it done before this patch is submitted, I will modify the tests in follow-up works.

[DiggerLin]

rebase the option --auxiliary-header option on the new option mechanism. and address comment.

[DiggerLin]

In D82549#3032815, @Esme wrote:

FYI. I am trying to add yaml2obj support for auxFileHeader to avoid using binary files as input in the tests. However, I am about to take a vacation and if I can't get it done before this patch is submitted, I will modify the tests in follow-up works.

thanks, Esme.

[jhenderson]

llvm/docs/CommandGuide/llvm-readobj.rst

315

The underline here needs to match the title length or I think you'll get an error when building the doc.

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

2

Even though it's not currently required, I'd suggest using ## for comments and # for lit & FileCheck directives in this file, as likely at some point it'll contain YAML, and then you'd have to change every line in the test.

4

I don't think that should prevent it being added at this point (although you wouldn't be able to test it). That being said, do a check of other format-specific options to see if --all dumps them.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

596

Are you sure you want to print something? I think for other formats, we often just don't do anything, or print an empty dictionary/set-like thing. E.g. what happens if you do --segments with an ELF object file?

599

I think AuxSize would be a more normal abbreviation, and would therefore be less likely to trip people up when typing.

601

I recommend explicitly initialising this to nullptr to avoid any risk of using an uninitialized variable.

605

Could this not just be a lambda? That would play friendlier with IDEs if nothing else.

633

CPU is an acronym, so should be all caps.

652

Delete the blank line after this comment. Also, I'd suggest you change it to something like "Handle unknown/unsupported data." or something like that.

653

Testing should be in the same patch as the code it is testing. Please don't split them into separate patches. If the functionality can be added standalone, and there's a sensible behaviour (even if that behaviour is "do nothing") without it, then that functionality and testing should be in a separate patch.

llvm/tools/llvm-readobj/llvm-readobj.cpp

275

Don't forget to clang-format your modified code.

[jhenderson]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

655–670

This is a mess, in my opinion. Please use reportWarning/reportUniqueWarning to report the warning as per other warnings in llvm-readobj. Printing the other raw data is fine, but should be prefixed with simply something like "Raw data from offset 0x1234:" rather than the ugly "!!!Warning".

678–681

See my above comment for the 32-bit case.

If possible, it would be good though if this check (whether it prints or not) was in some higher-level function that then calls this and the 32-bit function, rather than duplicating it.

682

As above.

684

= nullptr; as suggested above.

686

This is common between the two functions: consider moving it into a higher-level calling function, along with the check above.

688

Same as before: make this a lambda.

735

Same as before: use reportWarning/reportUniqueWarning to report warnings.

llvm/tools/llvm-readobj/llvm-readobj.cpp

100

File-format specific options are grouped separately to the generic options (see below for examples).

[Esme]

Tests required for this patch can now be generated with D111487:

1. dumping full contents of the aux file header for XCOFF32 and XCOFF64.
2. reporting the warning message for partial field by specifying the value of AuxiliaryHeaderSize in the FileHeader, ex.

--- !XCOFF
FileHeader:
  MagicNumber:         0x1DF
  AuxiliaryHeaderSize: 71
AuxiliaryHeader:
  Magic: 0x10B
...

3. reporting the warning message for extra data by set the value of AuxiliaryHeaderSize bigger than AuxFileHeaderSize32/64, ex.

--- !XCOFF
FileHeader:
  MagicNumber:         0x1DF
  AuxiliaryHeaderSize: 75
AuxiliaryHeader:
  Magic: 0x10B

4. dumping empty AuxiliaryHeader {} if AuxiliaryHeader is not set.

However, tests of this patch and tests of D111487 are interdependent. We just need to mark a TODO in this patch, and replace the tests after D111487 has landed too.

[Esme]

llvm/lib/Object/XCOFFObjectFile.cpp

1045

See D110320, it is better to report a detailed error message, i.g
auxiliary header with offset ** and size ** goes past the end of the file.

[Esme]

llvm/tools/llvm-readobj/Opts.td

29

To be consistent with the spec llvm/docs/CommandGuide/llvm-readobj.rst, please move it to XCOFF specific options with the title OPTIONS (XCOFF specific).

[DiggerLin]

In D82549#3054179, @Esme wrote:

Tests required for this patch can now be generated with D111487:

1. dumping full contents of the aux file header for XCOFF32 and XCOFF64.
2. reporting the warning message for partial field by specifying the value of AuxiliaryHeaderSize in the FileHeader, ex.

--- !XCOFF
FileHeader:
  MagicNumber:         0x1DF
  AuxiliaryHeaderSize: 71
AuxiliaryHeader:
  Magic: 0x10B
...

3. reporting the warning message for extra data by set the value of AuxiliaryHeaderSize bigger than AuxFileHeaderSize32/64, ex.

--- !XCOFF
FileHeader:
  MagicNumber:         0x1DF
  AuxiliaryHeaderSize: 75
AuxiliaryHeader:
  Magic: 0x10B

4. dumping empty AuxiliaryHeader {} if AuxiliaryHeader is not set.

However, tests of this patch and tests of D111487 are interdependent. We just need to mark a TODO in this patch, and replace the tests after D111487 has landed too.

thanks for your work.

llvm/docs/CommandGuide/llvm-readobj.rst

315

thanks

llvm/lib/Object/XCOFFObjectFile.cpp

1045

when we parses the FileHeader (line 894~897) , Section header, symbol table etc in the function. they use the same mechanism to deal with error as AuxiliaryHeader . If you strong suggestion that we need to report the detail error message , we can create a separate patch to deal with all the parse error at the same time after the patch landed.

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

2

thanks

4

if add --all option in the test, it will invoke the function

if (opts::UnwindInfo)
   Dumper->printUnwindInfo();

and for XCOFF. XCOFFDumper::printUnwindInfo() not implement

void XCOFFDumper::printUnwindInfo() {
  llvm_unreachable("Unimplemented functionality for XCOFFDumper");
}

It will be crash.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

596

I think print out some info will let user know what happen on the xcoff object file.

599

Ok, thanks

605

the T is a member of class XCOFFDumper.cpp , I do not think C++11 support a template lambda.

678–681

I think it is difficult . we call the function here

void XCOFFDumper::printAuxiliaryHeader() {
  if (Obj.is64Bit())
    printAuxiliaryHeader(Obj.auxiliaryHeader64());
  else
    printAuxiliaryHeader(Obj.auxiliaryHeader32());
}

686

I do not think I can not

DictScope DS(W, "AuxiliaryHeader");

in higher level,
if I put in the higher level as

void XCOFFDumper::printAuxiliaryHeader() {
  DictScope DS(W, "AuxiliaryHeader");
  if (Obj.is64Bit())
    printAuxiliaryHeader(Obj.auxiliaryHeader64());
  else
    printAuxiliaryHeader(Obj.auxiliaryHeader32());
}

it will always print out the "AuxiliaryHeader" even the (AuxHeader == nullptr)

688

same comment as before.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

653

added a new test for this.

[Esme]

llvm/tools/llvm-readobj/Opts.td

87

llvm/tools/llvm-readobj/XCOFFDumper.cpp

54

I noticed that the the two functions are very similar, can we make these common to avoid code duplication? As far as I know, almost all members are the same for 32-bit and 64-bit except for "Additional flags 64-bit XCOFF", XCOFF64Flag.
I.e. use template <typename AuxT> void printAuxiliaryHeader(AuxT *AuxHeader); to replace them.

[DiggerLin]

llvm/tools/llvm-readobj/Opts.td

87

thanks

llvm/tools/llvm-readobj/XCOFFDumper.cpp

54

I have think it over before, But I do not think it is possible to use a template to do it. because the member (and the order of the members) of XCOFFAuxiliaryHeader64 and XCOFFAuxiliaryHeader32 are different. and we need to print out the info based on the order which defined in the https://www.ibm.com/docs/en/aix/7.2?topic=formats-xcoff-object-file-format#XCOFF__fyovh386shar

[Esme]

I have no more comments. But please wait and see if James @jhenderson has other comments.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

54

Thanks.
In fact, I wouldn't require members to be printed strictly in the order of their offsets. But, of course, printing in order also makes sense to me.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

54

thanks

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

54

I keep to print out the member strictly in the order of their offsets. it is more easy to read and figure out the error if there is errors when we encode of the xcoff object

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

686

sorry. it should be "I do not think I can" in above message

[DiggerLin]

@jhenderson , @Esme , is there any more comments on the patch, if not ,I am very appreciated that anyone of you can approve it?

[Esme]

LGTM with small nits.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

597

Remove braces since it's a single-line if.

683

Ditto.

llvm/tools/llvm-readobj/llvm-readobj.cpp

354

Ditto.

[thakis]

This breaks tests on Windows: http://45.33.8.238/win/47662/step_11.txt

Please take a look and revert for now if it takes a while to fix.

[jhenderson]

I'm sorry I didn't come back to this - I've been struggling under an excessive review burden, and then had last week as PTO.

However, given my comments, I'd have preferred you had held off committing this a little longer. I don't believe this code is within acceptable LLVM quality at this time, and given the problem mentioned by @thakis, I think this should be reverted until the comments are addressed and the test fixed properly. (As far as I can tell, there is no reason why the test should not work on Windows, apart from some bug in this code - rGc2d2fb509306618b982bff94e1ad9acff6a41bcf appears to only be a temporary fix to get the build bot green again).

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

4

llvm_unreachable shouldn't be used where code is actually reachable. In such a case, it should just be a standard error message or similar (there are examples in LLVM/GNU style ELF output where one is implemented but not the other - look at those for the precedent). Not having a hard-error/crash would allow --all to be used.

125

Delete trailing blank line.

The final character at EOF should be \n, but not a character sequence of more than one \n.

llvm/tools/llvm-readobj/Opts.td

88

Existing help text style in llvm-readobj appears to be to have a leading capital (unlike error messages), so this should be "Display the auxiliary header".

llvm/tools/llvm-readobj/XCOFFDumper.cpp

605

Not sure I quite follow, but note that LLVM supports C++14 and auto can be used to much the same effect, if I'm not mistaken. I believe something like the following might work (note: I haven't tested, and am not particularly familiar with auto lambdas).

enum PrintStyle { Hex, Number };
auto PrintAuxMember32 = [&](PrintStyle P, StringRef S, auto *T) {
  ptrdiff_t Offset = T - AuxHeader;
  if (Offset + sizeof(*T) <= AuxSize) {
    switch(P) {
    case Hex:
      W.printHex(S, *T);
      break;
    case Number:
      W.printNumber(S, *T);
      break;
    }
  } else if (Offset < AuxSize) {
    PartialFieldOffset = Offset;
    PartialFieldName = S;
  }
}; 

PrintAuxMember32(Hex, "Magic", &AuxHeader->AuxMagic);
PrintAuxMember32(Hex, "Version", &AuxHeader->Version);
// etc...

You might be able to avoid the enum using some sort of std::bind call or similar, which would be nicer, but not essential.

Even without that, I think you should factor out the repeated offsetof calls into a variable, and use more meaningful variable names rather than H, S and T, just like you would for any function.

652

You ignored the second part of this comment...

654–655

Please refer to the LLVM coding standards for proper formatting of warning and error messages: such messages should start with a lower-case letter and not end with a full stop.

You don't need a string stream here, either. Just do:

std::string ErrInfo = (Twine("only partial field for") + PartialFieldName + " at offset (" + PartialFieldOffset)").str();

657–659

1. Use createError, not make_error. See existing usage throughout llvm-readobj. Same applies in all other usages of make_error.
2. Why are you using reportWarning(..., "-"); That reports a warning as if the error was in a file read in from stdin, which in many cases will be incorrect. a) We encourage using reportUniqueWarning in new llvm-readobj code, so that the same warning isn't reported multiple times; b) use the file name for the last argument, like it should be. Same below.

666

686

I think that's fine. We do similar things in other cases in ELF, see e.g. ELFDumper<ELFT>::printHashTable. It shows there is no content relevant to the AuxHeader printing, even though you asked for it.

735

This block looks very similar to the equivalent 32-bit version. Could you move it into a function?

[jhenderson]

@DiggerLin, have you seen my post-commit review?

[DiggerLin]

In D82549#3108279, @jhenderson wrote:

@DiggerLin, have you seen my post-commit review?

I saw it , I will address the comment.

[DiggerLin]

As far as I can tell, there is no reason why the test should not work on Windows, apart from some bug in this code - rGc2d2fb509306618b982bff94e1ad9acff6a41bcf appears to only be a temporary fix to get the build bot green again)

I think the fail in the window is related to
llvm-readobj --auxiliary-header %p/Inputs/xcoff-64-xlc-exec 2>&1

C:\src\llvm-project\llvm\test\tools\llvm-readobj\XCOFF\xcoff-auxiliary-header.test:91:11: error: WARN64: expected string not found in input
\# WARN64: {{.*}}llvm-readobj: warning: '<stdin>': There are extra data beyond auxiliary header

the window can not deal with 2>&1 correctly as linux (or aix) .
so I add a guardian
\# REQUIRES: system-aix
is OK. do you have any good solution ? @jhenderson

[jhenderson]

In D82549#3111784, @DiggerLin wrote:

As far as I can tell, there is no reason why the test should not work on Windows, apart from some bug in this code - rGc2d2fb509306618b982bff94e1ad9acff6a41bcf appears to only be a temporary fix to get the build bot green again)

I think the fail in the window is related to
llvm-readobj --auxiliary-header %p/Inputs/xcoff-64-xlc-exec 2>&1

C:\src\llvm-project\llvm\test\tools\llvm-readobj\XCOFF\xcoff-auxiliary-header.test:91:11: error: WARN64: expected string not found in input
\# WARN64: {{.*}}llvm-readobj: warning: '<stdin>': There are extra data beyond auxiliary header

the window can not deal with 2>&1 correctly as linux (or aix) .
so I add a guardian
\# REQUIRES: system-aix
is OK. do you have any good solution ? @jhenderson

2>&1 works fine in all other tests that use it (it is commonly used for testing errors), so it won't ever be that. I ran this test locally, and it was clear what the problem was: your warning check starts with {{.*}}llvm-readobj: but on Windows, the executable name is llvm-readobj.exe, so the ".exe" doesn't match and the test fails. Adding (.exe)? would fix this, although I believe we usually don't bother checking the tool name as part of the warning/error message (take a look at other test cases to see what we typically check for llvm-readobj errors).

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

1

You can remove xcoff from the test name, as it's in the XCOFF folder. The xcoff bit of the name is superfluous.

Same goes for the input files - no need for a leading xcoff.

[DiggerLin]

@jhenderson , I am so sorry that delay so long to address comment, when I tried to revert the patch, but I found patch https://reviews.llvm.org/D111487 "[XCOFF][yaml2obj] support for the auxiliary file header." is depend on the patch and it commit. if I revert the patch , I have to revert the patch D111487 too.
Base on current situation , it maybe better to create a NFC patch to address your comments (after commit) for the patch ? what is your suggestion ?

[jhenderson]

In D82549#3143604, @DiggerLin wrote:

@jhenderson , I am so sorry that delay so long to address comment, when I tried to revert the patch, but I found patch https://reviews.llvm.org/D111487 "[XCOFF][yaml2obj] support for the auxiliary file header." is depend on the patch and it commit. if I revert the patch , I have to revert the patch D111487 too.
Base on current situation , it maybe better to create a NFC patch to address your comments (after commit) for the patch ? what is your suggestion ?

Creating a new patch would be acceptable, yes. I'd suggest that, instead of additional reverts.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

605

there is an article about the auto for c++14. https://solarianprogrammer.com/2014/08/21/cpp-14-auto-tutorial/

G++ 4.9.1 also supports unconstrained generic functions, basically you can use auto in a function parameter list, so the above code can be further simplified as :

1 auto& add_one(auto& v) {
2 for(auto& it : v) {
3 it += 1;
4 }
5 return v;
6 }
7
8 void multiply_by_two(auto& v) {
9 for(auto& it : v) {
10 it *= 2;
11 }
12 }
unfortunately, this didn’t entered in the final C++14 standard. It is expected to be added later to the standard as a technical report.

I test the code in the clang,
the 'auto' not allowed in function prototype

@jhenderson

[jhenderson]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

605

You've missed where I've said specifically about lambdas. Please see https://en.cppreference.com/w/cpp/language/lambda. Specifically, look for "generic lambdas". Generic lambdas can take auto as type parameters, and are essentially equivalent to a templated function with the type specified as a template parameter (templated lambdas aren't in the C++14 standard, but generic lambdas mean they often aren't needed). I am not talking about auto in a function parameter list.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

605

thanks

735

create a help function.

[DiggerLin]

created a NFC patch for above comment https://reviews.llvm.org/D116220

[Esme]

llvm/include/llvm/Object/XCOFFObjectFile.h

116

The typename should be XCOFFAuxiliaryHeader64.
Sorry for not catching this error before.

llvm/docs/CommandGuide/llvm-readobj.rst

 .. option:: --coff-load-config
 
  Display the load config.
 
 .. option:: --coff-resources
 
  Display the .rsrc section.
 
+XCOFF SPECIFIC OPTIONS
+----------------------
+
+The following options are implemented only for the XCOFF file format.
+
+.. option:: --auxiliary-header
+
+  Display XCOFF Auxiliary header.
+
 EXIT STATUS
 -----------
 
 :program:`llvm-readobj` returns 0 under normal operation. It returns a non-zero
 exit code if there were any errors.
 
 SEE ALSO
 --------
 
 :manpage:`llvm-nm(1)`, :manpage:`llvm-objdump(1)`, :manpage:`llvm-readelf(1)`

llvm/include/llvm/Object/XCOFFObjectFile.h

struct XCOFFFileHeader64 {
   support::big32_t TimeStamp;
 
   support::ubig64_t SymbolTableOffset; // File offset to symbol table.
   support::ubig16_t AuxHeaderSize;
   support::ubig16_t Flags;
   support::ubig32_t NumberOfSymTableEntries;
 };
 
+template <typename T> struct XCOFFAuxiliaryHeader {
+  static constexpr uint8_t AuxiHeaderFlagMask = 0xF0;
+  static constexpr uint8_t AuxiHeaderTDataAlignmentMask = 0x0F;
+
+public:
+  uint8_t getFlag() const {
+    return static_cast<const T *>(this)->FlagAndTDataAlignment &
+           AuxiHeaderFlagMask;
+  }
+  uint8_t getTDataAlignment() const {
+    return static_cast<const T *>(this)->FlagAndTDataAlignment &
+           AuxiHeaderTDataAlignmentMask;
+  }
+};
+
+struct XCOFFAuxiliaryHeader32 : XCOFFAuxiliaryHeader<XCOFFAuxiliaryHeader32> {
+  support::ubig16_t
+      AuxMagic; ///< If the value of the o_vstamp field is greater than 1, the
+                ///< o_mflags field is reserved for future use and it should
+                ///< contain 0. Otherwise, this field is not used.
+  support::ubig16_t
+      Version; ///< The valid values are 1 and 2. When the o_vstamp field is 2
+               ///< in an XCOFF32 file, the new interpretation of the n_type
+               ///< field in the symbol table entry is used.
+  support::ubig32_t TextSize;
+  support::ubig32_t InitDataSize;
+  support::ubig32_t BssDataSize;
+  support::ubig32_t EntryPointAddr;
+  support::ubig32_t TextStartAddr;
+  support::ubig32_t DataStartAddr;
+  support::ubig32_t TOCAnchorAddr;
+  support::ubig16_t SecNumOfEntryPoint;
+  support::ubig16_t SecNumOfText;
+  support::ubig16_t SecNumOfData;
+  support::ubig16_t SecNumOfTOC;
+  support::ubig16_t SecNumOfLoader;
+  support::ubig16_t SecNumOfBSS;
+  support::ubig16_t MaxAlignOfText;
+  support::ubig16_t MaxAlignOfData;
+  support::ubig16_t ModuleType;
+  uint8_t CpuFlag;
+  uint8_t CpuType;
+  support::ubig32_t MaxStackSize; ///< If the value is 0, the system default
+                                  ///< maximum stack size is used.
+  support::ubig32_t MaxDataSize;  ///< If the value is 0, the system default
+                                  ///< maximum data size is used.
+  support::ubig32_t
+      ReservedForDebugger; ///< This field should contain 0. When a loaded
+                           ///< program is being debugged, the memory image of
+                           ///< this field may be modified by a debugger to
+                           ///< insert a trap instruction.
+  uint8_t TextPageSize;  ///< Specifies the size of pages for the exec text. The
+                         ///< default value is 0 (system-selected page size).
+  uint8_t DataPageSize;  ///< Specifies the size of pages for the exec data. The
+                         ///< default value is 0 (system-selected page size).
+  uint8_t StackPageSize; ///< Specifies the size of pages for the stack. The
+                         ///< default value is 0 (system-selected page size).
+  uint8_t FlagAndTDataAlignment;
+  support::ubig16_t SecNumOfTData;
+  support::ubig16_t SecNumOfTBSS;
+};
+
+struct XCOFFAuxiliaryHeader64 : XCOFFAuxiliaryHeader<XCOFFAuxiliaryHeader32> {
+  support::ubig16_t AuxMagic;
+  support::ubig16_t Version;
+  support::ubig32_t ReservedForDebugger;
+  support::ubig64_t TextStartAddr;
+  support::ubig64_t DataStartAddr;
+  support::ubig64_t TOCAnchorAddr;
+  support::ubig16_t SecNumOfEntryPoint;
+  support::ubig16_t SecNumOfText;
+  support::ubig16_t SecNumOfData;
+  support::ubig16_t SecNumOfTOC;
+  support::ubig16_t SecNumOfLoader;
+  support::ubig16_t SecNumOfBSS;
+  support::ubig16_t MaxAlignOfText;
+  support::ubig16_t MaxAlignOfData;
+  support::ubig16_t ModuleType;
+  uint8_t CpuFlag;
+  uint8_t CpuType;
+  uint8_t TextPageSize;
+  uint8_t DataPageSize;
+  uint8_t StackPageSize;
+  uint8_t FlagAndTDataAlignment;
+  support::ubig64_t TextSize;
+  support::ubig64_t InitDataSize;
+  support::ubig64_t BssDataSize;
+  support::ubig64_t EntryPointAddr;
+  support::ubig64_t MaxStackSize;
+  support::ubig64_t MaxDataSize;
+  support::ubig16_t SecNumOfTData;
+  support::ubig16_t SecNumOfTBSS;
+  support::ubig16_t XCOFF64Flag;
+};
+
 template <typename T> struct XCOFFSectionHeader {
   // Least significant 3 bits are reserved.
   static constexpr unsigned SectionFlagsReservedMask = 0x7;
 
   // The low order 16 bits of section flags denotes the section type.
   static constexpr unsigned SectionFlagsTypeMask = 0xffffu;
 
 public:
   StringRef getName() const;
   uint16_t getSectionType() const;
   bool isReservedSectionType() const;
 };
 
 // Explicit extern template declarations.
 struct XCOFFSectionHeader32;
 struct XCOFFSectionHeader64;
 extern template struct XCOFFSectionHeader<XCOFFSectionHeader32>;
 extern template struct XCOFFSectionHeader<XCOFFSectionHeader64>;
 
 struct XCOFFSectionHeader32 : XCOFFSectionHeader<XCOFFSectionHeader32> {
   char Name[XCOFF::NameSize];
   support::ubig32_t PhysicalAddress;
   support::ubig32_t VirtualAddress;
   support::ubig32_t SectionSize;
   support::ubig32_t FileOffsetToRawData;
   support::ubig32_t FileOffsetToRelocationInfo;
   support::ubig32_t FileOffsetToLineNumberInfo;
   support::ubig16_t NumberOfRelocations;
   support::ubig16_t NumberOfLineNumbers;
   support::big32_t Flags;
 };
 
 struct XCOFFSectionHeader64 : XCOFFSectionHeader<XCOFFSectionHeader64> {
   char Name[XCOFF::NameSize];
   support::ubig64_t PhysicalAddress;
   support::ubig64_t VirtualAddress;
   support::ubig64_t SectionSize;
   support::big64_t FileOffsetToRawData;
   support::big64_t FileOffsetToRelocationInfo;
   support::big64_t FileOffsetToLineNumberInfo;
   support::ubig32_t NumberOfRelocations;
   support::ubig32_t NumberOfLineNumbers;
   support::big32_t Flags;
   char Padding[4];
 };
 
 struct LoaderSectionHeader32 {
   support::ubig32_t Version;
   support::ubig32_t NumberOfSymTabEnt;
   support::ubig32_t NumberOfRelTabEnt;
   support::ubig32_t LengthOfImpidStrTbl;
   support::ubig32_t NumberOfImpid;
   support::big32_t OffsetToImpid;
   support::ubig32_t LengthOfStrTbl;
   support::big32_t OffsetToStrTbl;
 };
 
 struct LoaderSectionHeader64 {
   support::ubig32_t Version;
   support::ubig32_t NumberOfSymTabEnt;
   support::ubig32_t NumberOfRelTabEnt;
   support::ubig32_t LengthOfImpidStrTbl;
   support::ubig32_t NumberOfImpid;
   support::ubig32_t LengthOfStrTbl;
struct XCOFFCsectAuxEnt32 {
   support::ubig32_t StabInfoIndex;
   support::ubig16_t StabSectNum;
 };
 
 struct XCOFFCsectAuxEnt64 {
   support::ubig32_t SectionOrLengthLowByte;
   support::ubig32_t ParameterHashIndex;
   support::ubig16_t TypeChkSectNum;
   uint8_t SymbolAlignmentAndType;
   XCOFF::StorageMappingClass StorageMappingClass;
   support::ubig32_t SectionOrLengthHighByte;
   uint8_t Pad;
   XCOFF::SymbolAuxType AuxType;
 };
 
 class XCOFFCsectAuxRef {
 public:

 struct XCOFFRelocation32 : XCOFFRelocation<llvm::support::ubig32_t> {};
 struct XCOFFRelocation64 : XCOFFRelocation<llvm::support::ubig64_t> {};
 
 class XCOFFSymbolRef;
 
 class XCOFFObjectFile : public ObjectFile {
 private:
   const void *FileHeader = nullptr;
+  const void *AuxiliaryHeader = nullptr;
   const void *SectionHeaderTable = nullptr;
 
   const void *SymbolTblPtr = nullptr;
   XCOFFStringTable StringTable = {0, nullptr};
 
   const XCOFFFileHeader32 *fileHeader32() const;
   const XCOFFFileHeader64 *fileHeader64() const;
 
   const XCOFFSectionHeader32 *sectionHeaderTable32() const;
   const XCOFFSectionHeader64 *sectionHeaderTable64() const;
   template <typename T> const T *sectionHeaderTable() const;
 
   size_t getFileHeaderSize() const;
   size_t getSectionHeaderSize() const;
 
   const XCOFFSectionHeader32 *toSection32(DataRefImpl Ref) const;
   const XCOFFSectionHeader64 *toSection64(DataRefImpl Ref) const;
   uintptr_t getSectionHeaderTableAddress() const;
   uintptr_t getEndOfSymbolTableAddress() const;
   Expected<uintptr_t> getLoaderSectionAddress() const;
public:
   SubtargetFeatures getFeatures() const override;
   Expected<uint64_t> getStartAddress() const override;
   StringRef mapDebugSectionName(StringRef Name) const override;
   bool isRelocatableObject() const override;
 
   // Below here is the non-inherited interface.
   bool is64Bit() const;
 
+  const XCOFFAuxiliaryHeader32 *auxiliaryHeader32() const;
+  const XCOFFAuxiliaryHeader64 *auxiliaryHeader64() const;
+
   const void *getPointerToSymbolTable() const { return SymbolTblPtr; }
 
   Expected<StringRef> getSymbolSectionName(XCOFFSymbolRef Ref) const;
   unsigned getSymbolSectionID(SymbolRef Sym) const;
   XCOFFSymbolRef toSymbolRef(DataRefImpl Ref) const;
 
   // File header related interfaces.
   uint16_t getMagic() const;

llvm/lib/Object/XCOFFObjectFile.cpp

const XCOFFFileHeader32 *XCOFFObjectFile::fileHeader32() const {
   return static_cast<const XCOFFFileHeader32 *>(FileHeader);
 }
 
 const XCOFFFileHeader64 *XCOFFObjectFile::fileHeader64() const {
   assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
   return static_cast<const XCOFFFileHeader64 *>(FileHeader);
 }
 
+const XCOFFAuxiliaryHeader32 *XCOFFObjectFile::auxiliaryHeader32() const {
+  assert(!is64Bit() && "32-bit interface called on 64-bit object file.");
+  return static_cast<const XCOFFAuxiliaryHeader32 *>(AuxiliaryHeader);
+}
+
+const XCOFFAuxiliaryHeader64 *XCOFFObjectFile::auxiliaryHeader64() const {
+  assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
+  return static_cast<const XCOFFAuxiliaryHeader64 *>(AuxiliaryHeader);
+}
+
 template <typename T> const T *XCOFFObjectFile::sectionHeaderTable() const {
   return static_cast<const T *>(SectionHeaderTable);
 }
 
 const XCOFFSectionHeader32 *
 XCOFFObjectFile::sectionHeaderTable32() const {
   assert(!is64Bit() && "32-bit interface called on 64-bit object file.");
   return static_cast<const XCOFFSectionHeader32 *>(SectionHeaderTable);
XCOFFObjectFile::create(unsigned Type, MemoryBufferRef MBR) {
   // Parse file header.
   auto FileHeaderOrErr =
       getObject<void>(Data, Base + CurOffset, Obj->getFileHeaderSize());
   if (Error E = FileHeaderOrErr.takeError())
     return std::move(E);
   Obj->FileHeader = FileHeaderOrErr.get();
 
   CurOffset += Obj->getFileHeaderSize();
-  // TODO FIXME we don't have support for an optional header yet, so just skip
-  // past it.
+
+  if (Obj->getOptionalHeaderSize()) {
+    auto AuxiliaryHeaderOrErr =
+        getObject<void>(Data, Base + CurOffset, Obj->getOptionalHeaderSize());
+    if (Error E = AuxiliaryHeaderOrErr.takeError())
+      return std::move(E);
+    Obj->AuxiliaryHeader = AuxiliaryHeaderOrErr.get();
+  }
+
   CurOffset += Obj->getOptionalHeaderSize();
 
   // Parse the section header table if it is present.
   if (Obj->getNumberOfSections()) {
     uint64_t SectionHeadersSize =
         Obj->getNumberOfSections() * Obj->getSectionHeaderSize();
     auto SecHeadersOrErr =
         getObject<void>(Data, Base + CurOffset, SectionHeadersSize);

llvm/test/tools/llvm-readobj/XCOFF/lit.local.cfg

+if not 'PowerPC' in config.root.targets:
+    config.unsupported = True

llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test

+## This file tests the ability of llvm-readobj to display the auxiliary header for 64 bits XCOFF and 32 bits XCOFF object file.
+# RUN: llvm-readobj --auxiliary-header %p/Inputs/xcoff-64-xlc-exec 2>&1 | \
+# RUN:   FileCheck --check-prefixes=XLC64EXEC,WARN64 %s
+
+# RUN: llvm-readobj --auxiliary-header %p/Inputs/xcoff-32-xlc-exec | \
+# RUN:   FileCheck --check-prefix=XLC32EXEC %s
+
+# RUN: llvm-readobj --auxiliary-header %p/Inputs/xcoff-32-xlc-obj.o | \
+# RUN:   FileCheck --check-prefix=XLC32OBJ %s
+
+# RUN: llvm-readobj --headers %p/Inputs/xcoff-32-xlc-obj.o | \
+# RUN:   FileCheck --check-prefix=XLC32OBJ %s
+
+# RUN: llvm-readobj --auxiliary-header %p/Inputs/xcoff-32-xlc-obj-malform.o 2>&1 | \
+# RUN:   FileCheck  --check-prefixes=XLC32OBJ-PART,WARN-PART %s
+
+# XLC32EXEC: File: {{.*}}xcoff-32-xlc-exec
+# XLC32EXEC-NEXT: Format: aixcoff-rs6000
+# XLC32EXEC-NEXT: Arch: powerpc
+# XLC32EXEC-NEXT: AddressSize: 32bit
+# XLC32EXEC-NEXT: AuxiliaryHeader {
+# XLC32EXEC-NEXT:   Magic: 0x10B
+# XLC32EXEC-NEXT:   Version: 0x1
+# XLC32EXEC-NEXT:   Size of .text section: 0x498
+# XLC32EXEC-NEXT:   Size of .data section: 0xF0
+# XLC32EXEC-NEXT:   Size of .bss section: 0x4
+# XLC32EXEC-NEXT:   Entry point address: 0x20000658
+# XLC32EXEC-NEXT:   .text section start address: 0x10000128
+# XLC32EXEC-NEXT:   .data section start address: 0x200005C0
+# XLC32EXEC-NEXT:   TOC anchor address: 0x2000066C
+# XLC32EXEC-NEXT:   Section number of entryPoint: 2
+# XLC32EXEC-NEXT:   Section number of .text: 1
+# XLC32EXEC-NEXT:   Section number of .data: 2
+# XLC32EXEC-NEXT:   Section number of TOC: 2
+# XLC32EXEC-NEXT:   Section number of loader data: 4
+# XLC32EXEC-NEXT:   Section number of .bss: 3
+# XLC32EXEC-NEXT:   Maxium alignment of .text: 0x7
+# XLC32EXEC-NEXT:   Maxium alignment of .data: 0x3
+# XLC32EXEC-NEXT:   Module type: 0x314C
+# XLC32EXEC-NEXT:   CPU type of objects: 0x0
+# XLC32EXEC-NEXT:   (Reserved): 0x0
+# XLC32EXEC-NEXT:   Maximum stack size: 0x0
+# XLC32EXEC-NEXT:   Maximum data size: 0x0
+# XLC32EXEC-NEXT:   Reserved for debugger: 0x0
+# XLC32EXEC-NEXT:   Text page size: 0x0
+# XLC32EXEC-NEXT:   Data page size: 0x0
+# XLC32EXEC-NEXT:   Stack page size: 0x0
+# XLC32EXEC-NEXT:   Flag: 0x0
+# XLC32EXEC-NEXT:   Alignment of thread-local storage: 0x0
+# XLC32EXEC-NEXT:   Section number for .tdata: 0
+# XLC32EXEC-NEXT:   Section number for .tbss: 0
+# XLC32EXEC-NEXT: }
+
+
+# XLC64EXEC: File: {{.*}}xcoff-64-xlc-exec
+# XLC64EXEC-NEXT: Format: aix5coff64-rs6000
+# XLC64EXEC-NEXT: Arch: powerpc64
+# XLC64EXEC-NEXT: AddressSize: 64bit
+# XLC64EXEC-NEXT: AuxiliaryHeader {
+# XLC64EXEC-NEXT:   Magic: 0x10B
+# XLC64EXEC-NEXT:   Version: 0x1
+# XLC64EXEC-NEXT:   Reserved for debugger: 0x0
+# XLC64EXEC-NEXT:   .text section start address: 0x1000001F8
+# XLC64EXEC-NEXT:   .data section start address: 0x110000640
+# XLC64EXEC-NEXT:   TOC anchor address: 0x110000738
+# XLC64EXEC-NEXT:   Section number of entryPoint: 2
+# XLC64EXEC-NEXT:   Section number of .text: 1
+# XLC64EXEC-NEXT:   Section number of .data: 2
+# XLC64EXEC-NEXT:   Section number of TOC: 2
+# XLC64EXEC-NEXT:   Section number of loader data: 4
+# XLC64EXEC-NEXT:   Section number of .bss: 3
+# XLC64EXEC-NEXT:   Maxium alignment of .text: 0x7
+# XLC64EXEC-NEXT:   Maxium alignment of .data: 0x3
+# XLC64EXEC-NEXT:   Module type: 0x314C
+# XLC64EXEC-NEXT:   CPU type of objects: 0x0
+# XLC64EXEC-NEXT:   (Reserved): 0x0
+# XLC64EXEC-NEXT:   Text page size: 0x0
+# XLC64EXEC-NEXT:   Data page size: 0x0
+# XLC64EXEC-NEXT:   Stack page size: 0x0
+# XLC64EXEC-NEXT:   Flag: 0x0
+# XLC64EXEC-NEXT:   Alignment of thread-local storage: 0x0
+# XLC64EXEC-NEXT:   Size of .text section: 0x448
+# XLC64EXEC-NEXT:   Size of .data section: 0x180
+# XLC64EXEC-NEXT:   Size of .bss section: 0x8
+# XLC64EXEC-NEXT:   Entry point address: 0x110000710
+# XLC64EXEC-NEXT:   Maximum stack size: 0x0
+# XLC64EXEC-NEXT:   Maximum data size: 0x0
+# XLC64EXEC-NEXT:   Section number for .tdata: 0
+# XLC64EXEC-NEXT:   Section number for .tbss: 0
+# XLC64EXEC-NEXT:   Additional flags 64-bit XCOFF: 0x0
+# WARN64:           {{.*}}llvm-readobj: warning: '<stdin>': There are extra data beyond auxiliary header
+# XLC64EXEC-NEXT:  Extra raw data: (00 00 00 00 00 00 00 00 00 00)
+# XLC64EXEC-NEXT: }
+
+# XLC32OBJ: File: {{.*}}xcoff-32-xlc-obj.o
+# XLC32OBJ-NEXT: Format: aixcoff-rs6000
+# XLC32OBJ-NEXT: Arch: powerpc
+# XLC32OBJ-NEXT: AddressSize: 32bit
+# XLC32OBJ:      AuxiliaryHeader {
+# XLC32OBJ-NEXT:   Magic: 0x10B
+# XLC32OBJ-NEXT:   Version: 0x0
+# XLC32OBJ-NEXT:   Size of .text section: 0x200
+# XLC32OBJ-NEXT:   Size of .data section: 0x3C
+# XLC32OBJ-NEXT:   Size of .bss section: 0x0
+# XLC32OBJ-NEXT:   Entry point address: 0x0
+# XLC32OBJ-NEXT:   .text section start address: 0x0
+# XLC32OBJ-NEXT:   .data section start address: 0x200
+# XLC32OBJ-NEXT: }
+
+# XLC32OBJ-PART: File: {{.*}}xcoff-32-xlc-obj-malform.o
+# XLC32OBJ-PART-NEXT: Format: aixcoff-rs6000
+# XLC32OBJ-PART-NEXT: Arch: powerpc
+# XLC32OBJ-PART-NEXT: AddressSize: 32bit
+# XLC32OBJ-PART-NEXT: AuxiliaryHeader {
+# XLC32OBJ-PART-NEXT:   Magic: 0x10B
+# XLC32OBJ-PART-NEXT:   Version: 0x0
+# XLC32OBJ-PART-NEXT:   Size of .text section: 0x200
+# XLC32OBJ-PART-NEXT:   Size of .data section: 0x3C
+# XLC32OBJ-PART-NEXT:   Size of .bss section: 0x0
+# XLC32OBJ-PART-NEXT:   Entry point address: 0x0
+# XLC32OBJ-PART-NEXT:   .text section start address: 0x0
+# WARN-PART:   {{.*}}llvm-readobj: warning: '<stdin>': Only partial field for .data section start address at offset (24).
+# XLC32OBJ-PART-NEXT:   Raw data: (00 00 02)
+# XLC32OBJ-PART-NEXT: }
+

llvm/tools/llvm-readobj/ObjDumper.h

public:
   virtual void printCodeViewDebugInfo() { }
   virtual void
   mergeCodeViewTypes(llvm::codeview::MergingTypeTableBuilder &CVIDs,
                      llvm::codeview::MergingTypeTableBuilder &CVTypes,
                      llvm::codeview::GlobalTypeTableBuilder &GlobalCVIDs,
                      llvm::codeview::GlobalTypeTableBuilder &GlobalCVTypes,
                      bool GHash) {}
 
+  // Only implement for XCOFF
+  virtual void printAuxiliaryHeader() {}
+
   // Only implemented for MachO.
   virtual void printMachODataInCode() { }
   virtual void printMachOVersionMin() { }
   virtual void printMachODysymtab() { }
   virtual void printMachOSegment() { }
   virtual void printMachOIndirectSymbols() { }
   virtual void printMachOLinkerOptions() { }
 

llvm/tools/llvm-readobj/Opts.td

 def bb_addr_map : FF<"bb-addr-map", "Display the BB address map section">;
 def cg_profile : FF<"cg-profile", "Display call graph profile section">;
 defm demangle : BB<"demangle", "Demangle symbol names", "Do not demangle symbol names (default)">;
 def dependent_libraries : FF<"dependent-libraries", "Display the dependent libraries section">;
 def dyn_relocations : FF<"dyn-relocations", "Display the dynamic relocation entries in the file">;
 def dyn_syms : FF<"dyn-syms", "Display the dynamic symbol table">;
 def expand_relocs : FF<"expand-relocs", "Expand each shown relocation to multiple lines">;
 def file_header : FF<"file-header", "Display file header">;
 def headers : FF<"headers", "Equivalent to setting: --file-header, --program-headers, --section-headers">;
 defm hex_dump : Eq<"hex-dump", "Display the specified section(s) as hexadecimal bytes">, MetaVarName<"<name or index>">;
 def relocs : FF<"relocs", "Display the relocation entries in the file">;
 def section_data : FF<"section-data", "Display section data for each section shown. This option has no effect for GNU style output">;
 def section_details : FF<"section-details", "Display the section details">;
 def section_headers : FF<"section-headers", "Display section headers">;
 def section_mapping : FF<"section-mapping", "Display the section to segment mapping">;
 def section_mapping_EQ_false : FF<"section-mapping=false", "Don't display the section to segment mapping">, Flags<[HelpHidden]>;
 def section_relocations : FF<"section-relocations", "Display relocations for each section shown. This option has no effect for GNU style output">;

 def coff_debug_directory : FF<"coff-debug-directory", "Display debug directory">, Group<grp_coff>;
 def coff_directives : FF<"coff-directives", "Display .drectve section">, Group<grp_coff>;
 def coff_exports : FF<"coff-exports", "Display export table">, Group<grp_coff>;
 def coff_imports : FF<"coff-imports", "Display import table">, Group<grp_coff>;
 def coff_load_config : FF<"coff-load-config", "Display load config">, Group<grp_coff>;
 def coff_resources : FF<"coff-resources", "Display .rsrc section">, Group<grp_coff>;
 def coff_tls_directory : FF<"coff-tls-directory", "Display TLS directory">, Group<grp_coff>;
 
+// XCOFF specific options.
+def grp_xcoff : OptionGroup<"kind">, HelpText<"OPTIONS (XCOFF specific)">;
+def auxiliary_header : FF<"auxiliary-header" , "display the auxiliary header">, Group<grp_xcoff>;
+
 def help : FF<"help", "Display this help">;
 def version : FF<"version", "Display the version">;
 
 // Ignored for GNU readelf compatibility.
 def wide : FF<"wide", "Ignored for GNU readelf compatibility">;
 def : F<"W", "Ignored for GNU readelf compatibility">, Alias<wide>;
 
 // Traditional llvm-readobj Aliases.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

 //===----------------------------------------------------------------------===//
 
 #include "ObjDumper.h"
 #include "llvm-readobj.h"
 #include "llvm/Object/XCOFFObjectFile.h"
 #include "llvm/Support/FormattedStream.h"
 #include "llvm/Support/ScopedPrinter.h"
 
+#include <stddef.h>
+
 using namespace llvm;
 using namespace object;
 
 namespace {
 
 class XCOFFDumper : public ObjDumper {
 
 public:
   XCOFFDumper(const XCOFFObjectFile &Obj, ScopedPrinter &Writer)
       : ObjDumper(Writer, Obj.getFileName()), Obj(Obj) {}
 
   void printFileHeaders() override;
+  void printAuxiliaryHeader() override;
   void printSectionHeaders() override;
   void printRelocations() override;
   void printSymbols() override;
   void printDynamicSymbols() override;
   void printUnwindInfo() override;
   void printStackMap() const override;
   void printNeededLibraries() override;
   void printStringTable() override;
 
 private:
   template <typename T> void printSectionHeaders(ArrayRef<T> Sections);
   template <typename T> void printGenericSectionHeader(T &Sec) const;
   template <typename T> void printOverflowSectionHeader(T &Sec) const;
   void printFileAuxEnt(const XCOFFFileAuxEnt *AuxEntPtr);
   void printCsectAuxEnt(XCOFFCsectAuxRef AuxEntRef);
   void printSectAuxEntForStat(const XCOFFSectAuxEntForStat *AuxEntPtr);
   void printSymbol(const SymbolRef &);
   template <typename Shdr, typename RelTy>
   void printRelocations(ArrayRef<Shdr> Sections);
+  void printAuxiliaryHeader(const XCOFFAuxiliaryHeader32 *AuxHeader);
+  void printAuxiliaryHeader(const XCOFFAuxiliaryHeader64 *AuxHeader);
   const XCOFFObjectFile &Obj;
 };
 } // anonymous namespace
 
 void XCOFFDumper::printFileHeaders() {
   DictScope DS(W, "FileHeader");
   W.printHex("Magic", Obj.getMagic());
   W.printNumber("NumberOfSections", Obj.getNumberOfSections());
void XCOFFDumper::printFileHeaders() {
 
   W.printHex("OptionalHeaderSize", Obj.getOptionalHeaderSize());
   W.printHex("Flags", Obj.getFlags());
 
   // TODO FIXME Add support for the auxiliary header (if any) once
   // XCOFFObjectFile has the necessary support.
 }
 
+void XCOFFDumper::printAuxiliaryHeader() {
+  if (Obj.is64Bit())
+    printAuxiliaryHeader(Obj.auxiliaryHeader64());
+  else
+    printAuxiliaryHeader(Obj.auxiliaryHeader32());
+}
+
 void XCOFFDumper::printSectionHeaders() {
   if (Obj.is64Bit())
     printSectionHeaders(Obj.sections64());
   else
     printSectionHeaders(Obj.sections32());
 }
 
 void XCOFFDumper::printRelocations() {
void XCOFFDumper::printGenericSectionHeader(T &Sec) const {
   W.printHex("Size", Sec.SectionSize);
   W.printHex("RawDataOffset", Sec.FileOffsetToRawData);
   W.printHex("RelocationPointer", Sec.FileOffsetToRelocationInfo);
   W.printHex("LineNumberPointer", Sec.FileOffsetToLineNumberInfo);
   W.printNumber("NumberOfRelocations", Sec.NumberOfRelocations);
   W.printNumber("NumberOfLineNumbers", Sec.NumberOfLineNumbers);
 }
 
+void XCOFFDumper::printAuxiliaryHeader(
+    const XCOFFAuxiliaryHeader32 *AuxHeader) {
+  if (AuxHeader == nullptr)
+    return;
+  uint16_t AuxSize = Obj.getOptionalHeaderSize();
+  uint16_t PartialFieldOffset = AuxSize;
+  const char *PartialFieldName = nullptr;
+
+  DictScope DS(W, "AuxiliaryHeader");
+
+#define PrintAuxMember32(H, S, T)                                              \
+  if (offsetof(XCOFFAuxiliaryHeader32, T) +                                    \
+          sizeof(XCOFFAuxiliaryHeader32::T) <=                                 \
+      AuxSize)                                                                 \
+    W.print##H(S, AuxHeader->T);                                               \
+  else if (offsetof(XCOFFAuxiliaryHeader32, T) < AuxSize) {                    \
+    PartialFieldOffset = offsetof(XCOFFAuxiliaryHeader32, T);                  \
+    PartialFieldName = S;                                                      \
+  }
+
+  PrintAuxMember32(Hex, "Magic", AuxMagic);
+  PrintAuxMember32(Hex, "Version", Version);
+  PrintAuxMember32(Hex, "Size of .text section", TextSize);
+  PrintAuxMember32(Hex, "Size of .data section", InitDataSize);
+  PrintAuxMember32(Hex, "Size of .bss section", BssDataSize);
+  PrintAuxMember32(Hex, "Entry point address", EntryPointAddr);
+  PrintAuxMember32(Hex, ".text section start address", TextStartAddr);
+  PrintAuxMember32(Hex, ".data section start address", DataStartAddr);
+  PrintAuxMember32(Hex, "TOC anchor address", TOCAnchorAddr);
+  PrintAuxMember32(Number, "Section number of entryPoint", SecNumOfEntryPoint);
+  PrintAuxMember32(Number, "Section number of .text", SecNumOfText);
+  PrintAuxMember32(Number, "Section number of .data", SecNumOfData);
+  PrintAuxMember32(Number, "Section number of TOC", SecNumOfTOC);
+  PrintAuxMember32(Number, "Section number of loader data", SecNumOfLoader);
+  PrintAuxMember32(Number, "Section number of .bss", SecNumOfBSS);
+  PrintAuxMember32(Hex, "Maxium alignment of .text", MaxAlignOfText);
+  PrintAuxMember32(Hex, "Maxium alignment of .data", MaxAlignOfData);
+  PrintAuxMember32(Hex, "Module type", ModuleType);
+  PrintAuxMember32(Hex, "CPU type of objects", CpuFlag);
+  PrintAuxMember32(Hex, "(Reserved)", CpuType);
+  PrintAuxMember32(Hex, "Maximum stack size", MaxStackSize);
+  PrintAuxMember32(Hex, "Maximum data size", MaxDataSize);
+  PrintAuxMember32(Hex, "Reserved for debugger", ReservedForDebugger);
+  PrintAuxMember32(Hex, "Text page size", TextPageSize);
+  PrintAuxMember32(Hex, "Data page size", DataPageSize);
+  PrintAuxMember32(Hex, "Stack page size", StackPageSize);
+  if (offsetof(XCOFFAuxiliaryHeader32, FlagAndTDataAlignment) +
+          sizeof(XCOFFAuxiliaryHeader32::FlagAndTDataAlignment) <=
+      AuxSize) {
+    W.printHex("Flag", AuxHeader->getFlag());
+    W.printHex("Alignment of thread-local storage",
+               AuxHeader->getTDataAlignment());
+  }
+
+  PrintAuxMember32(Number, "Section number for .tdata", SecNumOfTData);
+  PrintAuxMember32(Number, "Section number for .tbss", SecNumOfTBSS);
+
+  // Deal with error.
+  if (PartialFieldOffset < AuxSize) {
+    std::string ErrInfo;
+    llvm::raw_string_ostream StringOS(ErrInfo);
+    StringOS << "Only partial field for " << PartialFieldName << " at offset ("
+             << PartialFieldOffset << ").";
+    StringOS.flush();
+    reportWarning(
+        make_error<GenericBinaryError>(ErrInfo, object_error::parse_failed),
+        "-");
+    W.printBinary(
+        "Raw data", "",
+        ArrayRef<uint8_t>((const uint8_t *)(AuxHeader) + PartialFieldOffset,
+                          AuxSize - PartialFieldOffset));
+  } else if (sizeof(XCOFFAuxiliaryHeader32) < AuxSize) {
+    reportWarning(make_error<GenericBinaryError>(
+                      "There are extra data beyond auxiliary header",
+                      object_error::parse_failed),
+                  "-");
+    W.printBinary("Extra raw data", "",
+                  ArrayRef<uint8_t>((const uint8_t *)(AuxHeader) +
+                                        sizeof(XCOFFAuxiliaryHeader32),
+                                    AuxSize - sizeof(XCOFFAuxiliaryHeader32)));
+  }
+
+#undef PrintAuxMember32
+}
+
+void XCOFFDumper::printAuxiliaryHeader(
+    const XCOFFAuxiliaryHeader64 *AuxHeader) {
+  if (AuxHeader == nullptr)
+    return;
+  uint16_t AuxSize = Obj.getOptionalHeaderSize();
+  uint16_t PartialFieldOffset = AuxSize;
+  const char *PartialFieldName = nullptr;
+
+  DictScope DS(W, "AuxiliaryHeader");
+
+#define PrintAuxMember64(H, S, T)                                              \
+  if (offsetof(XCOFFAuxiliaryHeader64, T) +                                    \
+          sizeof(XCOFFAuxiliaryHeader64::T) <=                                 \
+      AuxSize)                                                                 \
+    W.print##H(S, AuxHeader->T);                                               \
+  else if (offsetof(XCOFFAuxiliaryHeader64, T) < AuxSize) {                    \
+    PartialFieldOffset = offsetof(XCOFFAuxiliaryHeader64, T);                  \
+    PartialFieldName = S;                                                      \
+  }
+
+  PrintAuxMember64(Hex, "Magic", AuxMagic);
+  PrintAuxMember64(Hex, "Version", Version);
+  PrintAuxMember64(Hex, "Reserved for debugger", ReservedForDebugger);
+  PrintAuxMember64(Hex, ".text section start address", TextStartAddr);
+  PrintAuxMember64(Hex, ".data section start address", DataStartAddr);
+  PrintAuxMember64(Hex, "TOC anchor address", TOCAnchorAddr);
+  PrintAuxMember64(Number, "Section number of entryPoint", SecNumOfEntryPoint);
+  PrintAuxMember64(Number, "Section number of .text", SecNumOfText);
+  PrintAuxMember64(Number, "Section number of .data", SecNumOfData);
+  PrintAuxMember64(Number, "Section number of TOC", SecNumOfTOC);
+  PrintAuxMember64(Number, "Section number of loader data", SecNumOfLoader);
+  PrintAuxMember64(Number, "Section number of .bss", SecNumOfBSS);
+  PrintAuxMember64(Hex, "Maxium alignment of .text", MaxAlignOfText);
+  PrintAuxMember64(Hex, "Maxium alignment of .data", MaxAlignOfData);
+  PrintAuxMember64(Hex, "Module type", ModuleType);
+  PrintAuxMember64(Hex, "CPU type of objects", CpuFlag);
+  PrintAuxMember64(Hex, "(Reserved)", CpuType);
+  PrintAuxMember64(Hex, "Text page size", TextPageSize);
+  PrintAuxMember64(Hex, "Data page size", DataPageSize);
+  PrintAuxMember64(Hex, "Stack page size", StackPageSize);
+  if (offsetof(XCOFFAuxiliaryHeader64, FlagAndTDataAlignment) +
+          sizeof(XCOFFAuxiliaryHeader64::FlagAndTDataAlignment) <=
+      AuxSize) {
+    W.printHex("Flag", AuxHeader->getFlag());
+    W.printHex("Alignment of thread-local storage",
+               AuxHeader->getTDataAlignment());
+  }
+  PrintAuxMember64(Hex, "Size of .text section", TextSize);
+  PrintAuxMember64(Hex, "Size of .data section", InitDataSize);
+  PrintAuxMember64(Hex, "Size of .bss section", BssDataSize);
+  PrintAuxMember64(Hex, "Entry point address", EntryPointAddr);
+  PrintAuxMember64(Hex, "Maximum stack size", MaxStackSize);
+  PrintAuxMember64(Hex, "Maximum data size", MaxDataSize);
+  PrintAuxMember64(Number, "Section number for .tdata", SecNumOfTData);
+  PrintAuxMember64(Number, "Section number for .tbss", SecNumOfTBSS);
+  PrintAuxMember64(Hex, "Additional flags 64-bit XCOFF", XCOFF64Flag);
+
+  if (PartialFieldOffset < AuxSize) {
+    std::string ErrInfo;
+    llvm::raw_string_ostream StringOS(ErrInfo);
+    StringOS << "Only partial field for " << PartialFieldName << " at offset ("
+             << PartialFieldOffset << ").";
+    StringOS.flush();
+    reportWarning(
+        make_error<GenericBinaryError>(ErrInfo, object_error::parse_failed),
+        "-");
+    ;
+    W.printBinary(
+        "Raw data", "",
+        ArrayRef<uint8_t>((const uint8_t *)(AuxHeader) + PartialFieldOffset,
+                          AuxSize - PartialFieldOffset));
+  } else if (sizeof(XCOFFAuxiliaryHeader64) < AuxSize) {
+    reportWarning(make_error<GenericBinaryError>(
+                      "There are extra data beyond auxiliary header",
+                      object_error::parse_failed),
+                  "-");
+    W.printBinary("Extra raw data", "",
+                  ArrayRef<uint8_t>((const uint8_t *)(AuxHeader) +
+                                        sizeof(XCOFFAuxiliaryHeader64),
+                                    AuxSize - sizeof(XCOFFAuxiliaryHeader64)));
+  }
+
+#undef PrintAuxMember64
+}
+
 template <typename T>
 void XCOFFDumper::printSectionHeaders(ArrayRef<T> Sections) {
   ListScope Group(W, "Sections");
 
   uint16_t Index = 1;
   for (const T &Sec : Sections) {
     DictScope SecDS(W, "Section");
 

llvm/tools/llvm-readobj/llvm-readobj.cpp

public:
   ReadobjOptTable() : OptTable(InfoTable) { setGroupedShortOptions(true); }
 };
 
 enum OutputFormatTy { bsd, sysv, posix, darwin, just_symbols };
 } // namespace
 
 namespace opts {
 static bool Addrsig;
 static bool All;
 static bool ArchSpecificInfo;
 static bool BBAddrMap;
 bool ExpandRelocs;
 static bool CGProfile;
 bool Demangle;
 static bool DependentLibraries;
 static bool DynRelocs;
 static bool DynamicSymbols;
 static bool FileHeaders;
 static bool Headers;
 static std::vector<std::string> HexDump;
 static bool PrintStackMap;
 static bool PrintStackSizes;
 static bool Relocations;
 bool SectionData;
 static bool SectionDetails;
 static bool SectionHeaders;
 bool SectionRelocations;

 static bool COFFDebugDirectory;
 static bool COFFDirectives;
 static bool COFFExports;
 static bool COFFImports;
 static bool COFFLoadConfig;
 static bool COFFResources;
 static bool COFFTLSDirectory;
 
+// XCOFF specific options.
+static bool XCOFFAuxiliaryHeader;
+
 OutputStyleTy Output = OutputStyleTy::LLVM;
 static std::vector<std::string> InputFilenames;
 } // namespace opts
 
 static StringRef ToolName;
 
 namespace llvm {
 
static void parseOptions(const opt::InputArgList &Args) {
   opts::COFFDebugDirectory = Args.hasArg(OPT_coff_debug_directory);
   opts::COFFDirectives = Args.hasArg(OPT_coff_directives);
   opts::COFFExports = Args.hasArg(OPT_coff_exports);
   opts::COFFImports = Args.hasArg(OPT_coff_imports);
   opts::COFFLoadConfig = Args.hasArg(OPT_coff_load_config);
   opts::COFFResources = Args.hasArg(OPT_coff_resources);
   opts::COFFTLSDirectory = Args.hasArg(OPT_coff_tls_directory);
 
+  // XCOFF specific options.
+  opts::XCOFFAuxiliaryHeader = Args.hasArg(OPT_auxiliary_header);
+
   opts::InputFilenames = Args.getAllArgValues(OPT_INPUT);
 }
 
 namespace {
 struct ReadObjTypeTableBuilder {
   ReadObjTypeTableBuilder()
       : Allocator(), IDTable(Allocator), TypeTable(Allocator),
         GlobalIDTable(Allocator), GlobalTypeTable(Allocator) {}
Writer.printString(
         "AddressSize",
         std::string(formatv("{0}bit", 8 * Obj.getBytesInAddress())));
     Dumper->printLoadName();
   }
 
   if (opts::FileHeaders)
     Dumper->printFileHeaders();
 
+  if (Obj.isXCOFF() && opts::XCOFFAuxiliaryHeader)
+    Dumper->printAuxiliaryHeader();
+
   // This is only used for ELF currently. In some cases, when an object is
   // corrupt (e.g. truncated), we can't dump anything except the file header.
   if (!ContentErrString.empty())
     reportError(createError(ContentErrString), FileStr);
 
   if (opts::SectionDetails || opts::SectionHeaders) {
     if (opts::Output == opts::GNU && opts::SectionDetails)
       Dumper->printSectionDetails();
int main(int argc, char *argv[]) {
 
   // Default to print error if no filename is specified.
   if (opts::InputFilenames.empty()) {
     error("no input files specified");
   }
 
   if (opts::All) {
     opts::FileHeaders = true;
+    opts::XCOFFAuxiliaryHeader = true;
     opts::ProgramHeaders = true;
     opts::SectionHeaders = true;
     opts::Symbols = true;
     opts::Relocations = true;
     opts::DynamicTable = true;
     opts::Notes = true;
     opts::VersionInfo = true;
     opts::UnwindInfo = true;
     opts::SectionGroups = true;
     opts::HashHistogram = true;
     if (opts::Output == opts::LLVM) {
       opts::Addrsig = true;
       opts::PrintStackSizes = true;
     }
   }
 
   if (opts::Headers) {
     opts::FileHeaders = true;
+    opts::XCOFFAuxiliaryHeader = true;
     opts::ProgramHeaders = true;
     opts::SectionHeaders = true;
   }
 
   ScopedPrinter Writer(fouts());
   for (const std::string &I : opts::InputFilenames)
     dumpInput(I, Writer);