-
Notifications
You must be signed in to change notification settings - Fork 14.9k
[lldb] Add a null check, actually use new SharedModuleList class #162574
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
Conversation
Now that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class.
@llvm/pr-subscribers-lldb Author: Augusto Noronha (augusto2112) ChangesNow that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class. Full diff: https://github.com/llvm/llvm-project/pull/162574.diff 1 Files Affected:
diff --git a/lldb/source/Core/ModuleList.cpp b/lldb/source/Core/ModuleList.cpp
index 2ccebf3fabfc5..c40612c1ced5e 100644
--- a/lldb/source/Core/ModuleList.cpp
+++ b/lldb/source/Core/ModuleList.cpp
@@ -989,7 +989,7 @@ class SharedModuleList {
};
struct SharedModuleListInfo {
- ModuleList module_list;
+ SharedModuleList module_list;
ModuleListProperties module_list_properties;
};
}
@@ -1007,7 +1007,7 @@ static SharedModuleListInfo &GetSharedModuleListInfo()
return *g_shared_module_list_info;
}
-static ModuleList &GetSharedModuleList() {
+static SharedModuleList &GetSharedModuleList() {
return GetSharedModuleListInfo().module_list;
}
@@ -1017,8 +1017,8 @@ ModuleListProperties &ModuleList::GetGlobalModuleListProperties() {
bool ModuleList::ModuleIsInCache(const Module *module_ptr) {
if (module_ptr) {
- ModuleList &shared_module_list = GetSharedModuleList();
- return shared_module_list.FindModule(module_ptr).get() != nullptr;
+ SharedModuleList &shared_module_list = GetSharedModuleList();
+ return shared_module_list.FindModule(*module_ptr).get() != nullptr;
}
return false;
}
@@ -1041,9 +1041,8 @@ ModuleList::GetSharedModule(const ModuleSpec &module_spec, ModuleSP &module_sp,
const FileSpecList *module_search_paths_ptr,
llvm::SmallVectorImpl<lldb::ModuleSP> *old_modules,
bool *did_create_ptr, bool always_create) {
- ModuleList &shared_module_list = GetSharedModuleList();
- std::lock_guard<std::recursive_mutex> guard(
- shared_module_list.m_modules_mutex);
+ SharedModuleList &shared_module_list = GetSharedModuleList();
+ std::lock_guard<std::recursive_mutex> guard(shared_module_list.GetMutex());
char path[PATH_MAX];
Status error;
|
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 but the "Add a null check" part doesn't look relevant anymore?
Yes I cherrypicked the original commit and forgot to change that part. I'll change the commit title on merge. |
Now that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class.
Now that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class.
Now that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class.
Now that the use after free bug has been fixed (397181d), actually use the new SharedModuleList class.