llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.26k stars 12.09k forks source link

LLDB crashes on incorrect MSVC C++ ABI debug info #56458

Open mstorsjo opened 2 years ago

mstorsjo commented 2 years ago

C++ code on Windows can either be using the MSVC C++ ABI, or the Itanium ABI (in mingw mode). Since 25c8a061c5739677d2fc0af29a8cc9520207b923 and 3c867898c7be7ed2b5d119a2478a836a0c85f19b, it's possible to set the desired ABI mode when using LLDB. But for cases when running in the wrong mode, LLDB can flat out crash (triggers a failed assert) for some C++ code.

This is reproducible with the attached testcase cxxabi.zip.

To reproduce (which can be done on any OS, doesn't need to actually run on Windows):

$ lldb cxxabi.exe
(lldb) target create "cxxabi.exe"
Current executable set to '/home/martin/test/cxxabi.exe' (x86_64).
(lldb) l callFunc
Assertion failed: (IA && "Expected MSInheritanceAttr on the CXXRecordDecl!"), function getMSInheritanceModel, file /home/martin/code/llvm-project/clang/lib/AST/MicrosoftCXXABI.cpp, line 236. 

(Here, the issue is avoidable e.g. by starting lldb with lldb -O "settings set plugin.object-file.pe-coff.abi gnu" cxxabi.exe.)

CC @ZequanWu @alvinhochun @rnk @zmodem

I'm not sure exactly at what point this should be fixed - I guess it's somewhat acceptable for Clang, running in MSVC mode, to use such assertions - but when the input isn't from Clang itself but from debug info read by LLDB, some more error checks at some point seems to be needed.

llvmbot commented 2 years ago

@llvm/issue-subscribers-lldb

ZequanWu commented 2 years ago

I've filed a bug that crashed lldb at the same place. I don't know what's wrong yet, but it might be related, https://github.com/llvm/llvm-project/issues/56449

mstorsjo commented 2 years ago

FWIW the binary was built from source that looks like this:

class MyClass;
typedef void (MyClass::* MyClassFuncPtr)(int);

void callFunc(MyClass *obj, MyClassFuncPtr ptr) {
  (obj->*ptr)(42);
}       

int main(int argc, char* argv[]) {
  return 0;
}       

with an llvm-mingw toolchain, with a command like this: x86_64-w64-mingw32-g++ cxxabi.cpp -o cxxabi.exe -ggdb.

mstorsjo commented 2 years ago

Ping @ZequanWu - I presume the MSVC ABI case was fixed in #56449 - but this one still fails? (It fails because it's a mingw ABI binary, while the fix for #56449 was to interpret some fields from PDB files to fill in more information for the MSVC ABI.) If we interpret an executable in MSVC ABI mode but miss those necessary fields, is there any way to avoid crashing and dealing with the situation better?

ZequanWu commented 2 years ago

Oh, I see that's the different problem. #56449 fixes the problem in NativePDB plugin which process debug info in PDB format. The crash happens here is another problem caused by DWARF plugin which process dwarf format debug info. We should probably just add a clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance attribute for the parsed class types in dwarf plugin.

mstorsjo commented 2 years ago

We should probably just add a clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance attribute for the parsed class types in dwarf plugin.

I guess that makes sense - assuming that it doesn't break anything when you actually use the Itanium C++ ABI too. To test that, you'd run it with lldb -O "settings set plugin.object-file.pe-coff.abi gnu".

ZequanWu commented 2 years ago

Sent https://reviews.llvm.org/D130942 to fix it.

mstorsjo commented 2 years ago

Sent https://reviews.llvm.org/D130942 to fix it.

Thanks! The fix looks reasonable to me.

rnk commented 2 years ago

I think we may need to revisit this fix, I put some comments on the review.

weliveindetail commented 1 month ago

We cannot print the member pointers yet, but the crash should be fixed with: https://github.com/llvm/llvm-project/commit/699ce16b6284377e0cd9969b9f95e7367632a622