[AIX][XCOFF] address post-commit review comments of patch https://reviews.llvm.org/D82549

[DiggerLin]

Summary

Address post-commit review comments in the https://reviews.llvm.org/D82549, including

1. changed file name from llvm/test/tools/llvm-readobj/XCOFF/xcoff-auxiliary-header.test --> llvm/test/tools/llvm-readobj/XCOFF/auxiliary-header.test
2. replaced macro define by using lambda function.
3. added a helper function to reduce the duplicated check and print error code.

[jhenderson]

Your patch summary seems incorrect to me, compared to what it should be. Perhaps simply saying "address post-commit review comments" in the summary is fine. This is also not NFC, as some output has changed slightly (see the "extra data" warning for example). There should be testing for the changed output.

Some of the items I've highlighted (e.g. spurious blank lines, C++ style casts, function case naming) are things that I've asked you to fix many times in other reviews. Please could you take more care when making changes, and consider reviewing your own code first for these sort of things, before putting up patches for wider community review, so that we don't have to waste time pointing them out to you again and again.

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

2 ↗

(On Diff #396027)

I previously suggested test input files should be renamed too, i.e. 64-xlc-exec since it's already in an XCOFF subdirectory.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

19–20

This header shouldn't have been added in your previous patch. Please delete it here.

743

My original suggestion was that this should be a lambda witihin the relevant methods, so that appropriate members could be captured. However, you've pulled it out of those functions into a standalone variable. I don't really mind having it separate, but please explain to me why this is still a lambda now and not a template function?

744

Don't abbreviate unnecessarily. It harms readability.

I previously suggested you looked at std::bind to allow you to pass in the print* function, rather than having to have the PrintStyle enum. Did you look at doing that?

747

Don't use C-style casts. Use static_cast and reinterpret_cast.

758

762

Don't start functions with a blank line.

765

No need to provide llvm:: namespace qualifier.

However, I previously pointed out you don't want a string stream here at all. From my comment in D82549 (with a couple of minor fixes):

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

Alternatively, you could just do:

Dumper->reportUniqueWarning(Twine("only partial field for ") + PartialFieldName + " at offset (" + PartialFieldOffset + ")");

772

Use reinterpret_cast/static_cast, not C-style casts.

779

C++ style cast needed.

789–790

You seem to have ignored my comment chain in D82549 about putting this scope into a higher-level function. Yes, this means that there's empty output, if you don't have an auxiliary header, but that's fine, since you explicitly asked for the auxiliary header output. Additionally, that's what is already being done in LLVM-style ELF output.

[DiggerLin]

In D116220#3222197, @jhenderson wrote:

Your patch summary seems incorrect to me, compared to what it should be. Perhaps simply saying "address post-commit review comments" in the summary is fine. This is also not NFC, as some output has changed slightly (see the "extra data" warning for example). There should be testing for the changed output.

Some of the items I've highlighted (e.g. spurious blank lines, C++ style casts, function case naming) are things that I've asked you to fix many times in other reviews. Please could you take more care when making changes, and consider reviewing your own code first for these sort of things, before putting up patches for wider community review, so that we don't have to waste time pointing them out to you again and again.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

743

change to template function, thanks

744

sorry that std::bind do not work for a template function. it will compile error

[DiggerLin]

address comment

[DiggerLin]

llvm/test/tools/yaml2obj/XCOFF/aux-hdr-defaults.yaml

107

can you check why the yaml2obj generate the flag 0x80 here ,not flag 0? @Esme

llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml

49

according to comment https://reviews.llvm.org/D116220#inline-1115640, there will be empty AuxiliaryHeader { } be printed out even if there no Auxiliary Header in an object file when we use "llvm-readobj --headers".
what is your opinion ? @hubert.reinterpretcast .@Esme

[jhenderson]

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

3 ↗

(On Diff #412498)

15 ↗

(On Diff #412498)

17 ↗

(On Diff #412498)

54 ↗

(On Diff #412498)

Delete extra blank line.

91 ↗

(On Diff #412498)

There's no need to show the tool name is printed here.

Why do you have a warning in a "normal" case?

122 ↗

(On Diff #412498)

llvm/tools/llvm-readobj/XCOFFDumper.cpp

19–20

Ping. The header is still there. Please delete it, and please stop marking things as Done when they haven't been addressed.

744

You've not addressed my comment about the parameter name. Also, the function name shouldn't start with a capital letter.

sorry that std::bind do not work for a template function. it will compile error

Are you sure? (see e.g. https://stackoverflow.com/questions/26981608/how-to-use-template-function-parameter-in-stdbind).

[DiggerLin]

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

91 ↗

(On Diff #412498)

please see the comment in
"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."
https://reviews.llvm.org/D82549#inline-770356

llvm/tools/llvm-readobj/XCOFFDumper.cpp

19–20

thanks

744

if you look into the link info, when using std::bind, it need to explicit instantiate the template PrintAuxMemberHelper function.

there is info as
"Execute_For is a function template, therefore you need to either supply it with type template arguments"

and

https://stackoverflow.com/questions/14727313/c-how-to-reference-templated-functions-using-stdbind-stdfunction

https://en.cppreference.com/w/cpp/utility/functional/bind

"f - Callable object (function object, pointer to function, reference to function, pointer to member function, or pointer to data member) that will be bound to some arguments"

if we need to initiate the template , we need to define several helper function witch std::bind, what is the benefit of using std::bind ?

I need to define something as

using namespace std::placeholders;

auto PrintAuxMember16 = std::bind(PrintAuxMemberHelper<support::ubig16_t,XCOFFFileHeader32>, _1, _2, _3, AuxHeader,AuxSize,PartialFieldOffset, PartialFieldName, W);

auto PrintAuxMember32 = std::bind(PrintAuxMemberHelper<support::ubig32_t,XCOFFFileHeader32>, _1, _2, _3, AuxHeader,AuxSize,PartialFieldOffset, PartialFieldName, W);

[jhenderson]

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

17 ↗

(On Diff #413878)

You might be able to use [[FILE]] here too, instead of the wildcard regex.

90 ↗

(On Diff #413878)

Please delete the wildcard regex, as per my original suggestion.

Yes, my question isn't whether we should be printing the warning, but rather whether the normal case should have the warning. I don't think it should, and instead the warning case should be tested explicitly in a separate case. This is a common testing pattern used throughout: handle the normal case, and then have additional cases for errors and warnings.

Aside: if the xlc tool is explicitly generating data that llvm-readobj can't understand, then either llvm-readobj is missing some functionality, or there's a bug in xlc. I'm guessing it's llvm-readobj...

121 ↗

(On Diff #413878)

Ditto, please remove the wildcard regex.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

744

Also, the function name shouldn't start with a capital letter.

Still not addressed. Please use lowerCameCase for function names.

[DiggerLin]

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

17 ↗

(On Diff #413878)

thanks

90 ↗

(On Diff #413878)

Aside: if the xlc tool is explicitly generating data that llvm-readobj can't understand, then either llvm-readobj is missing some functionality, or there's a bug in xlc. I'm guessing it's llvm-readobj...

The llvm-readobj is implement is based on the https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/com.ibm.aix.files/XCOFF.htm

I think the extra raw data is align padding data.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

744

thanks

[jhenderson]

LGTM, but you probably want someone else with XCOFF experience to discuss the warning. As noted earlier inline, I don't think a warning in "regular" usage makes sense, since this warning will always be emitted and there's nothing the user can do about it. However, it's possible it means there's extra data that llvm-readobj might be intended to interpret that it isn't, I don't know.

llvm/include/llvm/Object/XCOFFObjectFile.h

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> {
+struct XCOFFAuxiliaryHeader64 : XCOFFAuxiliaryHeader<XCOFFAuxiliaryHeader64> {
   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;

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

-# REQUIRES: system-aix
-## 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/test/tools/yaml2obj/XCOFF/aux-hdr-defaults.yaml

 # CASE2-NEXT:   Maxium alignment of .text: 0x0
 # CASE2-NEXT:   Maxium alignment of .data: 0x0
 # CASE2-NEXT:   Module type: 0x0
 # CASE2-NEXT:   CPU type of objects: 0x0
 # CASE2-NEXT:   (Reserved): 0x0
 # CASE2-NEXT:   Text page size: 0x0
 # CASE2-NEXT:   Data page size: 0x0
 # CASE2-NEXT:   Stack page size: 0x0
-# CASE2-NEXT:   Flag: 0x0
+# CASE2-NEXT:   Flag: 0x80
 # CASE2-NEXT:   Alignment of thread-local storage: 0x0
 # CASE2-NEXT:   Size of .text section: 0x8
 # CASE2-NEXT:   Size of .data section: 0x8
 # CASE2-NEXT:   Size of .bss section: 0x8
 # CASE2-NEXT:   Entry point address: 0x0
 # CASE2-NEXT:   Maximum stack size: 0x0
 # CASE2-NEXT:   Maximum data size: 0x0
 # CASE2-NEXT:   Section number for .tdata: 8

 # CASE4-NEXT:   Maxium alignment of .text: 0x0
 # CASE4-NEXT:   Maxium alignment of .data: 0x0
 # CASE4-NEXT:   Module type: 0x0
 # CASE4-NEXT:   CPU type of objects: 0x0
 # CASE4-NEXT:   (Reserved): 0x0
 # CASE4-NEXT:   Text page size: 0x0
 # CASE4-NEXT:   Data page size: 0x0
 # CASE4-NEXT:   Stack page size: 0x0
-# CASE4-NEXT:   Flag: 0x0
+# CASE4-NEXT:   Flag: 0x80
 # CASE4-NEXT:   Alignment of thread-local storage: 0x0
 # CASE4-NEXT:   Size of .text section: 0x0
 # CASE4-NEXT:   Size of .data section: 0x0
 # CASE4-NEXT:   Size of .bss section: 0x0
 # CASE4-NEXT:   Entry point address: 0x0
 # CASE4-NEXT:   Maximum stack size: 0x0
 # CASE4-NEXT:   Maximum data size: 0x0
 # CASE4-NEXT:   Section number for .tdata: 0
 # CASE4-NEXT:   Section number for .tbss: 0
 # CASE4-NEXT:   Additional flags 64-bit XCOFF: 0x8000
 # CASE4-NEXT: }

llvm/test/tools/yaml2obj/XCOFF/aux-hdr-full-contents.yaml

 # CHECK64-NEXT:   Maxium alignment of .data: 0x3
 # CHECK64-NEXT:   Module type: 0x0
 # CHECK64-NEXT:   CPU type of objects: 0x1
 # CHECK64-NEXT:   (Reserved): 0x0
 # CHECK64-NEXT:   Text page size: 0x1
 # CHECK64-NEXT:   Data page size: 0x1
 # CHECK64-NEXT:   Stack page size: 0x1
 # CHECK64-NEXT:   Flag: 0x0
-# CHECK64-NEXT:   Alignment of thread-local storage: 0x0
+# CHECK64-NEXT:   Alignment of thread-local storage: 0x1
 # CHECK64-NEXT:   Size of .text section: 0x8
 # CHECK64-NEXT:   Size of .data section: 0x9
 # CHECK64-NEXT:   Size of .bss section: 0x10
 # CHECK64-NEXT:   Entry point address: 0x1111
 # CHECK64-NEXT:   Maximum stack size: 0x0
 # CHECK64-NEXT:   Maximum data size: 0x0
 # CHECK64-NEXT:   Section number for .tdata: 7
 # CHECK64-NEXT:   Section number for .tbss: 8

llvm/test/tools/yaml2obj/XCOFF/basic-doc.yaml

 # CHECK-NEXT:   Magic: 0x1DF
 # CHECK-NEXT:   NumberOfSections: 5
 # CHECK-NEXT:   TimeStamp: None (0x0)
 # CHECK-NEXT:   SymbolTableOffset: 0x104
 # CHECK-NEXT:   SymbolTableEntries: 5
 # CHECK-NEXT:   OptionalHeaderSize: 0x0
 # CHECK-NEXT:   Flags: 0x0
 # CHECK-NEXT: }
+# CHECK-NEXT: AuxiliaryHeader {
+# CHECK-NEXT: }
 # CHECK-NEXT: Sections [
 # CHECK-NEXT:   Section {
 # CHECK-NEXT:     Index: 1
 # CHECK-NEXT:     Name: .text
 # CHECK-NEXT:     PhysicalAddress: 0x0
 # CHECK-NEXT:     VirtualAddress: 0x0
 # CHECK-NEXT:     Size: 0x8
 # CHECK-NEXT:     RawDataOffset: 0xDC

llvm/test/tools/yaml2obj/XCOFF/basic-doc64.yaml

 # CHECK64-NEXT:   Magic: 0x1F7
 # CHECK64-NEXT:   NumberOfSections: 4
 # CHECK64-NEXT:   TimeStamp: None (0x0)
 # CHECK64-NEXT:   SymbolTableOffset: 0x152
 # CHECK64-NEXT:   SymbolTableEntries: 4
 # CHECK64-NEXT:   OptionalHeaderSize: 0x0
 # CHECK64-NEXT:   Flags: 0x0
 # CHECK64-NEXT: }
+# CHECK64-NEXT: AuxiliaryHeader {
+# CHECK64-NEXT: }
 # CHECK64-NEXT: Sections [
 # CHECK64-NEXT:   Section {
 # CHECK64-NEXT:     Index: 1
 # CHECK64-NEXT:     Name: .text
 # CHECK64-NEXT:     PhysicalAddress: 0x0
 # CHECK64-NEXT:     VirtualAddress: 0x0
 # CHECK64-NEXT:     Size: 0x0
 # CHECK64-NEXT:     RawDataOffset: 0x0

llvm/test/tools/yaml2obj/XCOFF/full-contents.yaml

 # CHECK-NEXT:   Magic: 0x1DF
 # CHECK-NEXT:   NumberOfSections: 2
 # CHECK-NEXT:   TimeStamp: None (0x0)
 # CHECK-NEXT:   SymbolTableOffset: 0x7A
 # CHECK-NEXT:   SymbolTableEntries: 4
 # CHECK-NEXT:   OptionalHeaderSize: 0x0
 # CHECK-NEXT:   Flags: 0x0
 # CHECK-NEXT: }
+# CHECK-NEXT: AuxiliaryHeader {
+# CHECK-NEXT: }
 # CHECK-NEXT: Sections [
 # CHECK-NEXT:   Section {
 # CHECK-NEXT:     Index: 1
 # CHECK-NEXT:     Name: .text
 # CHECK-NEXT:     PhysicalAddress: 0x0
 # CHECK-NEXT:     VirtualAddress: 0x0
 # CHECK-NEXT:     Size: 0x8
 # CHECK-NEXT:     RawDataOffset: 0x64

llvm/tools/llvm-readobj/Opts.td

 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 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>;
 

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 <ctime>
-#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;
 
+  ScopedPrinter &getScopedPrinter() const { return W; }
+
 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;
   template <typename T> const T *getAuxEntPtr(uintptr_t AuxAddress);
   void printFileAuxEnt(const XCOFFFileAuxEnt *AuxEntPtr);
   void printCsectAuxEnt(XCOFFCsectAuxRef AuxEntRef);
   void printSectAuxEntForStat(const XCOFFSectAuxEntForStat *AuxEntPtr);
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() {
+  DictScope DS(W, "AuxiliaryHeader");
+
   if (Obj.is64Bit())
     printAuxiliaryHeader(Obj.auxiliaryHeader64());
   else
     printAuxiliaryHeader(Obj.auxiliaryHeader32());
 }
 
 void XCOFFDumper::printSectionHeaders() {
   if (Obj.is64Bit())
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);
 }
 
+enum PrintStyle { Hex, Number };
+template <typename T, typename V>
+static void printAuxMemberHelper(PrintStyle Style, const char *MemberName,
+                                 const T &Member, const V *AuxHeader,
+                                 uint16_t AuxSize, uint16_t &PartialFieldOffset,
+                                 const char *&PartialFieldName,
+                                 ScopedPrinter &W) {
+  ptrdiff_t Offset = reinterpret_cast<const char *>(&Member) -
+                     reinterpret_cast<const char *>(AuxHeader);
+  if (Offset + sizeof(Member) <= AuxSize)
+    Style == Hex ? W.printHex(MemberName, Member)
+                 : W.printNumber(MemberName, Member);
+  else if (Offset < AuxSize) {
+    PartialFieldOffset = Offset;
+    PartialFieldName = MemberName;
+  }
+}
+
+template <class T>
+void checkAndPrintAuxHeaderParseError(const char *PartialFieldName,
+                                      uint16_t PartialFieldOffset,
+                                      uint16_t AuxSize, T &AuxHeader,
+                                      XCOFFDumper *Dumper) {
+  if (PartialFieldOffset < AuxSize) {
+    Dumper->reportUniqueWarning(Twine("only partial field for ") +
+                                PartialFieldName + " at offset (" +
+                                Twine(PartialFieldOffset) + ")");
+    Dumper->getScopedPrinter().printBinary(
+        "Raw data", "",
+        ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(&AuxHeader) +
+                              PartialFieldOffset,
+                          AuxSize - PartialFieldOffset));
+  } else if (sizeof(AuxHeader) < AuxSize)
+    Dumper->getScopedPrinter().printBinary(
+        "Extra raw data", "",
+        ArrayRef<uint8_t>(reinterpret_cast<const uint8_t *>(&AuxHeader) +
+                              sizeof(AuxHeader),
+                          AuxSize - sizeof(AuxHeader)));
+}
+
 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);
+  auto PrintAuxMember = [&](PrintStyle Style, const char *MemberName,
+                            auto &Member) {
+    printAuxMemberHelper(Style, MemberName, Member, AuxHeader, AuxSize,
+                         PartialFieldOffset, PartialFieldName, W);
+  };
+
+  PrintAuxMember(Hex, "Magic", AuxHeader->AuxMagic);
+  PrintAuxMember(Hex, "Version", AuxHeader->Version);
+  PrintAuxMember(Hex, "Size of .text section", AuxHeader->TextSize);
+  PrintAuxMember(Hex, "Size of .data section", AuxHeader->InitDataSize);
+  PrintAuxMember(Hex, "Size of .bss section", AuxHeader->BssDataSize);
+  PrintAuxMember(Hex, "Entry point address", AuxHeader->EntryPointAddr);
+  PrintAuxMember(Hex, ".text section start address", AuxHeader->TextStartAddr);
+  PrintAuxMember(Hex, ".data section start address", AuxHeader->DataStartAddr);
+  PrintAuxMember(Hex, "TOC anchor address", AuxHeader->TOCAnchorAddr);
+  PrintAuxMember(Number, "Section number of entryPoint",
+                 AuxHeader->SecNumOfEntryPoint);
+  PrintAuxMember(Number, "Section number of .text", AuxHeader->SecNumOfText);
+  PrintAuxMember(Number, "Section number of .data", AuxHeader->SecNumOfData);
+  PrintAuxMember(Number, "Section number of TOC", AuxHeader->SecNumOfTOC);
+  PrintAuxMember(Number, "Section number of loader data",
+                 AuxHeader->SecNumOfLoader);
+  PrintAuxMember(Number, "Section number of .bss", AuxHeader->SecNumOfBSS);
+  PrintAuxMember(Hex, "Maxium alignment of .text", AuxHeader->MaxAlignOfText);
+  PrintAuxMember(Hex, "Maxium alignment of .data", AuxHeader->MaxAlignOfData);
+  PrintAuxMember(Hex, "Module type", AuxHeader->ModuleType);
+  PrintAuxMember(Hex, "CPU type of objects", AuxHeader->CpuFlag);
+  PrintAuxMember(Hex, "(Reserved)", AuxHeader->CpuType);
+  PrintAuxMember(Hex, "Maximum stack size", AuxHeader->MaxStackSize);
+  PrintAuxMember(Hex, "Maximum data size", AuxHeader->MaxDataSize);
+  PrintAuxMember(Hex, "Reserved for debugger", AuxHeader->ReservedForDebugger);
+  PrintAuxMember(Hex, "Text page size", AuxHeader->TextPageSize);
+  PrintAuxMember(Hex, "Data page size", AuxHeader->DataPageSize);
+  PrintAuxMember(Hex, "Stack page size", AuxHeader->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);
+  PrintAuxMember(Number, "Section number for .tdata", AuxHeader->SecNumOfTData);
+  PrintAuxMember(Number, "Section number for .tbss", AuxHeader->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
+  checkAndPrintAuxHeaderParseError(PartialFieldName, PartialFieldOffset,
+                                   AuxSize, *AuxHeader, this);
 }
 
 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);
+  auto PrintAuxMember = [&](PrintStyle Style, const char *MemberName,
+                            auto &Member) {
+    printAuxMemberHelper(Style, MemberName, Member, AuxHeader, AuxSize,
+                         PartialFieldOffset, PartialFieldName, W);
+  };
+
+  PrintAuxMember(Hex, "Magic", AuxHeader->AuxMagic);
+  PrintAuxMember(Hex, "Version", AuxHeader->Version);
+  PrintAuxMember(Hex, "Reserved for debugger", AuxHeader->ReservedForDebugger);
+  PrintAuxMember(Hex, ".text section start address", AuxHeader->TextStartAddr);
+  PrintAuxMember(Hex, ".data section start address", AuxHeader->DataStartAddr);
+  PrintAuxMember(Hex, "TOC anchor address", AuxHeader->TOCAnchorAddr);
+  PrintAuxMember(Number, "Section number of entryPoint",
+                 AuxHeader->SecNumOfEntryPoint);
+  PrintAuxMember(Number, "Section number of .text", AuxHeader->SecNumOfText);
+  PrintAuxMember(Number, "Section number of .data", AuxHeader->SecNumOfData);
+  PrintAuxMember(Number, "Section number of TOC", AuxHeader->SecNumOfTOC);
+  PrintAuxMember(Number, "Section number of loader data",
+                 AuxHeader->SecNumOfLoader);
+  PrintAuxMember(Number, "Section number of .bss", AuxHeader->SecNumOfBSS);
+  PrintAuxMember(Hex, "Maxium alignment of .text", AuxHeader->MaxAlignOfText);
+  PrintAuxMember(Hex, "Maxium alignment of .data", AuxHeader->MaxAlignOfData);
+  PrintAuxMember(Hex, "Module type", AuxHeader->ModuleType);
+  PrintAuxMember(Hex, "CPU type of objects", AuxHeader->CpuFlag);
+  PrintAuxMember(Hex, "(Reserved)", AuxHeader->CpuType);
+  PrintAuxMember(Hex, "Text page size", AuxHeader->TextPageSize);
+  PrintAuxMember(Hex, "Data page size", AuxHeader->DataPageSize);
+  PrintAuxMember(Hex, "Stack page size", AuxHeader->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)));
-  }
+  PrintAuxMember(Hex, "Size of .text section", AuxHeader->TextSize);
+  PrintAuxMember(Hex, "Size of .data section", AuxHeader->InitDataSize);
+  PrintAuxMember(Hex, "Size of .bss section", AuxHeader->BssDataSize);
+  PrintAuxMember(Hex, "Entry point address", AuxHeader->EntryPointAddr);
+  PrintAuxMember(Hex, "Maximum stack size", AuxHeader->MaxStackSize);
+  PrintAuxMember(Hex, "Maximum data size", AuxHeader->MaxDataSize);
+  PrintAuxMember(Number, "Section number for .tdata", AuxHeader->SecNumOfTData);
+  PrintAuxMember(Number, "Section number for .tbss", AuxHeader->SecNumOfTBSS);
+  PrintAuxMember(Hex, "Additional flags 64-bit XCOFF", AuxHeader->XCOFF64Flag);
 
-#undef PrintAuxMember64
+  checkAndPrintAuxHeaderParseError(PartialFieldName, PartialFieldOffset,
+                                   AuxSize, *AuxHeader, this);
 }
 
 template <typename T>
 void XCOFFDumper::printSectionHeaders(ArrayRef<T> Sections) {
   ListScope Group(W, "Sections");
 
   uint16_t Index = 1;
   for (const T &Sec : Sections) {