[XCOFF][AIX] Enable tooling support for 64 bit symbol table parsing

[jasonliu]

Summary

Add in the ability of parsing symbol table for 64 bit object.

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

411

can we add new member function as getNumberOfSymbolTableEntries()
{

return is64Bit() is64Bit() ? getNumberOfSymbolTableEntries64()
              : getLogicalNumberOfSymbolTableEntries32();

}

the function can also use in
XCOFFObjectFile::create()
and
getSymbolNameByIndex()

[jasonliu]

llvm/lib/Object/XCOFFObjectFile.cpp

205

Please see my other comments regarding combining the 32bit and 64 bit version into 1 function.

209

Please see my other comments regarding combining the 32bit and 64 bit version into 1 function.

411

About all the comments mentioning if we could combining the 32bit and 64 bit version into 1 function.
I don't think it's good idea because people would ignore the fact that they are returning different types underneath.

[jhenderson]

In D85774#2216424, @jasonliu wrote:

I just wonder whether we can implement two separate structure XCOFFSymbolEntry32 and XCOFFSymbolEntry64 without so much union be used on currently implement.

Agree that current implementation have many union, and it's hard for people to parse what exactly is inside for the structure.
But separating them into two structures, namely, XCOFFSymbolEntry32 and XCOFFSymbolEntry64, would mean a lot more if (Obj->is64Bit()) check across all tooling, which sacrifice a lot in the usability department. A lot more logic would look duplicated.
One potential solution I thought about is for every data member, we introduce a getter to retrieve the data, and mark the data members private. So that most of the time, user of the structure do not need to look inside of the structure to figure out how to retrieve certain data. But the downside is we are going to introduce a lot of getters for that, and not sure if it would be worth the effort.

It seems to me like this should be using inheritance here. You have a base class that has the common members, and provides pure virtual declarations of the various getters, with the sub-classes defining them to do the right thing. Yes, it would introduce a number of getters, but I feel like it would make everything a bit cleaner from a usability standpoint. In most cases, you then don't need any is64Bit queries, because the getters hide that from you.

On a testing note, there are several places in the new code which detect some kind of error. You need testing for these code paths too.

llvm/lib/Object/XCOFFObjectFile.cpp

411

From my experience working with tools that had to support 32-bit and 64-bit ELF, you don't worry about the underlying type in most cases and always use the larger type. The same probably applies here. Of course, it becomes a bit moot if you add a common getter interface as suggested out-of-line, because those getters will have to return the larger of the two return types anyway.

Is there a strong reason to not use the larger type everywhere?

756

Related to my comments elsewhere - it looks to me like most consumers will need to handle both 32 and 64-bit versions, so they'll always have to do this dance. Thus your concern about how the caller uses them is misplaced - the caller is more likely to do the wrong thing i.e. call the wrong version than have problems with the return types.

792

Better than errorCodeToError(/*some error code*/) is to use createStringError() or createFileError() to provide more context to the failure (how did the parsing fail? where? etc).

807

I don't think you want to use int here. There's always going to be a positive number of entries, and there are no subtractions etc inolving Index here. Better would be an unsigned type of some form (presumably the return type of getNumberOfAuxEntries()).

816

Same comment as before - use createStringError() or createFileError().

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description64.test

14–15

I'm not going to stop you checking in a pre-compiled object, as I'm not an XCOFF maintainer, but as you are continuing to add more functionality here, I strongly advise you to write a yaml2obj XCOFF port, to avoid pre-canned binaries. You'll find pre-built binaries extremely inconvenient to work with as you maintain things going forward. Not only that, but they are harmful to the git repository size, especially if you have to occasionally rebuild them.

Using yaml2obj may also be about the only way you can test most parse failure paths.

If yaml2obj isn't viable, at least consider llvm-mc or similar, if possible.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

412–417

@grimar has gone to a lot of effort to get rid of unwrapOrError from the ELF dumping code. I'd prefer it if we could avoid using it here too. It is generally better in dumping tools to report a warning and abort dumping the current section than to emit an error and terminate the program, since it gives the user more of the information they've asked for.

[hubert.reinterpretcast]

llvm/lib/Object/XCOFFObjectFile.cpp

411

Is there a strong reason to not use the larger type everywhere?

I don't know what strength this reason has, but we had noticed that some of the tools do not reflect the width of the 32-bit format fields very well (even for relatively uninterpreted output). Where the producer of the binary is under development, developers are better served if the tools emit the correct width for fields in the format.

[jasonliu]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description64.test

14–15

I agree that we would want to move away from pre-canned binaries at some point.
When writing a yaml2obj port, we would still require tools such as llvm-readobj and llvm-objdump to make sure our yaml2obj implementation is correct. So we still have a chicken-or-egg problem here.
I think the current plan is to use pre-canned binary to develop the tooling support first. Then use the verified tooling support to verify XCOFF object file generation from llc. Then we could replace the pre-canned binary with llvm-mc/llc.

[grimar]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

412–417

Yeah. Having unwrapOrError available is my concern. I am trying to cleanup ELF dumper, but other files (e.g. COFF) are still using it, thought ideally I'd just remove this API from llvm-readobj code, it seems does more harm than good for a long term.
At least I'd be happy if people stop adding more calls to the code.

[DiggerLin]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

the name must be consistent with the aix OS file syms.h?
what about the to change to AUX_EXCEPT . it consistent with our current style.

[jasonliu]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

