Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mstorsjo opened this issue Jul 9, 2022 · 10 comments
Open

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

mstorsjo opened this issue Jul 9, 2022 · 10 comments

Comments

@mstorsjo
Copy link
Member

mstorsjo commented Jul 9, 2022

C++ code on Windows can either be using the MSVC C++ ABI, or the Itanium ABI (in mingw mode). Since 25c8a06 and 3c86789, 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
Copy link
Member

llvmbot commented Jul 9, 2022

@llvm/issue-subscribers-lldb

@ZequanWu
Copy link
Contributor

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, #56449

@mstorsjo
Copy link
Member Author

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
Copy link
Member Author

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
Copy link
Contributor

ZequanWu commented Aug 1, 2022

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
Copy link
Member Author

mstorsjo commented Aug 1, 2022

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
Copy link
Contributor

ZequanWu commented Aug 1, 2022

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

@mstorsjo
Copy link
Member Author

mstorsjo commented Aug 1, 2022

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

Thanks! The fix looks reasonable to me.

@rnk
Copy link
Collaborator

rnk commented Aug 3, 2022

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

@weliveindetail
Copy link
Member

We cannot print the member pointers yet, but the crash should be fixed with:
699ce16

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants