-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[lldb][Language] Pass SymbolNameFitsToLanguage parameter by const-ref #167684
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
[lldb][Language] Pass SymbolNameFitsToLanguage parameter by const-ref #167684
Conversation
We've been seeing (rare) crashes from both `CPlusPlusLanguage::SymbolNameFitsToLanguage` and `ObjCLanguage::SymbolNameFitsToLanguage` when we try to read contents of the `ConstString`s of the `Mangled` parameter. I'm not entirely sure how that can happen (current theory is corrupted stack somehow which overwrites `ConstString::m_string` to an invalid pointer) but I'm not able to confirm that. One thing these crashes had in common is that they operate on the `Mangled` object we copied into `SymbolNameFitsToLanguage` by value. While I can't see off the top why that would cause it to contain unintiailized/corrupt `ConstString`s, the class is sufficiently large enough to probably pass it by `const &` anyway. This is what this patch does. rdar://164519648
|
@llvm/pr-subscribers-lldb Author: Michael Buch (Michael137) ChangesWe've been seeing (rare) crashes from both One thing these crashes had in common is that they operate on the rdar://164519648 Full diff: https://github.com/llvm/llvm-project/pull/167684.diff 5 Files Affected:
diff --git a/lldb/include/lldb/Target/Language.h b/lldb/include/lldb/Target/Language.h
index 9958b6ea2f815..9292f790333a1 100644
--- a/lldb/include/lldb/Target/Language.h
+++ b/lldb/include/lldb/Target/Language.h
@@ -318,7 +318,9 @@ class Language : public PluginInterface {
///
/// This function should only return true if there is a high confidence
/// that the name actually belongs to this language.
- virtual bool SymbolNameFitsToLanguage(Mangled name) const { return false; }
+ virtual bool SymbolNameFitsToLanguage(const Mangled &name) const {
+ return false;
+ }
/// An individual data formatter may apply to several types and cross language
/// boundaries. Each of those languages may want to customize the display of
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
index a2199cb65cd35..e935ea8fab813 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.cpp
@@ -103,7 +103,7 @@ CPlusPlusLanguage::GetFunctionNameInfo(ConstString name) const {
return {func_name_type, ConstString(basename)};
}
-bool CPlusPlusLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
+bool CPlusPlusLanguage::SymbolNameFitsToLanguage(const Mangled &mangled) const {
auto mangling_scheme =
Mangled::GetManglingScheme(mangled.GetMangledName().GetStringRef());
return mangling_scheme == Mangled::eManglingSchemeItanium ||
diff --git a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
index 9a528ca7b03f9..13d436a68c691 100644
--- a/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
+++ b/lldb/source/Plugins/Language/CPlusPlus/CPlusPlusLanguage.h
@@ -92,7 +92,7 @@ class CPlusPlusLanguage : public Language {
static llvm::StringRef GetPluginNameStatic() { return "cplusplus"; }
- bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+ bool SymbolNameFitsToLanguage(const Mangled &mangled) const override;
bool DemangledNameContainsPath(llvm::StringRef path,
ConstString demangled) const override;
diff --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
index 3b8e21cbb9269..5e31faccb315c 100644
--- a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
+++ b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.cpp
@@ -235,7 +235,7 @@ ObjCLanguage::GetFunctionNameInfo(ConstString name) const {
return {func_name_type, std::nullopt};
}
-bool ObjCLanguage::SymbolNameFitsToLanguage(Mangled mangled) const {
+bool ObjCLanguage::SymbolNameFitsToLanguage(const Mangled &mangled) const {
ConstString demangled_name = mangled.GetDemangledName();
if (!demangled_name)
return false;
diff --git a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
index a68ea41c723de..729230dc2dccb 100644
--- a/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
+++ b/lldb/source/Plugins/Language/ObjC/ObjCLanguage.h
@@ -145,7 +145,7 @@ class ObjCLanguage : public Language {
std::pair<lldb::FunctionNameType, std::optional<ConstString>>
GetFunctionNameInfo(ConstString name) const override;
- bool SymbolNameFitsToLanguage(Mangled mangled) const override;
+ bool SymbolNameFitsToLanguage(const Mangled &mangled) const override;
lldb::TypeCategoryImplSP GetFormatters() override;
|
felipepiovezan
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…llvm#167684) We've been seeing (rare) crashes from both `CPlusPlusLanguage::SymbolNameFitsToLanguage` and `ObjCLanguage::SymbolNameFitsToLanguage` when we try to read contents of the `ConstString`s of the `Mangled` parameter. I'm not entirely sure how that can happen (current theory is corrupted stack somehow which overwrites `ConstString::m_string` to an invalid pointer) but I'm not able to confirm that. One thing these crashes had in common is that they operate on the `Mangled` object we copied into `SymbolNameFitsToLanguage` by value. While I can't see off the top why that would cause it to contain unintiailized/corrupt `ConstString`s, the class is sufficiently large enough to probably pass it by `const &` anyway. This is what this patch does. rdar://164519648 (cherry picked from commit 0f4dc93)
The API was changed in llvm#167684 to take `Mangled` by const-ref.
We've been seeing (rare) crashes from both
CPlusPlusLanguage::SymbolNameFitsToLanguageandObjCLanguage::SymbolNameFitsToLanguagewhen we try to read contents of theConstStrings of theMangledparameter. I'm not entirely sure how that can happen (current theory is corrupted stack somehow which overwritesConstString::m_stringto an invalid pointer) but I'm not able to confirm that.One thing these crashes had in common is that they operate on the
Mangledobject we copied intoSymbolNameFitsToLanguageby value. While I can't see off the top why that would cause it to contain unintiailized/corruptConstStrings, the class is sufficiently large enough to probably pass it byconst &anyway. This is what this patch does.rdar://164519648