There is filename in the reportWarning in the llvm-objdump, I do not want the file name show in the warning message . I added a new function "AddOffsetToValueInString" to convert the error message with the address from begin of object file.

Can this function be static too?

Please address these clang-tidy warnings by following LLVM coding standards.

If for some reason, you want to deliberately differ to what llvm-objdump does normally when printing warnings and not print the tool name, you could add an optional argument to reportWarning to suppress it.

You can follow this same technique to not print the file name too. Why don't you want the file name in the message though? If the error stream is being redirected to a file, you won't know where the warning is coming from if you are working with multiple files or archives.

Also, please fix the other whitespace and grammar issues, as I requested in my previous comment.

raw_ostream &operator<<(unsigned char C); exists. Take a look at raw_ostream.h. However, it may print the value as a character, not as an integer. How about using the format function to format your strings, using printf formatting, and PRIx8 to ensure the right type is being used without the need for casting? DWARFDebugLine.cpp has some good examples of that sort of thing.

I still use the const std::string &StrMsg as parameter here, for int Num = std::stoi(NumString, 0, 16) and Res += std::string(StrMsg, PrePos, Pos - PrePos); we still need to convert the StringRef to std:string.

I noticed that, on AIX, we can do:
objdump --private=traceback xcoff-traceback-table.o to print out traceback table alone.
But with llvm-objdump, we are not possible to do that since we emit traceback table info alone with each byte.
I am wondering are we intentional to not support using --private=traceback alone with llvm-objdump?

Should we use xlclang here?

minor: Please use ++Pos to avoid extra copy of variable value.

minor:

I am wondering what's the point of getting a uint8_t and cast it to TracebackTable::LanguageID? Can we adjust the interface to the following?

TracebackTable::LanguageID getLanguageID() const;

minor: also add a blank line above this line

minor:

Is that intentional that we omit printing out function name after the first no more than 4 bytes?

This line is causing build failure when I build. Should it be getVectorParmsInfoString?

The same question as Jason already mentioned? Please use c++ style cast static_cast if you have to. And also is that possible to adjust the return value of getExtensionTable() to ExtendedTBTableFlag enum type?

minor: s/padding/paddings

Can we add testcase to get line 351 - line 359 get tested?

May I ask why we don't put these two functions in objdump namespace as well?

llvm-objdump do not support --private= now.

yes, of course, we can use xlclang here, but I think using xlc is ok too.

of course , we can do as your suggestion. but I prefer use raw data type uint8_t as return type of getLanguageID() , and if we change the interface , there still need static_cast<TracebackTable::LanguageID> in the getLanguageID() , if you strong suggest it, I can change the interface.

We do not omit printing out the function name after the first no more 4 bytes.
what we do here is:
For hex, we print every four bytes in a line for function name . we print out whole function name in string in the first line of hex format as comment.
for example:

CHECK-NEXT: da: 66 6f 6f 6f # FunctionName = fooooo
CHECK-NEXT: de: 6f 6f
CHECK-NEXT: e0: 1f # AllocaRegister = 31

the patch has parent patch. https://reviews.llvm.org/D93659 , I think need to apply the parent patch before compile this patch.

the call stack as
PrettyPrinter::printInst() ->printRawData() ->getInstStartColumn()

the class PrettyPrinter is defined in anonymous namespace and these two new function only used by the class PrettyPrinter. So I define two function as the same namespace as PrettyPrinter .

sorry . I change the above comment
these two new function only used by the class PrettyPrinter
to
these two new function is used by the class PrettyPrinter

I think you should be able to use the alignTo function to do this alignment calculation. However, it looks like the Cursor and DataExtractor classes are missing any form of seek function to set the position directly, without needing to calculate from the current location. I think it would be useful to add this function to one or the other. You could then do something like one of the following:

Cur.seek(alignTo(Cur.tell(), 4));
DE.seek(Cur, alignTo(Cur.tell(), 4));

which I'm sure you'll agree is much clearer and less likely to have any issues.

Is there any particular reason you can't fold this in with the other cases?

Use - not _ in your prefix names, as per the prevailing style in this file.

This won't work. NEXT suffixes must be prefixed by a - not _.

Delete blank lines at file end.

1. This test needs additional comments explaining what the test is trying to test. How is the input invalid?
2. You should invest time in adding functionality for writing traceback tables to yaml2obj, so you can use it to generate the invalid output, rather than checking in an opaque and impossible to maintain binary (most people won't have access to the xlc compiler, for example, should there be a problem with the test).

Here and below.

Get rid of additional blank lines.

See my above comments - add traceback table generation to yaml2obj rather than adding canned binaries that people are unable to update.

Aside from the also incorrect --symbol-description help text, the help text for llvm-objdump options does not include a trailing full stop. Please remove.

Don't use std::stringstream. Use the LLVM raw_string_ostream class instead.

There are plenty of LLVM functions for string concatenation and manipulation using Twine and StringRef. Please use those, not std::string. Convert only to a std::string on return.

If you haven't already, please read the LLVM Programmer's Manual, especially the section here: https://llvm.org/docs/ProgrammersManual.html#passing-strings-the-stringref-and-twine-classes

That all being said, I don't think you should do any of this - see my comment below.

Macros are a pain to debug typically, if nothing else, and could cause problems later in the file (unlikely, but possible), unless you undef them.

So I really don't like the need to parse the error message for numbers and updating them. Aside from anything else, it's possible that a future message will include other numbers in it, unrelated to the address, so this function would incorrectly change them.

I assume you want to make it clear that the error message offsets are relative to the start of the traceback table. I think the following would be a way of clarifying:
"failure parsing traceback table with address 0x1234\n>>> " (etc)
And then leave the remaining numbers untouched. This helps because I think in most cases, it will be fairly obvious that the offsets in the error messages are relative to the address, because they won't make sense otherwise (e.g. they'll be smaller than the table's address). The raw data will also have the leading address values, which again will not usually line up with the offset, although you could also consider having the leading address actually be the offset value within the traceback table instead, whichever is simpler.

Finally, I don't think we need to worry too much about any risk of confusion - hopefully this warning will never be seen by users, and when it is, it should be straightforward enough to figure things out from context. Therefore, I think we can afford to keep things simple.

Remember to put a full stop at the end of sentences.

Shorter and more grammatically correct name. You "print" something usually, not "print out" something.

Why do we need another 4 bytes printed here? Explain why in comments, not what (it is clear from the code what is happening).

These functions are in a public header. Anybody who includes "llvm-objdump.h" whill get these two functions without any form of namespace scoping which is very undesirable.

Either 1) if the functions are only used in one file, move their definition into that file, or 2) if the functions are used in multiple places, put them in the llvm namespace at the bare minimum (and since you are in objdump, they should be in the llvm::objdump namespace.

I repeat my previous comment. How is this invalid?

Thsi comment is marked as done, but hasn't been addressed as far as I can see?

I assume?

Lambdas are objects, so should be named as such.

good idea, thanks. I will create a separate patch for it. https://reviews.llvm.org/D109603 . after the patch approve, I will change the code based on the new patch.

these one generate the object file. other generate asm.

thanks

thanks

agree with you. but not this moment, the yaml2obj not support the auxiliary entry for the symbol table, parsing traceback table depend on it.

in the XCOFFEmitter.cpp : line 332
"// TODO: Auxiliary entry is not supported yet."

agree with you. same comment as above.

thanks

thanks

I will add some new functionality to XCOFFEmitter.cpp before I can create a xcoff object file I can use here.