[Lldb-commits] [PATCH] D124597: Select the correct DWARF location list entry when looking for variable locations

[Jason Molenda via Phabricator via lldb-commits]

jasonmolenda created this revision.
jasonmolenda added reviewers: jingham, JDevlieghere.
jasonmolenda added a project: LLDB.
Herald added subscribers: pengfei, kristof.beyls.
Herald added a project: All.
jasonmolenda requested review of this revision.
Herald added a subscriber: lldb-commits.

When lldb is looking for a location list entry for a variable, it is using the pc value -- or return-pc value when up the stack -- to pick the correct DWARF location list entry.  Because it is using return-pc, the entry selected may be an entirely different basic block, or function.  We should be using the "pc for symbolication" -- return-pc decremented in most cases -- to select the correct block we should be looking up.  I added RegisterContext::GetPCForSymbolication and StackFrame::GetFrameCodeAddressForSymbolication last year in https://reviews.llvm.org/D97644 and this is a simple patch with those in place.

The bug that drove this is in the middle of a complex project, but I can elicit incorrect behavior with a small C file that calls abort() as its last instruction, with a different function immediately after that.  It must be built at -O1 with clang (on x86_64, AArch64) so that the debug info tracks variables in different registers during the function lifetime, using a DWARF location list.

I constructed a test case that does this, but it depends on clang's current codegen at -O1 on these two architectures.  If the codegen were to shift around, the test could pass without correctly exercising this codepath.  One possibility would be to include a test binary, DWARF for it, and corefile; that would eliminate the possibility of codegen changing.  But it's a lot of data, and I'm not sure how necessary it is.  Given that I get the same codegen on two architectures with clang at -O1, maybe this is a small enough exposure to the optimizer that we can use it in the testsuite.  Open to suggestions.

Without this patch, this test case will show argv as unavailable when you try to print it.  In the actual bug that I was tracking down, the return-pc pointed to a different location list entry for the variable so lldb showed the wrong value for that variable.  A much more subtle and confusing bug for the user.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124597

Files:
  lldb/source/Expression/DWARFExpression.cpp
  lldb/test/API/functionalities/location-list-lookup/Makefile
  lldb/test/API/functionalities/location-list-lookup/TestLocationListLookup.py
  lldb/test/API/functionalities/location-list-lookup/main.c

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D124597.425703.patch
Type: text/x-patch
Size: 4844 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20220428/308d66da/attachment.bin>