mstorsjo added inline comments.
================ Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1754 + m_ast.getASTContext(), + clang::MSInheritanceAttr::Spelling::Keyword_unspecified_inheritance)); + } ---------------- 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. 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
