Recognize MSVC style mangling in CPlusPlusLanguage::IsCPPMangledName

[davide]

Can you add a unittest for this? :)

[labath]

That seems extremely dodgy. Surely whether something is a mangled name does not depend on the compiler we built lldb with. I am aware that we have many places with _Z hardcoded, but this seems to be making the situation only worse.

I'm not sure how is this API being used, but maybe this should just recognize both mangling schemes unconditionally ?

[asmith]

IsCPPMangledName() is a public method to determine if a symbol has a mangled name.

This change is needed so that lldb can find symbols with MSVC style mangled names.

Open to suggestions on a better way to deal with this...

Here are all the places it is used:

source/Core/Module.cpp: if (CPlusPlusLanguage::IsCPPMangledName(name_cstr))
source/Core/Module.cpp: !CPlusPlusLanguage::IsCPPMangledName(name_cstr)) {
source/Core/Mangled.cpp: if (CPlusPlusLanguage::IsCPPMangledName(mangled_name))
source/Plugins/ExpressionParser/Clang/ClangExpressionDeclMap.cpp: !CPlusPlusLanguage::IsCPPMangledName(name)) ||
source/Expression/IRExecutionUnit.cpp: if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {
source/Expression/IRExecutionUnit.cpp: if (CPlusPlusLanguage::IsCPPMangledName(name.GetCString())) {

[asmith]

How about this?

if (name == nullptr)
  return false;
 
return ( (name[0] == '?') || (name[0] == '_' && name[1] == 'Z') );

[jingham]

We'd get into problems on a platform other than MSVC that uses ^? for symbol names that aren't C++ mangled ones - or equivalently if MSVC uses ^_Z for non-C++ mangled names.

It looks like everywhere we use this (or use GuessLanguage) we either turn around and mangle it, and we handle the case where we weren't actually right about that. If we had another language with a different mangling (swift for example) and on some platform that language uses ? for its initial prefix, then we might get into trouble.

We don't everywhere do IsCPPMangledName -> Try to demangle -> Try next possible language. So we would miss demangling some names.

The "correct" way to do this is to change all the places that use this API actually get the current C++ runtime and ask that if they recognize the name. But right now that's hard because we infer the C++ runtime from the Process (after all you could have both the FSF & LLVM libstdc++'s on the same platform, and you might not know which you will get when you run. Even worse, it used to be (on macOS in the early days of lldb) that you could have two different C++ standard libraries with different manglings. So you can't make a great guess about what mangling is going to be used from just a Target. If we suspected this confusion were a likely problem we'd have to make a TargetLanguageRuntime that you would fall back to if you didn't have a Process, and have that implement IsCPPMangledName. That wouldn't help with two manglings on one platform, but it would keep the platforms properly segregated.

OTOH, this seems like a fairly remote possibility, so I'm okay with leaving this as a possible danger, though it might be good to make the comment clearer about that possibility, since the danger it warns about has clearly arisen...

[zturner]

Looks good. Wish this function took a StringRef so you could just say return name.startswith("?") || name.startswith("_Z")

source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp

 }
 
 bool CPlusPlusLanguage::IsCPPMangledName(const char *name) {
   // FIXME, we should really run through all the known C++ Language plugins and
   // ask each one if
   // this is a C++ mangled name, but we can put that off till there is actually
   // more than one
   // we care about.
+#if defined(_MSC_VER)
+  return (name != nullptr && name[0] == '?');
+#endif
 
   return (name != nullptr && name[0] == '_' && name[1] == 'Z');
 }
 
 bool CPlusPlusLanguage::ExtractContextAndIdentifier(
     const char *name, llvm::StringRef &context, llvm::StringRef &identifier) {
   CPlusPlusNameParser parser(name);
   if (auto full_name = parser.ParseAsFullName()) {