[PATCH] D70720: [llvm-objdump] Display locations of variables alongside disassembly

[Fangrui Song via Phabricator via llvm-commits]

MaskRay added inline comments.


================
Comment at: llvm/include/llvm/Support/FormattedStream.h:108
 
-  /// getColumn - Return the column number
-  unsigned getColumn() { return Position.first; }
+  /// Return the column number.
+  unsigned getColumn() {
----------------
The function name self documents. The comment may be unnecessary.


================
Comment at: llvm/lib/DebugInfo/DWARF/DWARFExpression.cpp:351
+  ExprKind Kind;
+  SmallString<20> String;
+
----------------
There is certainly prior art. But if changing all SmallString<20> to SmallString<16> may decrease the code size?


================
Comment at: llvm/lib/Support/FormattedStream.cpp:29
   // special characters
   for (const char *End = Ptr + Size; Ptr != End; ++Ptr) {
+    // If this is a multi-byte UTF-8 character, skip the extra bytes, and don't
----------------
`Ptr < End` will be safer in -DLLVM_ENABLE_ASSERTIONS=off builds in case of a malformed UTF-8 code sequence.


================
Comment at: llvm/lib/Support/FormattedStream.cpp:33
+    bool MultiByte = true;
+    if ((*Ptr & 0b11100000) == 0b11000000)
+      Ptr += 1;
----------------
The following might be slight simpler.
```lang=cpp
auto C = static_cast<unsigned char>(*Ptr);
if (C < 0x80) MultiByte = false;
else if (C >= 0b11110000) Ptr += 3;
else if (C >= 0b11100000) Ptr += 2;
else Ptr++;
```


================
Comment at: llvm/lib/Support/FormattedStream.cpp:45
+
     ++Column;
+
----------------
This is incorrect due to East Asian Width.... http://www.unicode.org/reports/tr11/tr11-36.html

```lang=cpp
% cat a.cc
int main() {
  int 喵🙂 = 0;
  int 喵喵🙂 = 15;
  int 喵🙂喵🙂 = 24;
  喵🙂 = 28;
  喵喵🙂 = 32;
}
% clang -c a.cc
% llvm-objdump -S --no-show-raw-insn --debug-vars a.o

a.o:    file format elf64-x86-64


Disassembly of section .text:

0000000000000000 main:
; int main() {                                          ┠─ 喵🙂 = <unknown op DW_OP_fbreg (145)>
                                                        ┃ ┠─ 喵喵🙂 = <unknown op DW_OP_fbreg (145)> 
                                                        ┃ ┃ ┠─ 喵🙂喵🙂 = <unknown op DW_OP_fbreg (145)>
       0:       pushq   %rbp                            ┃ ┃ ┃     
       1:       movq    %rsp, %rbp                      ┃ ┃ ┃     
       4:       xorl    %eax, %eax                      ┃ ┃ ┃     
;   int 喵🙂 = 0;                                         ┃ ┃ ┃   
       6:       movl    $0, -4(%rbp)                    ┃ ┃ ┃     
;   int 喵喵🙂 = 15;                                       ┃ ┃ ┃   
       d:       movl    $15, -8(%rbp)                   ┃ ┃ ┃     
;   int 喵🙂喵🙂 = 24;                                      ┃ ┃ ┃   
      14:       movl    $24, -12(%rbp)                  ┃ ┃ ┃     
;   喵🙂 = 28;                                            ┃ ┃ ┃   
      1b:       movl    $28, -4(%rbp)                   ┃ ┃ ┃     
;   喵喵🙂 = 32;                                           ┃ ┃ ┃   
      22:       movl    $32, -8(%rbp)                   ┃ ┃ ┃     
; }                                                     ┃ ┃ ┃   
      29:       popq    %rbp                            ┃ ┃ ┃     
      2a:       retq                                    ┻ ┻ ┻
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70720/new/

https://reviews.llvm.org/D70720