Add comments to LLDB-MI disassembly.

[ki.stfu]

Summary

This patch adds the comments which are included in LLDB disassembly, but are currently missing in MI, e.g.

^done,asm_insns=[...,{address="0x00000000004004ff",func-name="main",offset="18",size="2",inst="jmp 0x40050b ; <+30> at main.c:7"}]

instead of

^done,asm_insns=[...,{address="0x00000000004004ff",func-name="main",offset="18",size="2",inst="jmp 0x40050b"}]

Thanks, Ewan

Patch from [e-mail]

[ki.stfu]

Looks like you want to implement the -data-disassemble command for mode=1 (see https://sourceware.org/gdb/onlinedocs/gdb/GDB_002fMI-Data-Manipulation.html).

I against to add a comment into the "inst" field. I think you should use the mode=1 (or 3) option to get the "line" and "file" fields. It will look like the following:

(gdb)
-data-disassemble -f basics.c -l 32 -n 3 -- 1
^done,asm_insns=[
src_and_asm_line={line="31",
file="../../../src/gdb/testsuite/gdb.mi/basics.c",
fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
line_asm_insn=[{address="0x000107bc",
func-name="main",offset="0",inst="save  %sp, -112, %sp"}]},
src_and_asm_line={line="32",
file="../../../src/gdb/testsuite/gdb.mi/basics.c",
fullname="/absolute/path/to/src/gdb/testsuite/gdb.mi/basics.c",
line_asm_insn=[{address="0x000107c0",
func-name="main",offset="4",inst="mov  2, %o0"},
{address="0x000107c4",func-name="main",offset="8",
inst="sethi  %hi(0x11800), %o2"}]}]
(gdb)

[ted]

The comment isn't there to get line and file; it's there to get symbolic branch targets.

Disassembling with the patch looks like this:

-data-disassemble --thread 1 -s 0x5164 -e 0x5165 -- 1
^done,asm_insns=[src_and_asm_line={line="34",file="factorial.c",line_asm_insn=[{address="0x00005164",func-name="main",offset="0",size="4",inst="{ call 0x52a0 }        ; printf"}]}]

Without the patch it looks like this:

-data-disassemble --thread 1 -s 0x5164 -e 0x5165 -- 1
^done,asm_insns=[src_and_asm_line={line="34",file="factorial.c",line_asm_insn=[{address="0x00005164",func-name="main",offset="0",size="4",inst="{ call 0x52a0 }"}]}]

[ki.stfu]

In D8964#154945, @ted wrote:

The comment isn't there to get line and file; it's there to get symbolic branch targets.

If so then create a separate field named "comment" and use it for comments. Also, update the MIExtensions.txt please.

[ted]

In D8964#154965, @ki.stfu wrote:

In D8964#154945, @ted wrote:

The comment isn't there to get line and file; it's there to get symbolic branch targets.

If so then create a separate field named "comment" and use it for comments. Also, update the MIExtensions.txt please.

It's not really an MI extension - disassembly of change-of-flow instructions isn't listed in the gdb-mi docs, but gdb prints the symbol out following the jump target. It looks like this:

{address="0x1d04c55c",func-name="wait_for_int",offset="48",inst="5cffe0ec     if !p0 jump 0x1d04c534 <wait_for_int+8>"}

[ki.stfu]

In D8964#154998, @ted wrote:

It's not really an MI extension - disassembly of change-of-flow instructions isn't listed in the gdb-mi docs, but gdb prints the symbol out following the jump target. It looks like this:

{address="0x1d04c55c",func-name="wait_for_int",offset="48",inst="5cffe0ec     if !p0 jump 0x1d04c534 <wait_for_int+8>"}

On OS X the -data-disassemble commands shows the following:

{address="0x0000000100000f0e",func-name="main",offset="225",size="2",inst="movb $0x0, %al"},{address="0x0000000100000f10",func-name="main",offset="227",size="5",inst="callq 0x100000f36"},{address="0x0000000100000f15",func-name="main",offset="232",size="3",inst="movl %eax, -0x54(%rbp)"}

With your patch the output is:

{address="0x0000000100000ee0",func-name="main",offset="179",size="5",inst="callq 0x100000f36		; symbol stub for: printf"}

But these spaces annoy me. I really don't want to use the '\t' char in lldb-mi output. May be we can use the following format?

416      strComment = CMIUtilString::Format("<%s>", pStrComment);

or

416      strComment = CMIUtilString::Format("; %s", pStrComment);

And the output will be the following:

{address="0x0000000100000f0e",func-name="main",offset="225",size="2",inst="movb $0x0, %al"},{address="0x0000000100000f10",func-name="main",offset="227",size="5",inst="callq 0x100000f36; symbol stub for: printf"},{address="0x0000000100000f15",func-name="main",offset="232",size="3",inst="movl %eax, -0x54(%rbp)"}

tools/lldb-mi/MICmdCmdData.cpp

416 ↗

(On Diff #23582)

here

[ted]

The spaces bother me too. Ewan, please remove them.

[EwanCrawford]

Thanks for the feedback, I've removed the spaces as requested.
Sorry for the delayed update.

[ki.stfu]

Should I commit it for you?

[EwanCrawford]

That would be great thanks.

[ki.stfu]

Rebase against ToT

lldb/trunk/tools/lldb-mi/MICmdCmdData.cpp

if(nInstructions > 0)
         start_offset = instructions.GetInstructionAtIndex(0).GetAddress().GetOffset();
 
     for (size_t i = 0; i < nInstructions; i++)
     {
         const MIchar *pUnknown = "??";
         lldb::SBInstruction instrt = instructions.GetInstructionAtIndex(i);
         const MIchar *pStrMnemonic = instrt.GetMnemonic(sbTarget);
         pStrMnemonic = (pStrMnemonic != nullptr) ? pStrMnemonic : pUnknown;
+        const MIchar *pStrComment = instrt.GetComment(sbTarget);
+        CMIUtilString strComment;
+        if (pStrComment != nullptr && *pStrComment != '\0')
+            strComment = CMIUtilString::Format("; %s", pStrComment);
         lldb::SBAddress address = instrt.GetAddress();
         lldb::addr_t addr = address.GetLoadAddress(sbTarget);
         const MIchar *pFnName = address.GetFunction().GetName();
         pFnName = (pFnName != nullptr) ? pFnName : pUnknown;
         lldb::addr_t addrOffSet = address.GetOffset() - start_offset;
         const MIchar *pStrOperands = instrt.GetOperands(sbTarget);
         pStrOperands = (pStrOperands != nullptr) ? pStrOperands : pUnknown;
         const size_t instrtSize = instrt.GetByteSize();
 
         // MI "{address=\"0x%016" PRIx64 "\",func-name=\"%s\",offset=\"%lld\",inst=\"%s %s\"}"
         const CMICmnMIValueConst miValueConst(CMIUtilString::Format("0x%016" PRIx64, addr));
         const CMICmnMIValueResult miValueResult("address", miValueConst);
         CMICmnMIValueTuple miValueTuple(miValueResult);
         const CMICmnMIValueConst miValueConst2(pFnName);
         const CMICmnMIValueResult miValueResult2("func-name", miValueConst2);
         miValueTuple.Add(miValueResult2);
         const CMICmnMIValueConst miValueConst3(CMIUtilString::Format("%lld", addrOffSet));
         const CMICmnMIValueResult miValueResult3("offset", miValueConst3);
         miValueTuple.Add(miValueResult3);
         const CMICmnMIValueConst miValueConst4(CMIUtilString::Format("%d", instrtSize));
         const CMICmnMIValueResult miValueResult4("size", miValueConst4);
         miValueTuple.Add(miValueResult4);
-        const CMICmnMIValueConst miValueConst5(CMIUtilString::Format("%s %s", pStrMnemonic, pStrOperands));
+        const CMICmnMIValueConst miValueConst5(CMIUtilString::Format("%s %s%s", pStrMnemonic, pStrOperands, strComment.c_str()));
         const CMICmnMIValueResult miValueResult5("inst", miValueConst5);
         miValueTuple.Add(miValueResult5);
 
         if (nDisasmMode == 1)
         {
             lldb::SBLineEntry lineEntry = address.GetLineEntry();
             const MIuint nLine = lineEntry.GetLine();
             const MIchar *pFileName = lineEntry.GetFileSpec().GetFilename();