rnk added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1754 + m_ast.getASTContext(), + clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance)); + } ---------------- mstorsjo wrote: > rnk wrote: > > I'm concerned that this isn't really the right fix. Changing the > > inheritance model changes the size of the member pointer representation, so > > the consequences of getting the wrong one could affect expression > > evaluation results. Zequan suggested guessing the inheritance model from > > the class definition, but we really ought to write down the inheritance > > model in the DWARF when using the MS ABI. This probably needs its own DWARF > > attribute. > FWIW, there’s two use cases at play here: > > 1. The executable actually is using the itanium ABI, but is being > (mis)interpreted with the MSVC ABI. When this happens, we currently crash, > which is a terrible user experience. In this case, there’s probably no good > solution (we can’t really get it right at this point) - but pretty much > anything is better than crashing. Before D127048, we always interpreted > everything with the MSVC C++ ABI; now we probably are getting it right in a > majority of cases at least. > > 2. People intentionally using dwarf debug into with MSVC ABI. Not very > common, but something that some people do use, as far as I know. Here we at > least have a chance of getting right. I see, I thought we already knew we were in use case 2 not 1. Having this as a workaround to not crash seems fine, but we really ought to warn when LLDB encounters these conditions: 1. MS C++ ABI is in use 2. DWARF is in use 3. A C++ source file is in use Using DWARF for C code in the MS ABI model is fine, no need to warn in that case. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D130942/new/ https://reviews.llvm.org/D130942 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