The current style in this file seems to be (correct me if I'm wrong):
Use a descriptive name if it's not directly taken from OS header. Otherwise, take the name directly from header and add detailed description/comment to it.
In this case, it follows the latter. Other enums member do not have '_' because OS version do not have it either.

[jasonliu]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

412–417

Thanks. Agreed. Will avoid using unwrapOrError in future code.

[hubert.reinterpretcast]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

The issue here is that the name from the OS header is a reserved name. So by reason of not wanting undefined behaviour, we cannot use the name taken from the OS header. Unfortunately, that means we cannot be consistent in terms of using the name taken from the OS header without switching all the enumerator names to be descriptive and in the LLVM style.

[jasonliu]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

hmm... Is the namespace XCOFF not enough to prevent the undefined behavior happening? Or are we afraid of people just use using namespace XCOFF to defeat it?

[hubert.reinterpretcast]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

The practical cause of undefined behaviour in such a case is usually that the instance of the identifier here is misparsed, or otherwise has surprising behaviour, either because it is defined as a macro or is an extension keyword. Whether the name turns out to be in scope elsewhere is not a factor for such mechanisms.

[jasonliu]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

Got it. I will switch the style of SymbolAuxType in the next revision. We will need to come up with a plan to switch the rest of the classes in this file (There are a lot).

[jhenderson]

llvm/include/llvm/BinaryFormat/XCOFF.h

300

+1 to dropping the underscore. I think it's okay for that to be the only change, but have no strong opinion either way, so happy with whichever you prefer.

305

Not that it really matters, but it's more traiditional to order enums in ascending numerical order. Any particular reason you've done this in the reverse order?

llvm/lib/Object/XCOFFObjectFile.cpp

411

In think in the context of printing the appropriately formatted output, you'd want to switch on the source type (i.e. is64Bit or whatever), at the formatting time. Certainly, this is how we've done it in our own internal code bases I work on, and there are examples of this in a number of other LLVM utilities. For example in https://github.com/llvm/llvm-project/blob/master/llvm/lib/DebugInfo/DWARF/DWARFCompileUnit.cpp#L17, the dump function dumps the offset with a width according to the DWARF format (i.e. 32 or 64 bit), but the getLength function returns a 64-bit value always. Similarly https://github.com/llvm/llvm-project/blob/master/llvm/tools/llvm-objdump/ELFDump.cpp#L257 identifies the ELF format kind and uses that in determining the width of offset, size and address fields (which are stored as uint64_t) when printing ELF program header tables.

There are certainly plenty of places where this hasn't been done. Sometimes this is a mistake, other times it's for consistency with GNU output, but I think the preferred approach is the "store large, explicitly specify format on output" approach.

[jhenderson]

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description64.test

14–15

Yeah, chicken-or-egg problem is a bit of an issue. I'm not sure there's always a clear answer to this. The one I've encouraged for yaml2obj DWARF support testing is to actually inspect the hex output (with sufficient additional commenting to make it clear what the output represents). By keeping the initial functionality small enough, you can boostrap up from there.

The issue is that a lot of our low-level tool testing (i.e. testing of things like llvm-readobj) has switched over to yaml2obj, but clearly we can't (in theory) then use llvm-readobj to test the basic output of yaml2obj or we end up with a circular test dependency - a bug in a common library might not obviously manifest itself in this context, but would if using a tool from outside the ecosystem.

Another strategy which I've used occasionally for testing DWARF parsing before the yaml2obj support existed was writing assembly using just .byte/.quad etc directives to craft the input format precisely, without relying on the higher-level assembly directives (like .file/.loc etc). This may not work in all situations though.

[jasonliu]

llvm/include/llvm/BinaryFormat/XCOFF.h

305

I sort of "copied" the list from the OS header, and that's just the order it appeared in OS header.
I don't think it's particular important to have it in current order, I could change it to ascending order.

[jhenderson]

llvm/include/llvm/BinaryFormat/XCOFF.h

305

I haven't got any particular preference, so am happy to defer to whatever you prefer on this one.

[jasonliu]

Addresses comments.
Add in test case to test errors.
Use view/reference class to encapsulate 32-bit and 64-bit differences instead.

[jasonliu]

In D85774#2217649, @jhenderson wrote:

It seems to me like this should be using inheritance here. You have a base class that has the common members, and provides pure virtual declarations of the various getters, with the sub-classes defining them to do the right thing. Yes, it would introduce a number of getters, but I feel like it would make everything a bit cleaner from a usability standpoint. In most cases, you then don't need any is64Bit queries, because the getters hide that from you.

@jhenderson I tried to use inheritance as suggested. But inheritance would mean I need to use pointers to enable the runtime polymorphism. Then there is a life time issue that need to be managed when using pointers. The easier way to achieve that is to return via unique_ptr. But using unique_ptr introduced usability issue in the caller/user side, as we would see std::move, SymbolRef.get() before getting to the query we want. Also underneath of the unique_ptr, new/delete is not very efficient as well.
In the end, I tried to solve this in similar manner as COFF does, which is using a view class without inheritance. Although the downside of it is we basically have an if query in every call to the view class to differentiate which version (32/64) we are having right now, the good thing is that caller side is much more cleaner.

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

158

change to
return Entry32 ? Entry32->ParameterHashIndex : Entry64->ParameterHashIndex

and change in the following functions too ?

176

change to return reinterpret_cast<uintptr_t>(Entry32 ? Entry32 : Entry64)

400

this means for getSymbolEntryAddressByIndex(uint32_t SymbolTableIndex) const ?

484

assert(OwningObjectPtr != nullptr) here ?

495–528

maybe we can use a macro here.
#define GETVALUE(X) Entry32 ? Entry32->X : Entry64 ->X

int16_t getSectionNumber() const {

return GETVALUE(SectionNumber);

}

uint16_t getSymbolType() const {

   return GETVALUE(SymbolType);
}

and so on

535

getAddress() may confuse with getting the address of the symbol. maybe good to rename to getEntryAddress() ?

llvm/lib/Object/XCOFFObjectFile.cpp

600

several place use above NumberOfSymTableEntries , maybe good to provide a helper function.

757

change to const uint64_t SymbolTableSize ?

823–825

not all the symbol has Csect entry.
what about to return
Optional<XCOFFCsectAuxRef>XCOFFSymbolRef::getXCOFFCsectAuxRef()

827

I think assert(isCsectSymbol()) myabe better.
sometime maybe our developer call getXCOFFCsectAuxRef() at a no CsectSymbol . it is not a object file parse failed.

llvm/tools/llvm-objdump/XCOFFDump.cpp

49

I can not see benefit to change
from
XCOFFSymbolRef SymRef(Sym.getRawDataRefImpl(), Obj);
to
XCOFFSymbolRef SymRef = Obj->toSymbolRef(Sym.getRawDataRefImpl());

[jasonliu]

llvm/include/llvm/Object/XCOFFObjectFile.h

158

I was initially worried about MSVC breakage here: https://github.com/llvm/llvm-project/commit/210314ae8c59bc0a8801c2528eda892cd5960c31
But after taking a closer look, it seems to be only a problem for conversion from ubig32_t value to a unit64_t, which does not apply here.
So I will give it a try.

[jasonliu]

llvm/lib/Object/XCOFFObjectFile.cpp

823–825

I returned Expected<XCOFFCsectAuxRef> partly because of the original comment on this function:
TODO: The function needs to return an error if there is no csect auxiliary
entry.

I believe that's a change from your previous commit. Is there any reason that you changed your mind?
In 32 bit mode, you could have a csect symbol, but without any auxiliary entry. That should return an error (which I haven't detected here, but I should).
Also, in 64 bit mode, it's possible that you have a csect symbol that has auxiliary entries, but do not have a csect auxiliary entry, that should be an error situation right? So it also makes sense to return error in that case.
I think the interface would be too complicated if we return an Expected wrap with an Optional, or the other way around.

827

Sure.

llvm/tools/llvm-objdump/XCOFFDump.cpp

49

I did it for consistency reason, i.e: always get XCOFFSymbolRef via toSymbolRef.

[jasonliu]

llvm/include/llvm/Object/XCOFFObjectFile.h

400

Sorry, what's the difference between Symbol and SymbolEntry?
I'm also seeing getSymbolIndex and getSymbolNameByIndex around this function. Any reason they are not "SymbolEntry"?

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

400

the value of symbol maybe a symbol relocation address , I was confused getSymbolAddress with getting the relocation address at my first glance of the code, getSymbolEntryAddress, that means we need the SymbolEntry address not relocation address of a symbol.

[jasonliu]

llvm/include/llvm/Object/XCOFFObjectFile.h

400

I don't think we have a symbol relocation address IMO. The address related to relocation could be relocation entry's address, or the virtual address data member inside of a relocation entry. But those addresses are not related to symbol in any ways.
Also, there is a function from base class which we overrides here called getSymbolAddress, which we don't want to change. So it would make sense to keep the same naming style here.

535

Do you still find it confusing after seeing my other comments about Symbol vs SymbolEntry?
I would think it's fine since we don't really have other addresses we could get in here.

[jasonliu]

llvm/include/llvm/Object/XCOFFObjectFile.h

400

hmm... Just realized what you meant.
getSymbolAddress actually returns toSymbolRef.getValue() which is a relocatable address.
This function is suppose to return the address of the symbol table entry within the object file.

535

Will change.

[jasonliu]

Address comments from Digger.

[MaskRay]

llvm/include/llvm/Object/XCOFFObjectFile.h

145

You probably don't need these assert. The dereference will crash anyway

[hubert.reinterpretcast]

llvm/include/llvm/Object/XCOFFObjectFile.h

145

That's not true. There are some number of addressable bytes containing 0 starting from address 0x0 on AIX.

[DiggerLin]

llvm/include/llvm/Object/XCOFFObjectFile.h

158

thanks let me know.

171

what about return reinterpret_cast<uintptr_t>(Entry32 ? Entry32 : Entry64) ?

202

ruse GETVALUE(SymbolAlignmentAndType) ?

425

not sure we want to Distance to be negative value future? I think change to int32_t Distance, means that we can backward

435

not sure whether we want to define a enum for the LanguageID in this patch.
The values for this field are defined in the e_lang field in "Exception Section"

484

"Symbol table pointer can not be nullptr!" --> "Symbol table entry pointer can not be nullptr!"

493

using GETVALUE(Value) for consistent ?

llvm/lib/Object/XCOFFObjectFile.cpp

223

const int16_t SectNum ?

498

const int16_t SectionNum ?

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

805–812

const int16_t SectNum

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

839–845

create a static function getSymbolAuxType in a file scope maybe better?
All the Aux symbol of 64bit all has the AuxType . we can use the function for other type too in other place later ?

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

797

if we not enable -ffunction-sections , function entry is label.

839–845

there already has a function on XCOFFCsectAuxRef ::getAuxType64()

[Esme]

Hi Jason, what's your plan about the patch? When will you move forward with it?
Currently D100375 relies on the patch for its 64-bit llvm-readobj. In additional, D97656, D99164 and D98003 should be rebased on it.
Besides, I would like to add support for the line number dump in llvm-objdump if this patch is ready.
It looks like this is a fundamental patch for tools implementation. And it looks good except for some comments haven't been addressed yet.
Please let me know if I can be of any help. Thanks!

[jasonliu]

@Esme I will rebase and address comments asap.

[jasonliu]

Rebase and Address comments.

llvm/include/llvm/Object/XCOFFObjectFile.h

171

No, you could not do that. It only works if Entry32 and Entry64 are the same type. But they are not here.

425

I don't see a need to jump backward now. If it's needed in the future, we could always change in the future patch.

435

I think we are already doing enum mapping in tools/llvm-readobj/XCOFFDumper.cpp. I don't see a strong need to create an enum for it.

493

I would prefer to be more explicit here because we are doing a conversion to larger value for 32 bit version, which is different from the rest of GETVALUE(Value).

llvm/lib/Object/XCOFFObjectFile.cpp

797

Thanks. I brought back the old behavior and added the FIXME to say that this function does not return a correct value if we have -ffunction-sections enabled.

839–845

Yes, there is a XCOFFCsectAuxRef ::getAuxType64(), but if you notice, this function is used to create an XCOFFCsectAuxRef object. So you don't have that function available in the creator.
And I don't think a static function is needed, because when you created XCOFFCsectAuxRef object through this function, then you could call the XCOFFCsectAuxRef ::getAuxType64() to get your type. So this lambda should only exists in this function.

[jasonliu]

Address clang-tidy comment and added binary file.

[jasonliu]

Ping.

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

831

Is this error user-facing (I'm assuming so)? Assuming it is, you should record here which symbol is causing the problem. Otherwise the user will be faced with an error along the lines of this:

error: this csec symbol contains no auxiliary entry

which is not really actionable (imagine the input had 100000 symbols in - the user can't realistically go through each to find the offending one).

848

Similar to my above comment - which entry was not found? Give the user more context so that they can act on the problem.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

416

This will write the error inline, rather than to stderr. Are you sure that's what you want? it isn't what most dumping tools do on failure.

[jasonliu]

Address comments.

[jhenderson]

llvm/lib/Object/XCOFFObjectFile.cpp

831

I believe this requires std::move(Error);, as you're returning an Expected, not an Error.

835–836

I think it would make more sense to insert the name in the middle of the message to make it a bit more concise. Something like:
"csect symbol name contains no auxiliary entry"

863

I'd suggest quoting somehow the symbol name, so that any whitespace or similar that happens to end up in the name (rare, but possible to write using assembly, at least for other platforms) is easily understood to be part of the name.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

414–415

Use reportError or reportWarning so that the error is reported in a clean manner and consistent with other llvm-readobj varieties, and not report_fatal_error which looks like a crash. General rule of thumb: try to avoid using report_fatal_error, especially in tool code where it is easy to report errors properly.

In llvm-readobj for ELF, we try to avoid even using reportError where possible, as that stops the tool from continuing dumping, which can be problematic occasionally. We prefer reportWarning (or more specifically the local reportUniqueWarning which avoids reporting the same warning multiple times) and bailing out of the current routine. Take a look at ELFDumper.cpp for examples.

[jasonliu]

Address comments.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

414–415

Thanks for the elaboration. That clears up things a lot for me. I will use reportUniqueWarning here.

[jhenderson]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

414–415

reportUniqueWarning can take an Error directly, so you can just do:

if (!ErrOrCsectAuxRef)
  reportUniqueWarning(ErrOrCsectAuxRef.takeError());

Also, be careful, as the program continues, so referencing ErrOrCsectAuxRef after this may result in things going wrong...

[jasonliu]

Address comments.

[jasonliu]

@jhenderson @DiggerLin @Esme
Any more comments?

[jhenderson]

In D85774#2738962, @jasonliu wrote:

@jhenderson @DiggerLin @Esme
Any more comments?

I'll try to take another look in the next day or two. The pre-merge bots are failing. Is that an issue with this patch?

[jasonliu]

In D85774#2738969, @jhenderson wrote:

In D85774#2738962, @jasonliu wrote:

@jhenderson @DiggerLin @Esme
Any more comments?

I'll try to take another look in the next day or two. The pre-merge bots are failing. Is that an issue with this patch?

Thanks a lot.
I think it's more of an issue that the binaries did not get into the phabricator probably. Not really an issue with the patch itself.

[jhenderson]

Only some minor nits, otherwise LGTM. I haven't attempted to review the test coverage for all the new code, as I don't feel like I'm in a good position to do that, not being an XCOFF developer. I assume someone else has though.

llvm/include/llvm/Object/XCOFFObjectFile.h

483

485

llvm/lib/Object/XCOFFObjectFile.cpp

797

808

Basically any time you use consumeError, add a comment explaining why it's justified that we don't report the error to the user.

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

390–391

this is only for the 32bits .
" By convention, the csect auxiliary entry in an XCOFF32 file must be the last auxiliary entry for any external symbol that has more than one auxiliary entry"

for 64bit, it maybe look for the x_auxtype ==AUX_CSECT

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

187

if (AuxEntPtr->AuxType != XCOFF::AUX_FILE ) , it should not be parsed as
XCOFF::AUX_FILE
it may better to print out raw data in the printSymbol()

236

if (AuxEntPtr->AuxType != XCOFF::AUX_CSECT) , it should not be parsed as
XCOFF::AUX_CSECT above
it may better to print out raw data in the printSymbol()

[jasonliu]

Address comments.

[jasonliu]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

187

I think it's the caller's responsibility to make sure they are passing in the right auxiliary type. So we should assume when we enter this function, we have the right auxiliary type here. So I modified it and made it an assert instead.

236

Same above. I modified it to be an assertion instead.

390–391

Good point. Updated the code.

[DiggerLin]

llvm/lib/Object/XCOFFObjectFile.cpp

94

SymbolAuxType is only for the 64 bits.
add assert(is64Bit() ) before return ?

llvm/tools/llvm-readobj/XCOFFDumper.cpp

378

as you mention "I think it's the caller's responsibility to make sure they are passing in the right auxiliary type. "

I think we need to check the AuxType == XCOFF::AUX_FILE for 64 bits. if not , print out the raw data as AUX_CSECT did ?

[DiggerLin]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

416

we have iterated auxiliary entries from line 382~396 and reiterated again in the printCsectAuxEnt(). I think we can improve on it.

And in 64bits, The auxiliary entries maybe be reordered in above implement.
it will print out all no AUX_CSECT auxiliary entries first and then AUX_CSECT entry, even if the AUX_CSECT is in the middle of the auxiliary entries
and if there are two AUX_CSECT on auxiliary entries, we only print out one.

[jasonliu]

llvm/tools/llvm-readobj/XCOFFDumper.cpp

416

I don't think we reiterated again in printCsectAuxEnt() for other auxiliary entries.
We did similar things for 382~396 in getXCOFFCsectAuxRef, but that function is specifically designed to only get XCOFFCsectAuxRef. So it's really not intended to pass information out there.
I don't really think the re-iteration should be a big concern because in theory 382~396 won't be executed all that much. If it does, I would rather have it implement properly (i.e. actually recognize the missing auxiliary entry) instead of just printing out raw datas.

[DiggerLin]

LGTM with address comment.

llvm/tools/llvm-readobj/XCOFFDumper.cpp

366

it need "continue;"
here

[DiggerLin]

and please run clang-format

[jasonliu]

Address comments.

[jhenderson]

I took a quick look at the pre-merge output. The message is that the object isn't recognised as a valid object file, not that it wasn't found, which suggests either the object or code is broken in some manner, or you're missing a dependent patch. Does this patch depend on another patch that isn't in main yet?

llvm/tools/llvm-readobj/XCOFFDumper.cpp

357–372

i -> I

(you're changing most of the loop body - you might as well fix this whilst you're here)

363

Test case?

392–394

My English language ping went off at the word "till" in these two sentences. I'd probably change it to "to". Also, use "first" rather than "1st", I suggest in both places.

Also "skips" -> "skip" for grammatical consistency.

396

i -> I

[jasonliu]

Address comments.

[jasonliu]

In D85774#2758956, @jhenderson wrote:

I took a quick look at the pre-merge output. The message is that the object isn't recognised as a valid object file, not that it wasn't found, which suggests either the object or code is broken in some manner, or you're missing a dependent patch. Does this patch depend on another patch that isn't in main yet?

No this patch does not depend on other patches. I think this is caused by git generated binary for the patch doesn't really assemble to the same one?

[jhenderson]

In D85774#2760205, @jasonliu wrote:

In D85774#2758956, @jhenderson wrote:

I took a quick look at the pre-merge output. The message is that the object isn't recognised as a valid object file, not that it wasn't found, which suggests either the object or code is broken in some manner, or you're missing a dependent patch. Does this patch depend on another patch that isn't in main yet?

No this patch does not depend on other patches. I think this is caused by git generated binary for the patch doesn't really assemble to the same one?

Sounds vaguely plausible, I guess.

llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test

2–3

Fix a couple of grammar issues and a premature line-wrap.

5–6

20

The output on this line looks incorrect to me. Possibly a bug in the code resulting from a missing newe line?

[jasonliu]

Address comments.

[jasonliu]

llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test

20

I added an newline after all the raw bytes. Other than that, I think the output is good. We have 18 bytes per symbol table entry, and we are printing 18 raw bytes here.

[jhenderson]

llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test

20

I don't know if it is, but you probably want 00fb indented to line up nicely with the previous line. You can then confirm that this indentation is maintained by enabling --match-full-lines and --strict-whitespace in FileCheck. (If you do that, you'll need to remove the space after CHECK-NEXT:)

[jasonliu]

Adjustment the print out.

[jasonliu]

llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test

20

Hi James, I adjusted the code to print 00fb in the same line, I think that actually works better because we could have multiply auxiliary entry data to print out. Putting each in the same line is easier to parse.

[jhenderson]

Looks good again. Sorry for the delay - I was off work last week.

[jasonliu]

In D85774#2801934, @jhenderson wrote:

Looks good again. Sorry for the delay - I was off work last week.

No worries. Thanks for all the reviews.

llvm/include/llvm/BinaryFormat/XCOFF.h

 };
 
 enum CFileCpuId : uint8_t {
   TCPU_PPC64 = 2, ///< PowerPC common architecture 64-bit mode.
   TCPU_COM = 3,   ///< POWER and PowerPC architecture common.
   TCPU_970 = 19   ///< PPC970 - PowerPC 64-bit architecture.
 };
 
+enum SymbolAuxType : uint8_t {
+  AUX_EXCEPT = 255, ///< Identifies an exception auxiliary entry.
+  AUX_FCN = 254,    ///< Identifies a function auxiliary entry.
+  AUX_SYM = 253,    ///< Identifies a symbol auxiliary entry.
+  AUX_FILE = 252,   ///< Identifies a file auxiliary entry.
+  AUX_CSECT = 251,  ///< Identifies a csect auxiliary entry.
+  AUX_SECT = 250    ///< Identifies a SECT auxiliary entry.
+};                  // 64-bit XCOFF file only.
+
 StringRef getMappingClassString(XCOFF::StorageMappingClass SMC);
 StringRef getRelocationTypeString(XCOFF::RelocationType Type);
 SmallString<32> parseParmsType(uint32_t Value, unsigned ParmsNum);
 
 struct TracebackTable {
   enum LanguageID : uint8_t {
     C,
     Fortran,

llvm/include/llvm/Object/XCOFFObjectFile.h

struct XCOFFSectionHeader64 : XCOFFSectionHeader<XCOFFSectionHeader64> {
   support::big64_t FileOffsetToRelocationInfo;
   support::big64_t FileOffsetToLineNumberInfo;
   support::ubig32_t NumberOfRelocations;
   support::ubig32_t NumberOfLineNumbers;
   support::big32_t Flags;
   char Padding[4];
 };
 
-struct XCOFFSymbolEntry {
-  enum { NAME_IN_STR_TBL_MAGIC = 0x0 };
-  typedef struct {
-    support::big32_t Magic; // Zero indicates name in string table.
-    support::ubig32_t Offset;
-  } NameInStrTblType;
-
-  typedef struct {
-    uint8_t LanguageId;
-    uint8_t CpuTypeId;
-  } CFileLanguageIdAndTypeIdType;
-
-  union {
-    char SymbolName[XCOFF::NameSize];
-    NameInStrTblType NameInStrTbl;
-  };
-
-  support::ubig32_t Value; // Symbol value; storage class-dependent.
-  support::big16_t SectionNumber;
-
-  union {
-    support::ubig16_t SymbolType;
-    CFileLanguageIdAndTypeIdType CFileLanguageIdAndTypeId;
-  };
-
-  XCOFF::StorageClass StorageClass;
-  uint8_t NumberOfAuxEntries;
-};
-
 struct XCOFFStringTable {
   uint32_t Size;
   const char *Data;
 };
 
 struct XCOFFCsectAuxEnt32 {
+  support::ubig32_t SectionOrLength;
+  support::ubig32_t ParameterHashIndex;
+  support::ubig16_t TypeChkSectNum;
+  uint8_t SymbolAlignmentAndType;
+  XCOFF::StorageMappingClass StorageMappingClass;
+  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:
   static constexpr uint8_t SymbolTypeMask = 0x07;
   static constexpr uint8_t SymbolAlignmentMask = 0xF8;
   static constexpr size_t SymbolAlignmentBitOffset = 3;
 
-  support::ubig32_t
-      SectionOrLength; // If the symbol type is XTY_SD or XTY_CM, the csect
-                       // length.
-                       // If the symbol type is XTY_LD, the symbol table
-                       // index of the containing csect.
-                       // If the symbol type is XTY_ER, 0.
-  support::ubig32_t ParameterHashIndex;
-  support::ubig16_t TypeChkSectNum;
-  uint8_t SymbolAlignmentAndType;
-  XCOFF::StorageMappingClass StorageMappingClass;
-  support::ubig32_t StabInfoIndex;
-  support::ubig16_t StabSectNum;
+  XCOFFCsectAuxRef(const XCOFFCsectAuxEnt32 *Entry32) : Entry32(Entry32) {}
+  XCOFFCsectAuxRef(const XCOFFCsectAuxEnt64 *Entry64) : Entry64(Entry64) {}
+
+  // For getSectionOrLength(),
+  // If the symbol type is XTY_SD or XTY_CM, the csect length.
+  uint64_t getSectionOrLength() const {
+    return Entry32 ? getSectionOrLength32() : getSectionOrLength64();
+  }
+
+  uint32_t getSectionOrLength32() const {
+    assert(Entry32 && "32-bit interface called on 64-bit object file.");
+    return Entry32->SectionOrLength;
+  }
+
+  uint64_t getSectionOrLength64() const {
+    assert(Entry64 && "64-bit interface called on 32-bit object file.");
+    return (static_cast<uint64_t>(Entry64->SectionOrLengthHighByte) << 32) |
+           Entry64->SectionOrLengthLowByte;
+  }
+
+#define GETVALUE(X) Entry32 ? Entry32->X : Entry64->X
+
+  uint32_t getParameterHashIndex() const {
+    return GETVALUE(ParameterHashIndex);
+  }
+
+  uint16_t getTypeChkSectNum() const { return GETVALUE(TypeChkSectNum); }
+
+  XCOFF::StorageMappingClass getStorageMappingClass() const {
+    return GETVALUE(StorageMappingClass);
+  }
+
+  uintptr_t getEntryAddress() const {
+    return Entry32 ? reinterpret_cast<uintptr_t>(Entry32)
+                   : reinterpret_cast<uintptr_t>(Entry64);
+  }
 
   uint16_t getAlignmentLog2() const {
-    return (SymbolAlignmentAndType & SymbolAlignmentMask) >>
+    return (getSymbolAlignmentAndType() & SymbolAlignmentMask) >>
            SymbolAlignmentBitOffset;
   }
 
   uint8_t getSymbolType() const {
-    return SymbolAlignmentAndType & SymbolTypeMask;
+    return getSymbolAlignmentAndType() & SymbolTypeMask;
   }
 
   bool isLabel() const { return getSymbolType() == XCOFF::XTY_LD; }
+
+  uint32_t getStabInfoIndex32() const {
+    assert(Entry32 && "32-bit interface called on 64-bit object file.");
+    return Entry32->StabInfoIndex;
+  }
+
+  uint16_t getStabSectNum32() const {
+    assert(Entry32 && "32-bit interface called on 64-bit object file.");
+    return Entry32->StabSectNum;
+  }
+
+  XCOFF::SymbolAuxType getAuxType64() const {
+    assert(Entry64 && "64-bit interface called on 32-bit object file.");
+    return Entry64->AuxType;
+  }
+
+private:
+  uint8_t getSymbolAlignmentAndType() const {
+    return GETVALUE(SymbolAlignmentAndType);
+  }
+
+#undef GETVALUE
+
+  const XCOFFCsectAuxEnt32 *Entry32 = nullptr;
+  const XCOFFCsectAuxEnt64 *Entry64 = nullptr;
 };
 
 struct XCOFFFileAuxEnt {
   typedef struct {
     support::big32_t Magic; // Zero indicates name in string table.
     support::ubig32_t Offset;
     char NamePad[XCOFF::FileNamePadSize];
   } NameInStrTblType;
   union {
     char Name[XCOFF::NameSize + XCOFF::FileNamePadSize];
     NameInStrTblType NameInStrTbl;
   };
   XCOFF::CFileStringType Type;
   uint8_t ReservedZeros[2];
-  uint8_t AuxType; // 64-bit XCOFF file only.
+  XCOFF::SymbolAuxType AuxType; // 64-bit XCOFF file only.
 };
 
 struct XCOFFSectAuxEntForStat {
   support::ubig32_t SectionLength;
   support::ubig16_t NumberOfRelocEnt;
   support::ubig16_t NumberOfLineNum;
   uint8_t Pad[10];
-};
+}; // 32-bit XCOFF file only.
 
 struct XCOFFRelocation32 {
   // Masks for packing/unpacking the r_rsize field of relocations.
 
   // The msb is used to indicate if the bits being relocated are signed or
   // unsigned.
   static constexpr uint8_t XR_SIGN_INDICATOR_MASK = 0x80;
 

 public:
   bool isRelocationSigned() const;
   bool isFixupIndicated() const;
 
   // Returns the number of bits being relocated.
   uint8_t getRelocatedLength() const;
 };
 
+class XCOFFSymbolRef;
+
 class XCOFFObjectFile : public ObjectFile {
 private:
   const void *FileHeader = nullptr;
   const void *SectionHeaderTable = nullptr;
 
-  const XCOFFSymbolEntry *SymbolTblPtr = 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;
 
   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;
 
   // This returns a pointer to the start of the storage for the name field of
   // the 32-bit or 64-bit SectionHeader struct. This string is *not* necessarily
   // null-terminated.
   const char *getSectionNameInternal(DataRefImpl Sec) const;
 
-  // This function returns string table entry.
-  Expected<StringRef> getStringTableEntry(uint32_t Offset) const;
-
   static bool isReservedSectionNumber(int16_t SectionNumber);
 
   // Constructor and "create" factory function. The constructor is only a thin
   // wrapper around the base constructor. The "create" function fills out the
   // XCOFF-specific information and performs the error checking along the way.
   XCOFFObjectFile(unsigned Type, MemoryBufferRef Object);
   static Expected<std::unique_ptr<XCOFFObjectFile>> create(unsigned Type,
                                                            MemoryBufferRef MBR);
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 XCOFFSymbolEntry *getPointerToSymbolTable() const {
-    assert(!is64Bit() && "Symbol table handling not supported yet.");
-    return SymbolTblPtr;
-  }
+  const void *getPointerToSymbolTable() const { return SymbolTblPtr; }
 
-  Expected<StringRef>
-  getSymbolSectionName(const XCOFFSymbolEntry *SymEntPtr) const;
+  Expected<StringRef> getSymbolSectionName(XCOFFSymbolRef Ref) const;
 
-  const XCOFFSymbolEntry *toSymbolEntry(DataRefImpl Ref) const;
+  XCOFFSymbolRef toSymbolRef(DataRefImpl Ref) const;
 
   // File header related interfaces.
   uint16_t getMagic() const;
   uint16_t getNumberOfSections() const;
   int32_t getTimeStamp() const;
 
   // Symbol table offset and entry count are handled differently between
   // XCOFF32 and XCOFF64.
   uint32_t getSymbolTableOffset32() const;
   uint64_t getSymbolTableOffset64() const;
 
   // Note that this value is signed and might return a negative value. Negative
   // values are reserved for future use.
   int32_t getRawNumberOfSymbolTableEntries32() const;
 
   // The sanitized value appropriate to use as an index into the symbol table.
   uint32_t getLogicalNumberOfSymbolTableEntries32() const;
 
   uint32_t getNumberOfSymbolTableEntries64() const;
+
+  // Return getLogicalNumberOfSymbolTableEntries32 or
+  // getNumberOfSymbolTableEntries64 depending on the object mode.
+  uint32_t getNumberOfSymbolTableEntries() const;
+
   uint32_t getSymbolIndex(uintptr_t SymEntPtr) const;
+  uintptr_t getSymbolEntryAddressByIndex(uint32_t SymbolTableIndex) const;
   Expected<StringRef> getSymbolNameByIndex(uint32_t SymbolTableIndex) const;
 
   Expected<StringRef> getCFileName(const XCOFFFileAuxEnt *CFileEntPtr) const;
   uint16_t getOptionalHeaderSize() const;
   uint16_t getFlags() const;
 
   // Section header table related interfaces.
   ArrayRef<XCOFFSectionHeader32> sections32() const;
   ArrayRef<XCOFFSectionHeader64> sections64() const;
 
   int32_t getSectionFlags(DataRefImpl Sec) const;
   Expected<DataRefImpl> getSectionByNum(int16_t Num) const;
 
   void checkSymbolEntryPointer(uintptr_t SymbolEntPtr) const;
 
   // Relocation-related interfaces.
   Expected<uint32_t>
   getLogicalNumberOfRelocationEntries(const XCOFFSectionHeader32 &Sec) const;
 
   Expected<ArrayRef<XCOFFRelocation32>>
   relocations(const XCOFFSectionHeader32 &) const;
 
+  // This function returns string table entry.
+  Expected<StringRef> getStringTableEntry(uint32_t Offset) const;
+
+  const XCOFF::SymbolAuxType *getSymbolAuxType(uintptr_t AuxEntryAddress) const;
+
+  static uintptr_t getAdvancedSymbolEntryAddress(uintptr_t CurrentAddress,
+                                                 uint32_t Distance);
+
   static bool classof(const Binary *B) { return B->isXCOFF(); }
 }; // XCOFFObjectFile
 
-class XCOFFSymbolRef {
-  const DataRefImpl SymEntDataRef;
-  const XCOFFObjectFile *const OwningObjectPtr;
+typedef struct {
+  uint8_t LanguageId;
+  uint8_t CpuTypeId;
+} CFileLanguageIdAndTypeIdType;
+
+struct XCOFFSymbolEntry32 {
+  typedef struct {
+    support::big32_t Magic; // Zero indicates name in string table.
+    support::ubig32_t Offset;
+  } NameInStrTblType;
+
+  union {
+    char SymbolName[XCOFF::NameSize];
+    NameInStrTblType NameInStrTbl;
+  };
+
+  support::ubig32_t Value; // Symbol value; storage class-dependent.
+  support::big16_t SectionNumber;
+
+  union {
+    support::ubig16_t SymbolType;
+    CFileLanguageIdAndTypeIdType CFileLanguageIdAndTypeId;
+  };
 
+  XCOFF::StorageClass StorageClass;
+  uint8_t NumberOfAuxEntries;
+};
+
+struct XCOFFSymbolEntry64 {
+  support::ubig64_t Value; // Symbol value; storage class-dependent.
+  support::ubig32_t Offset;
+  support::big16_t SectionNumber;
+
+  union {
+    support::ubig16_t SymbolType;
+    CFileLanguageIdAndTypeIdType CFileLanguageIdAndTypeId;
+  };
+
+  XCOFF::StorageClass StorageClass;
+  uint8_t NumberOfAuxEntries;
+};
+
+class XCOFFSymbolRef {
 public:
+  enum { NAME_IN_STR_TBL_MAGIC = 0x0 };
+
   XCOFFSymbolRef(DataRefImpl SymEntDataRef,
                  const XCOFFObjectFile *OwningObjectPtr)
-      : SymEntDataRef(SymEntDataRef), OwningObjectPtr(OwningObjectPtr){};
+      : OwningObjectPtr(OwningObjectPtr) {
+    assert(OwningObjectPtr && "OwningObjectPtr cannot be nullptr!");
+    assert(SymEntDataRef.p != 0 &&
+           "Symbol table entry pointer cannot be nullptr!");
+
+    if (OwningObjectPtr->is64Bit())
+      Entry64 = reinterpret_cast<const XCOFFSymbolEntry64 *>(SymEntDataRef.p);
+    else
+      Entry32 = reinterpret_cast<const XCOFFSymbolEntry32 *>(SymEntDataRef.p);
+  }
+
+  uint64_t getValue() const { return Entry32 ? getValue32() : getValue64(); }
+
+  uint32_t getValue32() const { return Entry32->Value; }
+
+  uint64_t getValue64() const { return Entry64->Value; }
+
+#define GETVALUE(X) Entry32 ? Entry32->X : Entry64->X
+
+  int16_t getSectionNumber() const { return GETVALUE(SectionNumber); }
+
+  uint16_t getSymbolType() const { return GETVALUE(SymbolType); }
+
+  uint8_t getLanguageIdForCFile() const {
+    assert(getStorageClass() == XCOFF::C_FILE &&
+           "This interface is for C_FILE only.");
+    return GETVALUE(CFileLanguageIdAndTypeId.LanguageId);
+  }
+
+  uint8_t getCPUTypeIddForCFile() const {
+    assert(getStorageClass() == XCOFF::C_FILE &&
+           "This interface is for C_FILE only.");
+    return GETVALUE(CFileLanguageIdAndTypeId.CpuTypeId);
+  }
+
+  XCOFF::StorageClass getStorageClass() const { return GETVALUE(StorageClass); }
+
+  uint8_t getNumberOfAuxEntries() const { return GETVALUE(NumberOfAuxEntries); }
 
-  XCOFF::StorageClass getStorageClass() const;
-  uint8_t getNumberOfAuxEntries() const;
-  const XCOFFCsectAuxEnt32 *getXCOFFCsectAuxEnt32() const;
-  uint16_t getType() const;
-  int16_t getSectionNumber() const;
+#undef GETVALUE
 
-  bool hasCsectAuxEnt() const;
+  uintptr_t getEntryAddress() const {
+    return Entry32 ? reinterpret_cast<uintptr_t>(Entry32)
+                   : reinterpret_cast<uintptr_t>(Entry64);
+  }
+
+  Expected<StringRef> getName() const;
   bool isFunction() const;
+  bool isCsectSymbol() const;
+  Expected<XCOFFCsectAuxRef> getXCOFFCsectAuxRef() const;
+
+private:
+  const XCOFFObjectFile *OwningObjectPtr;
+  const XCOFFSymbolEntry32 *Entry32 = nullptr;
+  const XCOFFSymbolEntry64 *Entry64 = nullptr;
 };
 
 class TBVectorExt {
   friend class XCOFFTracebackTable;
 
   uint16_t Data;
   uint32_t VecParmsInfo;
 

llvm/lib/Object/XCOFFObjectFile.cpp

 
 namespace llvm {
 
 using namespace XCOFF;
 
 namespace object {
 
 static const uint8_t FunctionSym = 0x20;
-static const uint8_t SymTypeMask = 0x07;
 static const uint16_t NoRelMask = 0x0001;
+static const size_t SymbolAuxTypeOffset = 17;
 
 // Checks that [Ptr, Ptr + Size) bytes fall inside the memory buffer
 // 'M'. Returns a pointer to the underlying object on success.
 template <typename T>
 static Expected<const T *> getObject(MemoryBufferRef M, const void *Ptr,
                                      const uint64_t Size = sizeof(T)) {
   uintptr_t Addr = reinterpret_cast<uintptr_t>(Ptr);
   if (Error E = Binary::checkOffset(M, Addr, Size))

 }
 
 uint8_t XCOFFRelocation32::getRelocatedLength() const {
   // The relocation encodes the bit length being relocated minus 1. Add back
   // the 1 to get the actual length being relocated.
   return (Info & XR_BIASED_LENGTH_MASK) + 1;
 }
 
+uintptr_t
+XCOFFObjectFile::getAdvancedSymbolEntryAddress(uintptr_t CurrentAddress,
+                                               uint32_t Distance) {
+  return getWithOffset(CurrentAddress, Distance * XCOFF::SymbolTableEntrySize);
+}
+
+const XCOFF::SymbolAuxType *
+XCOFFObjectFile::getSymbolAuxType(uintptr_t AuxEntryAddress) const {
+  assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
+  return viewAs<XCOFF::SymbolAuxType>(
+      getWithOffset(AuxEntryAddress, SymbolAuxTypeOffset));
+}
+
 void XCOFFObjectFile::checkSectionAddress(uintptr_t Addr,
                                           uintptr_t TableAddress) const {
   if (Addr < TableAddress)
     report_fatal_error("Section header outside of section header table.");
 
   uintptr_t Offset = Addr - TableAddress;
   if (Offset >= getSectionHeaderSize() * getNumberOfSections())
     report_fatal_error("Section header outside of section header table.");

 XCOFFObjectFile::toSection64(DataRefImpl Ref) const {
   assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
 #ifndef NDEBUG
   checkSectionAddress(Ref.p, getSectionHeaderTableAddress());
 #endif
   return viewAs<XCOFFSectionHeader64>(Ref.p);
 }
 
-const XCOFFSymbolEntry *XCOFFObjectFile::toSymbolEntry(DataRefImpl Ref) const {
-  assert(!is64Bit() && "Symbol table support not implemented for 64-bit.");
+XCOFFSymbolRef XCOFFObjectFile::toSymbolRef(DataRefImpl Ref) const {
   assert(Ref.p != 0 && "Symbol table pointer can not be nullptr!");
 #ifndef NDEBUG
   checkSymbolEntryPointer(Ref.p);
 #endif
-  auto SymEntPtr = viewAs<XCOFFSymbolEntry>(Ref.p);
-  return SymEntPtr;
+  return XCOFFSymbolRef(Ref, this);
 }
 
 const XCOFFFileHeader32 *XCOFFObjectFile::fileHeader32() const {
   assert(!is64Bit() && "32-bit interface called on 64-bit object file.");
   return static_cast<const XCOFFFileHeader32 *>(FileHeader);
 }
 
 const XCOFFFileHeader64 *XCOFFObjectFile::fileHeader64() const {

 
 const XCOFFSectionHeader64 *
 XCOFFObjectFile::sectionHeaderTable64() const {
   assert(is64Bit() && "64-bit interface called on a 32-bit object file.");
   return static_cast<const XCOFFSectionHeader64 *>(SectionHeaderTable);
 }
 
 void XCOFFObjectFile::moveSymbolNext(DataRefImpl &Symb) const {
-  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
-  SymEntPtr += SymEntPtr->NumberOfAuxEntries + 1;
+  uintptr_t NextSymbolAddr = getAdvancedSymbolEntryAddress(
+      Symb.p, toSymbolRef(Symb).getNumberOfAuxEntries() + 1);
 #ifndef NDEBUG
   // This function is used by basic_symbol_iterator, which allows to
   // point to the end-of-symbol-table address.
-  if (reinterpret_cast<uintptr_t>(SymEntPtr) != getEndOfSymbolTableAddress())
-    checkSymbolEntryPointer(reinterpret_cast<uintptr_t>(SymEntPtr));
+  if (NextSymbolAddr != getEndOfSymbolTableAddress())
+    checkSymbolEntryPointer(NextSymbolAddr);
 #endif
-  Symb.p = reinterpret_cast<uintptr_t>(SymEntPtr);
+  Symb.p = NextSymbolAddr;
 }
 
 Expected<StringRef>
 XCOFFObjectFile::getStringTableEntry(uint32_t Offset) const {
   // The byte offset is relative to the start of the string table.
   // A byte offset value of 0 is a null or zero-length symbol
   // name. A byte offset in the range 1 to 3 (inclusive) points into the length
   // field; as a soft-error recovery mechanism, we treat such cases as having an
   // offset of 0.
   if (Offset < 4)
     return StringRef(nullptr, 0);
 
   if (StringTable.Data != nullptr && StringTable.Size > Offset)
     return (StringTable.Data + Offset);
 
   return make_error<GenericBinaryError>("Bad offset for string table entry",
                                         object_error::parse_failed);
 }
 
 Expected<StringRef>
 XCOFFObjectFile::getCFileName(const XCOFFFileAuxEnt *CFileEntPtr) const {
-  if (CFileEntPtr->NameInStrTbl.Magic !=
-      XCOFFSymbolEntry::NAME_IN_STR_TBL_MAGIC)
+  if (CFileEntPtr->NameInStrTbl.Magic != XCOFFSymbolRef::NAME_IN_STR_TBL_MAGIC)
     return generateXCOFFFixedNameStringRef(CFileEntPtr->Name);
   return getStringTableEntry(CFileEntPtr->NameInStrTbl.Offset);
 }
 
 Expected<StringRef> XCOFFObjectFile::getSymbolName(DataRefImpl Symb) const {
-  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
-
-  // A storage class value with the high-order bit on indicates that the name is
-  // a symbolic debugger stabstring.
-  if (SymEntPtr->StorageClass & 0x80)
-    return StringRef("Unimplemented Debug Name");
-
-  if (SymEntPtr->NameInStrTbl.Magic != XCOFFSymbolEntry::NAME_IN_STR_TBL_MAGIC)
-    return generateXCOFFFixedNameStringRef(SymEntPtr->SymbolName);
-
-  return getStringTableEntry(SymEntPtr->NameInStrTbl.Offset);
+  return toSymbolRef(Symb).getName();
 }
 
 Expected<uint64_t> XCOFFObjectFile::getSymbolAddress(DataRefImpl Symb) const {
-  assert(!is64Bit() && "Symbol table support not implemented for 64-bit.");
-  return toSymbolEntry(Symb)->Value;
+  return toSymbolRef(Symb).getValue();
 }
 
 uint64_t XCOFFObjectFile::getSymbolValueImpl(DataRefImpl Symb) const {
-  assert(!is64Bit() && "Symbol table support not implemented for 64-bit.");
-  return toSymbolEntry(Symb)->Value;
+  return toSymbolRef(Symb).getValue();
 }
 
 uint64_t XCOFFObjectFile::getCommonSymbolSizeImpl(DataRefImpl Symb) const {
   uint64_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 Expected<SymbolRef::Type>
 XCOFFObjectFile::getSymbolType(DataRefImpl Symb) const {
   llvm_unreachable("Not yet implemented!");
   return SymbolRef::ST_Other;
 }
 
 Expected<section_iterator>
 XCOFFObjectFile::getSymbolSection(DataRefImpl Symb) const {
-  const XCOFFSymbolEntry *SymEntPtr = toSymbolEntry(Symb);
-  int16_t SectNum = SymEntPtr->SectionNumber;
+  const int16_t SectNum = toSymbolRef(Symb).getSectionNumber();
 
   if (isReservedSectionNumber(SectNum))
     return section_end();
 
   Expected<DataRefImpl> ExpSec = getSectionByNum(SectNum);
   if (!ExpSec)
     return ExpSec.takeError();
 
if (is64Bit())
     report_fatal_error("64-bit support not implemented yet");
   const XCOFFRelocation32 *Reloc = viewAs<XCOFFRelocation32>(Rel.p);
   const uint32_t Index = Reloc->SymbolIndex;
 
   if (Index >= getLogicalNumberOfSymbolTableEntries32())
     return symbol_end();
 
   DataRefImpl SymDRI;
-  SymDRI.p = reinterpret_cast<uintptr_t>(getPointerToSymbolTable() + Index);
+  SymDRI.p = getSymbolEntryAddressByIndex(Index);
   return symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 uint64_t XCOFFObjectFile::getRelocationType(DataRefImpl Rel) const {
   if (is64Bit())
     report_fatal_error("64-bit support not implemented yet");
   return viewAs<XCOFFRelocation32>(Rel.p)->Type;
 }

 
 Expected<uint32_t> XCOFFObjectFile::getSymbolFlags(DataRefImpl Symb) const {
   uint32_t Result = 0;
   llvm_unreachable("Not yet implemented!");
   return Result;
 }
 
 basic_symbol_iterator XCOFFObjectFile::symbol_begin() const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
   DataRefImpl SymDRI;
   SymDRI.p = reinterpret_cast<uintptr_t>(SymbolTblPtr);
   return basic_symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 basic_symbol_iterator XCOFFObjectFile::symbol_end() const {
-  if (is64Bit())
-    report_fatal_error("64-bit support not implemented yet");
   DataRefImpl SymDRI;
-  SymDRI.p = reinterpret_cast<uintptr_t>(
-      SymbolTblPtr + getLogicalNumberOfSymbolTableEntries32());
+  const uint32_t NumberOfSymbolTableEntries = getNumberOfSymbolTableEntries();
+  SymDRI.p = getSymbolEntryAddressByIndex(NumberOfSymbolTableEntries);
   return basic_symbol_iterator(SymbolRef(SymDRI, this));
 }
 
 section_iterator XCOFFObjectFile::section_begin() const {
   DataRefImpl DRI;
   DRI.p = getSectionHeaderTableAddress();
   return section_iterator(SectionRef(DRI, this));
 }
 
Expected<DataRefImpl> XCOFFObjectFile::getSectionByNum(int16_t Num) const {
 
   DataRefImpl DRI;
   DRI.p = getWithOffset(getSectionHeaderTableAddress(),
                         getSectionHeaderSize() * (Num - 1));
   return DRI;
 }
 
 Expected<StringRef>
-XCOFFObjectFile::getSymbolSectionName(const XCOFFSymbolEntry *SymEntPtr) const {
-  assert(!is64Bit() && "Symbol table support not implemented for 64-bit.");
-  int16_t SectionNum = SymEntPtr->SectionNumber;
+XCOFFObjectFile::getSymbolSectionName(XCOFFSymbolRef SymEntPtr) const {
+  const int16_t SectionNum = SymEntPtr.getSectionNumber();
 
   switch (SectionNum) {
   case XCOFF::N_DEBUG:
     return "N_DEBUG";
   case XCOFF::N_ABS:
     return "N_ABS";
   case XCOFF::N_UNDEF:
     return "N_UNDEF";

 uint64_t XCOFFObjectFile::getSymbolTableOffset64() const {
   return fileHeader64()->SymbolTableOffset;
 }
 
 uint32_t XCOFFObjectFile::getNumberOfSymbolTableEntries64() const {
   return fileHeader64()->NumberOfSymTableEntries;
 }
 
-uintptr_t XCOFFObjectFile::getEndOfSymbolTableAddress() const {
-  uint32_t NumberOfSymTableEntries =
-      is64Bit() ? getNumberOfSymbolTableEntries64()
+uint32_t XCOFFObjectFile::getNumberOfSymbolTableEntries() const {
+  return is64Bit() ? getNumberOfSymbolTableEntries64()
                    : getLogicalNumberOfSymbolTableEntries32();
+}
+
+uintptr_t XCOFFObjectFile::getEndOfSymbolTableAddress() const {
+  const uint32_t NumberOfSymTableEntries = getNumberOfSymbolTableEntries();
   return getWithOffset(reinterpret_cast<uintptr_t>(SymbolTblPtr),
                        XCOFF::SymbolTableEntrySize * NumberOfSymTableEntries);
 }
 
 void XCOFFObjectFile::checkSymbolEntryPointer(uintptr_t SymbolEntPtr) const {
   if (SymbolEntPtr < reinterpret_cast<uintptr_t>(SymbolTblPtr))
     report_fatal_error("Symbol table entry is outside of symbol table.");
 

 }
 
 uint32_t XCOFFObjectFile::getSymbolIndex(uintptr_t SymbolEntPtr) const {
   return (reinterpret_cast<const char *>(SymbolEntPtr) -
           reinterpret_cast<const char *>(SymbolTblPtr)) /
          XCOFF::SymbolTableEntrySize;
 }
 
+uintptr_t XCOFFObjectFile::getSymbolEntryAddressByIndex(uint32_t Index) const {
+  return getAdvancedSymbolEntryAddress(
+      reinterpret_cast<uintptr_t>(getPointerToSymbolTable()), Index);
+}
+
 Expected<StringRef>
 XCOFFObjectFile::getSymbolNameByIndex(uint32_t Index) const {
-  if (is64Bit())
-    report_fatal_error("64-bit symbol table support not implemented yet.");
+  const uint32_t NumberOfSymTableEntries = getNumberOfSymbolTableEntries();
 
-  if (Index >= getLogicalNumberOfSymbolTableEntries32())
+  if (Index >= NumberOfSymTableEntries)
     return errorCodeToError(object_error::invalid_symbol_index);
 
   DataRefImpl SymDRI;
-  SymDRI.p = reinterpret_cast<uintptr_t>(getPointerToSymbolTable() + Index);
+  SymDRI.p = getSymbolEntryAddressByIndex(Index);
   return getSymbolName(SymDRI);
 }
 
 uint16_t XCOFFObjectFile::getFlags() const {
   return is64Bit() ? fileHeader64()->Flags : fileHeader32()->Flags;
 }
 
 const char *XCOFFObjectFile::getSectionNameInternal(DataRefImpl Sec) const {
if (Obj->getNumberOfSections()) {
     auto SecHeadersOrErr = getObject<void>(Data, Base + CurOffset,
                                            Obj->getNumberOfSections() *
                                                Obj->getSectionHeaderSize());
     if (Error E = SecHeadersOrErr.takeError())
       return std::move(E);
     Obj->SectionHeaderTable = SecHeadersOrErr.get();
   }
 
-  // 64-bit object supports only file header and section headers for now.
-  if (Obj->is64Bit())
-    return std::move(Obj);
+  const uint32_t NumberOfSymbolTableEntries =
+      Obj->getNumberOfSymbolTableEntries();
 
   // If there is no symbol table we are done parsing the memory buffer.
-  if (Obj->getLogicalNumberOfSymbolTableEntries32() == 0)
+  if (NumberOfSymbolTableEntries == 0)
     return std::move(Obj);
 
   // Parse symbol table.
-  CurOffset = Obj->fileHeader32()->SymbolTableOffset;
-  uint64_t SymbolTableSize = (uint64_t)(sizeof(XCOFFSymbolEntry)) *
-                             Obj->getLogicalNumberOfSymbolTableEntries32();
+  CurOffset = Obj->is64Bit() ? Obj->getSymbolTableOffset64()
+                             : Obj->getSymbolTableOffset32();
+  const uint64_t SymbolTableSize =
+      static_cast<uint64_t>(XCOFF::SymbolTableEntrySize) *
+      NumberOfSymbolTableEntries;
   auto SymTableOrErr =
-      getObject<XCOFFSymbolEntry>(Data, Base + CurOffset, SymbolTableSize);
+      getObject<void *>(Data, Base + CurOffset, SymbolTableSize);
   if (Error E = SymTableOrErr.takeError())
     return std::move(E);
   Obj->SymbolTblPtr = SymTableOrErr.get();
   CurOffset += SymbolTableSize;
 
   // Parse String table.
   Expected<XCOFFStringTable> StringTableOrErr =
       parseStringTable(Obj.get(), CurOffset);
   if (Error E = StringTableOrErr.takeError())
     return std::move(E);
   Obj->StringTable = StringTableOrErr.get();
 
   return std::move(Obj);
 }
 
 Expected<std::unique_ptr<ObjectFile>>
 ObjectFile::createXCOFFObjectFile(MemoryBufferRef MemBufRef,
                                   unsigned FileType) {
   return XCOFFObjectFile::create(FileType, MemBufRef);
 }
 
-XCOFF::StorageClass XCOFFSymbolRef::getStorageClass() const {
-  return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->StorageClass;
-}
+bool XCOFFSymbolRef::isFunction() const {
+  if (!isCsectSymbol())
+    return false;
 
-uint8_t XCOFFSymbolRef::getNumberOfAuxEntries() const {
-  return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->NumberOfAuxEntries;
-}
+  if (getSymbolType() & FunctionSym)
+    return true;
 
-// TODO: The function needs to return an error if there is no csect auxiliary
-// entry.
-const XCOFFCsectAuxEnt32 *XCOFFSymbolRef::getXCOFFCsectAuxEnt32() const {
-  assert(!OwningObjectPtr->is64Bit() &&
-         "32-bit interface called on 64-bit object file.");
-  assert(hasCsectAuxEnt() && "No Csect Auxiliary Entry is found.");
+  Expected<XCOFFCsectAuxRef> ExpCsectAuxEnt = getXCOFFCsectAuxRef();
+  if (!ExpCsectAuxEnt)
+    return false;
 
-  // In XCOFF32, the csect auxilliary entry is always the last auxiliary
-  // entry for the symbol.
-  uintptr_t AuxAddr = getWithOffset(
-      SymEntDataRef.p, XCOFF::SymbolTableEntrySize * getNumberOfAuxEntries());
+  const XCOFFCsectAuxRef CsectAuxRef = ExpCsectAuxEnt.get();
 
-#ifndef NDEBUG
-  OwningObjectPtr->checkSymbolEntryPointer(AuxAddr);
-#endif
+  // A function definition should be a label definition.
+  // FIXME: This is not necessarily the case when -ffunction-sections is
+  // enabled.
+  if (!CsectAuxRef.isLabel())
+    return false;
 
-  return reinterpret_cast<const XCOFFCsectAuxEnt32 *>(AuxAddr);
-}
+  if (CsectAuxRef.getStorageMappingClass() != XCOFF::XMC_PR)
+    return false;
 
-uint16_t XCOFFSymbolRef::getType() const {
-  return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->SymbolType;
+  const int16_t SectNum = getSectionNumber();
+  Expected<DataRefImpl> SI = OwningObjectPtr->getSectionByNum(SectNum);
+  if (!SI) {
+    // If we could not get the section, then this symbol should not be
+    // a function. So consume the error and return `false` to move on.
+    consumeError(SI.takeError());
+    return false;
   }
 
-int16_t XCOFFSymbolRef::getSectionNumber() const {
-  return OwningObjectPtr->toSymbolEntry(SymEntDataRef)->SectionNumber;
+  return (OwningObjectPtr->getSectionFlags(SI.get()) & XCOFF::STYP_TEXT);
 }
 
-// TODO: The function name needs to be changed to express the purpose of the
-// function.
-bool XCOFFSymbolRef::hasCsectAuxEnt() const {
+bool XCOFFSymbolRef::isCsectSymbol() const {
   XCOFF::StorageClass SC = getStorageClass();
   return (SC == XCOFF::C_EXT || SC == XCOFF::C_WEAKEXT ||
           SC == XCOFF::C_HIDEXT);
 }
 
-bool XCOFFSymbolRef::isFunction() const {
-  if (OwningObjectPtr->is64Bit())
-    report_fatal_error("64-bit support is unimplemented yet.");
+Expected<XCOFFCsectAuxRef> XCOFFSymbolRef::getXCOFFCsectAuxRef() const {
+  assert(isCsectSymbol() &&
+         "Calling csect symbol interface with a non-csect symbol.");
 
-  if (getType() & FunctionSym)
-    return true;
+  uint8_t NumberOfAuxEntries = getNumberOfAuxEntries();
 
-  if (!hasCsectAuxEnt())
-    return false;
+  Expected<StringRef> NameOrErr = getName();
+  if (auto Err = NameOrErr.takeError())
+    return std::move(Err);
 
-  const XCOFFCsectAuxEnt32 *CsectAuxEnt = getXCOFFCsectAuxEnt32();
+  if (!NumberOfAuxEntries) {
+    return createStringError(object_error::parse_failed,
+                             "csect symbol \"" + *NameOrErr +
+                                 "\" contains no auxiliary entry");
+  }
 
-  // A function definition should be a label definition.
-  if ((CsectAuxEnt->SymbolAlignmentAndType & SymTypeMask) != XCOFF::XTY_LD)
-    return false;
+  if (!OwningObjectPtr->is64Bit()) {
+    // In XCOFF32, the csect auxilliary entry is always the last auxiliary
+    // entry for the symbol.
+    uintptr_t AuxAddr = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
+        getEntryAddress(), NumberOfAuxEntries);
+    return XCOFFCsectAuxRef(viewAs<XCOFFCsectAuxEnt32>(AuxAddr));
+  }
 
-  if (CsectAuxEnt->StorageMappingClass != XCOFF::XMC_PR)
-    return false;
+  // XCOFF64 uses SymbolAuxType to identify the auxiliary entry type.
+  // We need to iterate through all the auxiliary entries to find it.
+  for (uint8_t Index = NumberOfAuxEntries; Index > 0; --Index) {
+    uintptr_t AuxAddr = XCOFFObjectFile::getAdvancedSymbolEntryAddress(
+        getEntryAddress(), Index);
+    if (*OwningObjectPtr->getSymbolAuxType(AuxAddr) ==
+        XCOFF::SymbolAuxType::AUX_CSECT) {
+#ifndef NDEBUG
+      OwningObjectPtr->checkSymbolEntryPointer(AuxAddr);
+#endif
+      return XCOFFCsectAuxRef(viewAs<XCOFFCsectAuxEnt64>(AuxAddr));
+    }
+  }
 
-  int16_t SectNum = getSectionNumber();
-  Expected<DataRefImpl> SI = OwningObjectPtr->getSectionByNum(SectNum);
-  if (!SI)
-    return false;
+  return createStringError(
+      object_error::parse_failed,
+      "a csect auxiliary entry is not found for symbol \"" + *NameOrErr + "\"");
+}
 
-  return (OwningObjectPtr->getSectionFlags(SI.get()) & XCOFF::STYP_TEXT);
+Expected<StringRef> XCOFFSymbolRef::getName() const {
+  // A storage class value with the high-order bit on indicates that the name is
+  // a symbolic debugger stabstring.
+  if (getStorageClass() & 0x80)
+    return StringRef("Unimplemented Debug Name");
+
+  if (Entry32) {
+    if (Entry32->NameInStrTbl.Magic != XCOFFSymbolRef::NAME_IN_STR_TBL_MAGIC)
+      return generateXCOFFFixedNameStringRef(Entry32->SymbolName);
+
+    return OwningObjectPtr->getStringTableEntry(Entry32->NameInStrTbl.Offset);
+  }
+
+  return OwningObjectPtr->getStringTableEntry(Entry64->Offset);
 }
 
 // Explictly instantiate template classes.
 template struct XCOFFSectionHeader<XCOFFSectionHeader32>;
 template struct XCOFFSectionHeader<XCOFFSectionHeader64>;
 
 bool doesXCOFFTracebackTableBegin(ArrayRef<uint8_t> Bytes) {
   if (Bytes.size() < 4)

llvm/test/tools/llvm-objdump/XCOFF/disassemble-symbol-description64.test

+# REQUIRES: powerpc-registered-target
+
+# RUN: llvm-objdump -D %p/Inputs/xcoff-section-headers64.o | \
+# RUN:   FileCheck --check-prefixes=COMMON,PLAIN %s
+
+# RUN: llvm-objdump -D --symbol-description %p/Inputs/xcoff-section-headers64.o | \
+# RUN:   FileCheck --check-prefixes=COMMON,DESC %s
+
+# RUN: not --crash llvm-objdump -D -r --symbol-description %p/Inputs/xcoff-section-headers64.o 2>&1 | \
+# RUN:   FileCheck --check-prefix=ERROR %s
+# ERROR: 64-bit support not implemented yet
+
+## xcoff-section-headers64.o Compiled with IBM XL C/C++ for AIX, V16.1.0
+## compiler command: xlc -q64 -qtls -o xcoff-section-headers64.o -c test.c
+
+## test.c:
+## int a;
+## int b = 12345;
+## __thread int c;
+## __thread double d = 3.14159;
+##
+## int func(void)  {
+##   return a;
+## }
+
+COMMON:       Inputs/xcoff-section-headers64.o: file format aix5coff64-rs6000
+COMMON:       Disassembly of section .text:
+COMMON-EMPTY:
+PLAIN:        0000000000000000 <.func>:
+DESC:         0000000000000000 (idx: 6) .func:
+COMMON-NEXT:         0: e8 62 00 08   ld 3, 8(2)
+COMMON-NEXT:         4: e8 63 00 02   lwa 3, 0(3)
+COMMON-NEXT:         8: 4e 80 00 20   blr
+COMMON-NEXT:         c: 00 00 00 00   <unknown>
+COMMON-NEXT:        10: 00 00 20 40   <unknown>
+COMMON-NEXT:        14: 00 00 00 01   <unknown>
+COMMON-NEXT:        18: 00 00 00 0c   <unknown>
+COMMON-NEXT:        1c: 00 04 66 75   <unknown>
+COMMON-NEXT:        20: 6e 63 00 00   xoris 3, 19, 0
+COMMON-NEXT:                  ...
+COMMON-EMPTY:
+COMMON-NEXT:  Disassembly of section .data:
+COMMON-EMPTY:
+PLAIN:        0000000000000080 <func>:
+DESC:         0000000000000080 (idx: 12) func[TC]:
+COMMON-NEXT:        80: 00 00 00 00   <unknown>
+COMMON-NEXT:        84: 00 00 00 a8   <unknown>
+COMMON-EMPTY:
+PLAIN:        0000000000000088 <a>:
+DESC:         0000000000000088 (idx: 16) a[TC]:
+COMMON-NEXT:        88: 00 00 00 00   <unknown>
+COMMON-NEXT:        8c: 00 00 00 c8   <unknown>
+COMMON-EMPTY:
+PLAIN:        0000000000000090 <b>:
+DESC:         0000000000000090 (idx: 20) b[TC]:
+COMMON-NEXT:        90: 00 00 00 00   <unknown>
+COMMON-NEXT:        94: 00 00 00 c0   <unknown>
+COMMON-EMPTY:
+PLAIN:        0000000000000098 <c>:
+DESC:         0000000000000098 (idx: 24) c[TC]:
+COMMON-NEXT:        98: 00 00 00 00   <unknown>
+COMMON-NEXT:        9c: 00 00 00 08   <unknown>
+COMMON-EMPTY:
+PLAIN:        00000000000000a0 <d>:
+DESC:         00000000000000a0 (idx: 28) d[TC]:
+COMMON-NEXT:                  ...
+COMMON-EMPTY:
+PLAIN:        00000000000000a8 <func>:
+DESC:         00000000000000a8 (idx: 10) func[DS]:
+COMMON-NEXT:                  ...
+COMMON-NEXT:        b4: 00 00 00 80   <unknown>
+COMMON-NEXT:                  ...
+COMMON-EMPTY:
+PLAIN:        00000000000000c0 <b>:
+DESC:         00000000000000c0 (idx: 18) b[RW]:
+COMMON-NEXT:        c0: 00 00 30 39   <unknown>
+COMMON-NEXT:        c4: 00 00 00 00   <unknown>
+COMMON-EMPTY:
+COMMON-NEXT:  Disassembly of section .bss:
+COMMON-EMPTY:
+PLAIN:        00000000000000c8 <a>:
+DESC:         00000000000000c8 (idx: 14) a[RW]:
+COMMON-NEXT:  ...
+COMMON-EMPTY:
+COMMON-NEXT:  Disassembly of section .tdata:
+COMMON-EMPTY:
+PLAIN:        0000000000000000 <d>:
+DESC:         0000000000000000 (idx: 26) d[TL]:
+COMMON-NEXT:         0: 40 09 21 f9   bdnzfl  9, 0x21f8
+COMMON-NEXT:         4: f0 1b 86 6e   <unknown>
+COMMON-EMPTY:
+COMMON-NEXT:  Disassembly of section .tbss:
+COMMON-EMPTY:
+PLAIN:        0000000000000008 <c>:
+DESC:         0000000000000008 (idx: 22) c[UL]:
+COMMON-NEXT:  ...

llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test

+## This file tests the raw data output ability when a file auxiliary entry does
+## not have the matching auxiliary type.
+
+# RUN: llvm-readobj --symbols %p/Inputs/file-aux-wrong64.o | FileCheck %s
+
+#      CHECK: Symbols [
+# CHECK-NEXT:   Symbol {
+# CHECK-NEXT:     Index: 0
+# CHECK-NEXT:     Name: .file
+# CHECK-NEXT:     Value (SymbolTableIndex): 0x0
+# CHECK-NEXT:     Section: N_DEBUG
+# CHECK-NEXT:     Source Language ID: 0xC
+# CHECK-NEXT:     CPU Version ID: TCPU_PPC64 (0x2)
+# CHECK-NEXT:     StorageClass: C_FILE (0x67)
+# CHECK-NEXT:     NumberOfAuxEntries: 1
+# CHECK-NEXT:     !Unexpected raw auxiliary entry data:
+# CHECK-NEXT:     612e7300 00000000 00000000 00000000 00fb
+# CHECK-NEXT:   }
+# CHECK-NEXT: ]

llvm/test/tools/llvm-readobj/XCOFF/symbols64.test

+## This file tests the ability of llvm-readobj to display the symbol table for a
+## 64-bit XCOFF object file.
+## The object file used is generated by the following source file
+## and command on AIX:
+##
+## > cat test8.c
+##
+## extern int i;
+## extern int TestforXcoff;
+## extern int fun(int i);
+## static int static_i;
+## char* p="abcd";
+## int fun1(int j) {
+##   static_i++;
+##   j++;
+##   j=j+*p;
+##   return j;
+## }
+##
+## int main() {
+##   i++;
+##   fun(i);
+##   return fun1(i);
+## }
+##
+## > xlc -q64 -c test8.c -o symbol64.o
+
+# RUN: llvm-readobj --symbols %p/Inputs/symbol64.o | \
+# RUN:   FileCheck --check-prefix=SYMBOL64 %s
+
+# SYMBOL64:       File: {{.*}}symbol64.o
+# SYMBOL64-NEXT:  Format: aix5coff64-rs6000
+# SYMBOL64-NEXT:  Arch: powerpc64
+# SYMBOL64-NEXT:  AddressSize: 64bit
+# SYMBOL64-NEXT:  Symbols [
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 0
+# SYMBOL64-NEXT:      Name: .file
+# SYMBOL64-NEXT:      Value (SymbolTableIndex): 0x0
+# SYMBOL64-NEXT:      Section: N_DEBUG
+# SYMBOL64-NEXT:      Source Language ID: TB_C (0x0)
+# SYMBOL64-NEXT:      CPU Version ID: TCPU_PPC64 (0x2)
+# SYMBOL64-NEXT:      StorageClass: C_FILE (0x67)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 3
+# SYMBOL64-NEXT:      File Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 1
+# SYMBOL64-NEXT:        Name: test64.c
+# SYMBOL64-NEXT:        Type: XFT_FN (0x0)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_FILE (0xFC)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:      File Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 2
+# SYMBOL64-NEXT:        Name: Mon Aug 10 16:07:48 2020
+# SYMBOL64-NEXT:        Type: XFT_CT (0x1)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_FILE (0xFC)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:      File Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 3
+# SYMBOL64-NEXT:        Name: IBM XL C for AIX, Version 16.1.0.6
+# SYMBOL64-NEXT:        Type: XFT_CV (0x2)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_FILE (0xFC)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:    }
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 4
+# SYMBOL64-NEXT:      Name:
+# SYMBOL64-NEXT:      Value (RelocatableAddress): 0x0
+# SYMBOL64-NEXT:      Section: .text
+# SYMBOL64-NEXT:      Type: 0x0
+# SYMBOL64-NEXT:      StorageClass: C_HIDEXT (0x6B)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 1
+# SYMBOL64-NEXT:      CSECT Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 5
+# SYMBOL64-NEXT:        SectionLen: 256
+# SYMBOL64-NEXT:        ParameterHashIndex: 0x0
+# SYMBOL64-NEXT:        TypeChkSectNum: 0x0
+# SYMBOL64-NEXT:        SymbolAlignmentLog2: 7
+# SYMBOL64-NEXT:        SymbolType: XTY_SD (0x1)
+# SYMBOL64-NEXT:        StorageMappingClass: XMC_PR (0x0)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_CSECT (0xFB)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:    }
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 6
+# SYMBOL64-NEXT:      Name: .fun1
+# SYMBOL64-NEXT:      Value (RelocatableAddress): 0x0
+# SYMBOL64-NEXT:      Section: .text
+# SYMBOL64-NEXT:      Type: 0x20
+# SYMBOL64-NEXT:      StorageClass: C_EXT (0x2)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 1
+# SYMBOL64-NEXT:      CSECT Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 7
+# SYMBOL64-NEXT:        ContainingCsectSymbolIndex: 4
+# SYMBOL64-NEXT:        ParameterHashIndex: 0x0
+# SYMBOL64-NEXT:        TypeChkSectNum: 0x0
+# SYMBOL64-NEXT:        SymbolAlignmentLog2: 0
+# SYMBOL64-NEXT:        SymbolType: XTY_LD (0x2)
+# SYMBOL64-NEXT:        StorageMappingClass: XMC_PR (0x0)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_CSECT (0xFB)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:    }
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 8
+# SYMBOL64-NEXT:      Name: .main
+# SYMBOL64-NEXT:      Value (RelocatableAddress): 0x80
+# SYMBOL64-NEXT:      Section: .text
+# SYMBOL64-NEXT:      Type: 0x20
+# SYMBOL64-NEXT:      StorageClass: C_EXT (0x2)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 1
+# SYMBOL64-NEXT:      CSECT Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 9
+# SYMBOL64-NEXT:        ContainingCsectSymbolIndex: 4
+# SYMBOL64-NEXT:        ParameterHashIndex: 0x0
+# SYMBOL64-NEXT:        TypeChkSectNum: 0x0
+# SYMBOL64-NEXT:        SymbolAlignmentLog2: 0
+# SYMBOL64-NEXT:        SymbolType: XTY_LD (0x2)
+# SYMBOL64-NEXT:        StorageMappingClass: XMC_PR (0x0)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_CSECT (0xFB)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:    }
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 10
+# SYMBOL64-NEXT:      Name: TOC
+# SYMBOL64-NEXT:      Value (RelocatableAddress): 0x100
+# SYMBOL64-NEXT:      Section: .data
+# SYMBOL64-NEXT:      Type: 0x0
+# SYMBOL64-NEXT:      StorageClass: C_HIDEXT (0x6B)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 1
+# SYMBOL64-NEXT:      CSECT Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 11
+# SYMBOL64-NEXT:        SectionLen: 0
+# SYMBOL64-NEXT:        ParameterHashIndex: 0x0
+# SYMBOL64-NEXT:        TypeChkSectNum: 0x0
+# SYMBOL64-NEXT:        SymbolAlignmentLog2: 2
+# SYMBOL64-NEXT:        SymbolType: XTY_SD (0x1)
+# SYMBOL64-NEXT:        StorageMappingClass: XMC_TC0 (0xF)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_CSECT (0xFB)
+# SYMBOL64-NEXT:      }
+# SYMBOL64-NEXT:    }
+# SYMBOL64-NEXT:    Symbol {
+# SYMBOL64-NEXT:      Index: 12
+# SYMBOL64-NEXT:      Name:
+# SYMBOL64-NEXT:      Value (RelocatableAddress): 0x128
+# SYMBOL64-NEXT:      Section: .data
+# SYMBOL64-NEXT:      Type: 0x0
+# SYMBOL64-NEXT:      StorageClass: C_HIDEXT (0x6B)
+# SYMBOL64-NEXT:      NumberOfAuxEntries: 1
+# SYMBOL64-NEXT:      CSECT Auxiliary Entry {
+# SYMBOL64-NEXT:        Index: 13
+# SYMBOL64-NEXT:        SectionLen: 8
+# SYMBOL64-NEXT:        ParameterHashIndex: 0x0
+# SYMBOL64-NEXT:        TypeChkSectNum: 0x0
+# SYMBOL64-NEXT:        SymbolAlignmentLog2: 3
+# SYMBOL64-NEXT:        SymbolType: XTY_SD (0x1)
+# SYMBOL64-NEXT:        StorageMappingClass: XMC_TC (0x3)
+# SYMBOL64-NEXT:        Auxiliary Type: AUX_CSECT (0xFB)
+# SYMBOL64-NEXT:      }