[Lldb-commits] [PATCH] D43059: Add implementation for MSVC in CPlusPlusLanguage::IsCPPMangledName

[Jim Ingham via Phabricator via lldb-commits]

jingham added a comment.

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...


Repository:
  rL LLVM

https://reviews.llvm.org/D43059