• update

• update

• Make FramePointerKind enum class.
• Replace two if-else-if blocks to switch statements.

• Address reviewer's comments.

In D64674#1584157, @MaskRay wrote:

Do you find another use case of printDynamicString? If yes, can you name it?

• Replace printDynamicString with getDynamicString.
• Change tests accordingly.

• rebase after D64294.

LGTM

• update

• update

Thank you for capturing that. The added new line in the previous revision is for GNU output. I should have done what LLVM output did: add a prefixing new line.

• update

In D56353, I remember @chandlerc thought -f(no-)omit-frame-pointer should win over -m(no-)omit-leaf-frame-pointer. I'm not sure what his thoughts on this now. @chandlerc ?

• update

• update

• update

In D63521#1563367, @MaskRay wrote:

In D63521#1562497, @ychen wrote:

Hi @MaskRay, from @rnk 's inputs, seems this is not a common case worth pursuing at least for the moment, is it ok to move forward with the first choice I described?

My comment on your first version of this patch is about the potential code duplication. Now the codeview concern of the code duplication has been addressed.

The current version doesn't look bad to me. Do you think the initial version is better?

The expected client for this new symbolizeCode interface is for archive members and COFF is unlikely to symbolize plain object files for the moment.
I think the concern here the code movement for removing the potential duplication puts some untested code in the path for DWARF. I assume if there is a need for the COFF archive member symbolization we could easily deduplicate the code in the patch for COFF?

• update

• update

Hi @MaskRay, from @rnk 's inputs, seems this is not a common case worth pursuing at least for the moment, is it ok to move forward with the first choice I described?

• update

• update

I don't think it's a loss of functionality versus the previous version which creates DWARFContext even for COFF object (not work obviously). This version still does not work for COFF archive but looks a little bit more reasonable. IMHO, we should have no or all support for COFF archive, not partial support. I could either

• update

• update

• update

In D63524#1555266, @jhenderson wrote:

Hi @ychen, would you mind posting a patch to update the docs for this behaviour change? I didn't mention it previously, because rL364019, which added --disassemble-functions to the listed options, was still under review, but it landed before this change did.

• address reviewer's comments.

Hi @MaskRay, does this version look good to you?

To help writing up the discussion thread, I'm trying to gather some data on how each target handle the case with GNU objdump. Weird that I could not reproduce the results @MaskRay @peter.smith was able to obtain. This is not to show favor for either choice but to understand the current situation on GNU side so we make a sensible decision.

@MaskRay @jhenderson @peter.smith Thank you for the comments. I'll start a thread to collect more opinions.

• update

Thank you @MaskRay. Description changed. I intentionally leave out -t since I suspect it is a bug. See my comment in https://bugs.llvm.org/show_bug.cgi?id=41911.

• update

• update

In D63280#1552108, @MaskRay wrote:

Sorry for doing this. It is very late in my timezone, but I think I have to mark it as "Request Changes" as I saw it accepted and I really don't want it to be committed...

If you all agree bar2@plt-0x20 is less ideal and our .plt is better, I'm not sure why you want to copy the behavior of GNU objdump. I believe this belongs to the aesthetical area where 100% compatibility is not necessary. Programs/shell scripts parsing <bar@plt-0x20> should be extremely brittle and they should be avoided. If the programs need addresses, they should just parse the address part, not the stuff in angle quotes.

I have found someone who agrees with me that firstsym-x is awful and the behavior should not be replicated. Let me check if I can get more opinions tomorrow.

In D63524#1549930, @jhenderson wrote:

Can you persuade git/svn to do a rename of the test rather than a delete and add? That would make the diff easier to see.

Didn't find a way to achieve that unless splitting the file rename part out. Seems it is how Git works (https://stackoverflow.com/questions/7938582/how-does-git-detect-similar-files-for-its-rename-detection),

• update comment

• update

• update

• update

In D63280#1549635, @MaskRay wrote:

riscv-objdump appears to use STT_SECTION symbols to symbolize addresses, e.g.:

Disassembly of section .plt:

000027d0 <.plt>:
    27d0:       00011397                auipc   t2,0x11

x86-64 and some other may not. I need to dig into the details. I also filed a binutils-gdb bug about this https://sourceware.org/bugzilla/show_bug.cgi?id=24702

Thanks. That's great to know! Once it is confirmed from GNU side, I will update the patch to reflect that.

• update

• - perform start symbols insertion before disassembling

• update test

• update test

• update

In D63393#1547972, @jhenderson wrote:

I don't think you need the tr bit. If you have any tabs, the FileCheck will fail, because the whitespace in the lines doesn't contain any tabs.

• update

• update test