Page MenuHomePhabricator

Added GDB pretty printer for StringMap
ClosedPublic

Authored by MoritzS on Nov 10 2020, 9:21 AM.

Diff Detail

Unit TestsFailed

TimeTest
390 mslinux > HWAddressSanitizer-x86_64.TestCases::sizes.cpp
Script: -- : 'RUN: at line 3'; /mnt/disks/ssd0/agent/llvm-project/build/./bin/clang --driver-mode=g++ -m64 -gline-tables-only -fsanitize=hwaddress -fuse-ld=lld -mcmodel=large -mllvm -hwasan-globals -mllvm -hwasan-use-short-granules -mllvm -hwasan-instrument-landing-pads=0 -mllvm -hwasan-instrument-personality-functions /mnt/disks/ssd0/agent/llvm-project/compiler-rt/test/hwasan/TestCases/sizes.cpp -nostdlib++ -lstdc++ -o /mnt/disks/ssd0/agent/llvm-project/build/projects/compiler-rt/test/hwasan/X86_64/TestCases/Output/sizes.cpp.tmp

Event Timeline

MoritzS created this revision.Nov 10 2020, 9:21 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 10 2020, 9:21 AM
MoritzS requested review of this revision.Nov 10 2020, 9:21 AM

Thanks!

Would it be possible to the StringMap to the existing test? (Note that the debug-info tests are not run as part of the pre-merge checks, you need to run them manually).

llvm/utils/gdb-scripts/prettyprinters.py
206–207

Disclaimer: I have no Python experience.

Have you considered using a generator function? I suspect it might avoid the self.first toggle and generally make the implementation a little simpler.

MoritzS updated this revision to Diff 304300.Nov 10 2020, 12:21 PM
  • Added test case
  • Use generator instead of iterator class
MoritzS added inline comments.Nov 10 2020, 12:23 PM
llvm/utils/gdb-scripts/prettyprinters.py
206–207

You are right, I haven't even thought of that. It looks much nicer as a generator function!

csigg accepted this revision.Nov 11 2020, 10:11 AM

Looks great, thanks!

This revision is now accepted and ready to land.Nov 11 2020, 10:11 AM
dblaikie added inline comments.Nov 11 2020, 12:56 PM
llvm/utils/gdb-scripts/prettyprinters.py
215

Any chance we could avoid hardcoding the tombstone value here? I guess maybe if the source were modified to store the tombstone value as a constexpr (not sure if it could be constexpr - with the pointer bitfiddling, etc - but maybe) member of StringMapImpl, would that be accessible from the pretty printer reliably?

MoritzS added inline comments.Nov 12 2020, 1:11 AM
llvm/utils/gdb-scripts/prettyprinters.py
215

I could call StringMapImpl::getTombstoneVal() from gdb to get it, but that depends on that function to not be optimized out. But this kind of function is pretty likely to be inlined and optimized out even with -O1. Even with a constexpr member I think that would have the same problems.

dblaikie added inline comments.Nov 16 2020, 7:38 PM
llvm/utils/gdb-scripts/prettyprinters.py
215

Yeah, relying on a function call would be problematic for the reason you mentioned & for the fact that then you can't debug a core dump (because you can't execute the function) and otherwise risk corrupting the target process by running code when trying to pretty print.

But I don't think Clang will optimize out a constexpr member & the value could be queried without executing code:

$ clang++-tot constexpr.cpp -g -c -O3 && llvm-dwarfdump-tot constexpr.o
constexpr.o:    file format elf64-x86-64

.debug_info contents:
0x00000000: Compile Unit: length = 0x00000052, format = DWARF32, version = 0x0004, abbr_offset = 0x0000, addr_size = 0x08 (next unit at 0x00000056)

0x0000000b: DW_TAG_compile_unit
              DW_AT_producer    ("clang version 12.0.0 (git@github.com:llvm/llvm-project.git 2c196bbc6bd897b3dcc1d87a3baac28e1e88df41)")
              DW_AT_language    (DW_LANG_C_plus_plus_14)
              DW_AT_name        ("constexpr.cpp")
              DW_AT_stmt_list   (0x00000000)
              DW_AT_comp_dir    ("/usr/local/google/home/blaikie/dev/scratch")

0x0000001e:   DW_TAG_variable
                DW_AT_name      ("f")
                DW_AT_type      (0x00000033 "foo")
                DW_AT_external  (true)
                DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/constexpr.cpp")
                DW_AT_decl_line (4)
                DW_AT_location  (DW_OP_addr 0x0)

0x00000033:   DW_TAG_structure_type
                DW_AT_calling_convention        (DW_CC_pass_by_value)
                DW_AT_name      ("foo")
                DW_AT_byte_size (0x01)
                DW_AT_decl_file ("/usr/local/google/home/blaikie/dev/scratch/constexpr.cpp")
                DW_AT_decl_line (1)

0x0000003c:     DW_TAG_member
                  DW_AT_name    ("i")
                  DW_AT_type    (0x00000049 "const int")
                  DW_AT_decl_file       ("/usr/local/google/home/blaikie/dev/scratch/constexpr.cpp")
                  DW_AT_decl_line       (2)
                  DW_AT_external        (true)
                  DW_AT_declaration     (true)
                  DW_AT_const_value     (7)

0x00000048:     NULL

0x00000049:   DW_TAG_const_type
                DW_AT_type      (0x0000004e "int")

0x0000004e:   DW_TAG_base_type
                DW_AT_name      ("int")
                DW_AT_encoding  (DW_ATE_signed)
                DW_AT_byte_size (0x04)

0x00000055:   NULL
blaikie@blaikie-linux2:~/dev/scratch$ cat constexpr.cpp
struct foo {
  static constexpr int i = 7;
};
foo f;
MoritzS updated this revision to Diff 305860.Nov 17 2020, 11:21 AM

Read tombstone value from a constexpr variable. The getTombstoneVal() function
is still necessary, though, as reinterpret_cast is not a constant expression.

MoritzS added inline comments.Nov 17 2020, 11:22 AM
llvm/utils/gdb-scripts/prettyprinters.py
215

You are right, it also works with gcc! I changed it.

dblaikie accepted this revision.Nov 17 2020, 2:31 PM
dblaikie added inline comments.
llvm/utils/gdb-scripts/prettyprinters.py
215

Awesome, thanks for that!

Can you commit this please? I don't have commit access.

This revision was landed with ongoing or failed builds.Nov 18 2020, 4:33 PM
This revision was automatically updated to reflect the committed changes.