This is an archive of the discontinued LLVM Phabricator instance.

[LLDB][NativePDB] Fix a crash when S_DEFRANGE_SUBFIELD_REGISTER descirbes a simple type
ClosedPublic

Authored by zequanwu on Apr 1 2022, 3:45 PM.

Details

Summary

When a variable is simple type and has 64 bits, the debug info may look like the following when targeting 32bit windows. The variable's content is split into two 32bits registers.

480 | S_LOCAL [size = 12] `x`
      type=0x0013 (__int64), flags = param
492 | S_DEFRANGE_SUBFIELD_REGISTER [size = 20]
      register = EAX, may have no name = true, offset in parent = 0
      range = [0001:0073,+7), gaps = []
512 | S_DEFRANGE_SUBFIELD_REGISTER [size = 20]
      register = ECX, may have no name = true, offset in parent = 4
      range = [0001:0073,+7), gaps = []

Diff Detail

Event Timeline

zequanwu created this revision.Apr 1 2022, 3:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 3:45 PM
zequanwu requested review of this revision.Apr 1 2022, 3:45 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 1 2022, 3:45 PM
labath added a comment.Apr 4 2022, 6:14 AM

Seems reasonable, but I don't know much about pdb's. @rnk, do you want to take a look?

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
53–54

I think SmallVectorImpl is still the official way to take SmallVector references.

aganea added a comment.Apr 4 2022, 7:50 PM

There's a typo in the title, s/descirbes/describes/

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
53–54

ArrayRef would be even better! :) ie. llvm::ArrayRef<std::pair<RegisterId, uint32_t>> members_info;

lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s
28

nitpick: Is anything after (and including) ", valid ranges" necessary to cover this change?

zequanwu updated this revision to Diff 421281.Apr 7 2022, 10:43 AM
zequanwu marked 2 inline comments as done.

Update.

lldb/source/Plugins/SymbolFile/NativePDB/PdbUtil.cpp
53–54

Using SmallVectorImpl because we want to insert elements in visitKnownMember.

lldb/test/Shell/SymbolFile/NativePDB/subfield_register_simple_type.s
28

Yes, the structure of location is basically what we are testing against though the register number maybe incorrect due to the FIXME above.

labath accepted this revision.Apr 14 2022, 10:40 AM
This revision is now accepted and ready to land.Apr 14 2022, 10:40 AM

Hi @zequanwu, your patch's new test seems to be failing on CI (https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/). Could you fix it or revert the changes? Thanks!

Hi @zequanwu, your patch's new test seems to be failing on CI (https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/). Could you fix it or revert the changes? Thanks!

Thanks, https://github.com/llvm/llvm-project/commit/2f78f9455f85e61fcfeb1dc3a13d62c912aa0c96 should fix it.