[lldb][NFC] Small fixes identified by the clang static analyzer #148773
+36
−42
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
I ran the clang static analyzer over the codebase on macOS, it identified some minor issues but nothing really good. Mostly dead variable assignments and a couple of places where we can leak an object if we early-return, etc.
|
@llvm/pr-subscribers-lldb @llvm/pr-subscribers-backend-hexagon Author: Jason Molenda (jasonmolenda) ChangesI ran the clang static analyzer over the codebase on macOS, it identified some minor issues but nothing really good. Mostly dead variable assignments and a couple of places where we can leak an object if we early-return, etc. Full diff: https://github.com/llvm/llvm-project/pull/148773.diff 17 Files Affected:
diff --git a/lldb/source/Expression/DWARFExpression.cpp b/lldb/source/Expression/DWARFExpression.cpp
index 52891fcefd68b..7aac546311c30 100644
--- a/lldb/source/Expression/DWARFExpression.cpp
+++ b/lldb/source/Expression/DWARFExpression.cpp
@@ -2015,8 +2015,6 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
if (stack.size() < 1) {
UpdateValueTypeFromLocationDescription(log, dwarf_cu,
LocationDescriptionKind::Empty);
- // Reset for the next piece.
- dwarf4_location_description_kind = Memory;
return llvm::createStringError(
"expression stack needs at least 1 item for DW_OP_bit_piece");
} else {
@@ -2077,7 +2075,6 @@ llvm::Expected<Value> DWARFExpression::Evaluate(
}
case DW_OP_implicit_pointer: {
- dwarf4_location_description_kind = Implicit;
return llvm::createStringError("Could not evaluate %s.",
DW_OP_value_to_name(op));
}
diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm
index 3c1d1179963d2..ec93b9b33e982 100644
--- a/lldb/source/Host/macosx/objcxx/Host.mm
+++ b/lldb/source/Host/macosx/objcxx/Host.mm
@@ -315,8 +315,10 @@ repeat with the_window in (get windows)\n\
llvm::Expected<HostThread> accept_thread = ThreadLauncher::LaunchThread(
unix_socket_name, [&] { return AcceptPIDFromInferior(connect_url); });
- if (!accept_thread)
+ if (!accept_thread) {
+ [applescript release];
return Status::FromError(accept_thread.takeError());
+ }
[applescript executeAndReturnError:nil];
@@ -1058,6 +1060,8 @@ static Status LaunchProcessXPC(const char *exe_path,
LLDB_LOG(log, "error: {0}", error);
}
+ xpc_release(reply);
+ xpc_release(message);
return error;
#else
Status error;
diff --git a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
index 62c0fb0ff4eb8..fe4774bb72438 100644
--- a/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
+++ b/lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
@@ -90,7 +90,7 @@ bool HexagonDYLDRendezvous::Resolve() {
if (!(cursor = ReadWord(cursor, &info.state, word_size)))
return false;
- if (!(cursor = ReadPointer(cursor + padding, &info.ldbase)))
+ if (!ReadPointer(cursor + padding, &info.ldbase))
return false;
// The rendezvous was successfully read. Update our internal state.
@@ -276,7 +276,7 @@ bool HexagonDYLDRendezvous::ReadSOEntryFromMemory(lldb::addr_t addr,
if (!(addr = ReadPointer(addr, &entry.next)))
return false;
- if (!(addr = ReadPointer(addr, &entry.prev)))
+ if (!ReadPointer(addr, &entry.prev))
return false;
entry.path = ReadStringFromMemory(entry.path_addr);
diff --git a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
index cb8ba05d461d4..114c32f959403 100644
--- a/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
+++ b/lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
@@ -112,7 +112,6 @@ size_t ObjectFileJSON::GetModuleSpecifications(
data_sp = MapFileData(file, length, file_offset);
if (!data_sp)
return 0;
- data_offset = 0;
}
Log *log = GetLog(LLDBLog::Symbols);
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 1db7bc78013d7..aa59838401b0c 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -874,7 +874,6 @@ PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
new_annotations_sp = ExtractCrashInfoAnnotations(process);
if (new_annotations_sp && new_annotations_sp->GetSize()) {
process_dict_sp->AddItem(crash_info_key, new_annotations_sp);
- annotations = new_annotations_sp.get();
}
}
@@ -883,10 +882,8 @@ PlatformDarwin::FetchExtendedCrashInformation(Process &process) {
if (!process_dict_sp->GetValueForKeyAsDictionary(asi_info_key,
app_specific_info)) {
new_app_specific_info_sp = ExtractAppSpecificInfo(process);
- if (new_app_specific_info_sp && new_app_specific_info_sp->GetSize()) {
+ if (new_app_specific_info_sp && new_app_specific_info_sp->GetSize())
process_dict_sp->AddItem(asi_info_key, new_app_specific_info_sp);
- app_specific_info = new_app_specific_info_sp.get();
- }
}
// Now get anything else that was in the process info dict, and add it to the
diff --git a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
index f3e79d3d56154..05be34f0928de 100644
--- a/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
+++ b/lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
@@ -477,6 +477,7 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
stringWithUTF8String:args.GetArgumentAtIndex(idx)]];
[options setObject:args_array forKey:kSimDeviceSpawnArguments];
+ [args_array release];
}
NSMutableDictionary *env_dict = [[NSMutableDictionary alloc] init];
@@ -539,6 +540,8 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
: "unable to launch");
}
+ [env_dict release];
+ [options release];
return CoreSimulatorSupport::Process(pid, std::move(error));
}
diff --git a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
index befc28b09d185..c0f452bf67554 100644
--- a/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
+++ b/lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
@@ -657,8 +657,8 @@ PlatformPOSIX::MakeLoadImageUtilityFunction(ExecutionContext &exe_ctx,
arguments.PushValue(value);
arguments.PushValue(value);
arguments.PushValue(value);
-
- do_dlopen_function = dlopen_utility_func_up->MakeFunctionCaller(
+
+ dlopen_utility_func_up->MakeFunctionCaller(
clang_void_pointer_type, arguments, exe_ctx.GetThreadSP(), utility_error);
if (utility_error.Fail()) {
error = Status::FromErrorStringWithFormat(
diff --git a/lldb/source/Plugins/Process/Utility/ARMUtils.h b/lldb/source/Plugins/Process/Utility/ARMUtils.h
index 9256f926275b8..8bcb5e8376550 100644
--- a/lldb/source/Plugins/Process/Utility/ARMUtils.h
+++ b/lldb/source/Plugins/Process/Utility/ARMUtils.h
@@ -95,7 +95,7 @@ static inline uint32_t LSL_C(const uint32_t value, const uint32_t amount,
return 0;
}
*success = true;
- carry_out = amount <= 32 ? Bit32(value, 32 - amount) : 0;
+ carry_out = amount < 32 ? Bit32(value, 32 - amount) : 0;
return value << amount;
}
@@ -119,7 +119,7 @@ static inline uint32_t LSR_C(const uint32_t value, const uint32_t amount,
return 0;
}
*success = true;
- carry_out = amount <= 32 ? Bit32(value, amount - 1) : 0;
+ carry_out = amount < 32 ? Bit32(value, amount - 1) : 0;
return value >> amount;
}
diff --git a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp
index 3038cbcb72371..27b730f71d8d3 100644
--- a/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp
+++ b/lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp
@@ -94,8 +94,10 @@ const RegisterInfo *NativeRegisterContextDBReg_x86::GetDR(int num) const {
Status NativeRegisterContextDBReg_x86::IsWatchpointHit(uint32_t wp_index,
bool &is_hit) {
- if (wp_index >= NumSupportedHardwareWatchpoints())
+ if (wp_index >= NumSupportedHardwareWatchpoints()) {
+ is_hit = false;
return Status::FromErrorString("Watchpoint index out of range");
+ }
RegisterValue dr6;
Status error = ReadRegister(GetDR(6), dr6);
@@ -127,8 +129,10 @@ NativeRegisterContextDBReg_x86::GetWatchpointHitIndex(uint32_t &wp_index,
Status NativeRegisterContextDBReg_x86::IsWatchpointVacant(uint32_t wp_index,
bool &is_vacant) {
- if (wp_index >= NumSupportedHardwareWatchpoints())
+ if (wp_index >= NumSupportedHardwareWatchpoints()) {
+ is_vacant = false;
return Status::FromErrorString("Watchpoint index out of range");
+ }
RegisterValue dr7;
Status error = ReadRegister(GetDR(7), dr7);
diff --git a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
index 7015c3c65cc7d..1e78775e25787 100644
--- a/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
+++ b/lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
@@ -192,18 +192,22 @@ ThreadElfCore::CreateRegisterContextForFrame(StackFrame *frame) {
m_thread_reg_ctx_sp = std::make_shared<RegisterContextCorePOSIX_arm>(
*this, std::make_unique<RegisterInfoPOSIX_arm>(arch), m_gpregset_data,
m_notes);
+ delete reg_interface;
break;
case llvm::Triple::loongarch64:
m_thread_reg_ctx_sp = RegisterContextCorePOSIX_loongarch64::Create(
*this, arch, m_gpregset_data, m_notes);
+ delete reg_interface;
break;
case llvm::Triple::riscv32:
m_thread_reg_ctx_sp = RegisterContextCorePOSIX_riscv32::Create(
*this, arch, m_gpregset_data, m_notes);
+ delete reg_interface;
break;
case llvm::Triple::riscv64:
m_thread_reg_ctx_sp = RegisterContextCorePOSIX_riscv64::Create(
*this, arch, m_gpregset_data, m_notes);
+ delete reg_interface;
break;
case llvm::Triple::mipsel:
case llvm::Triple::mips:
diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
index a2c34ddfc252e..f16c6f9fbf62e 100644
--- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
+++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
@@ -3447,13 +3447,7 @@ ProcessGDBRemote::EstablishConnectionIfNeeded(const ProcessInfo &process_info) {
if (platform_sp && !platform_sp->IsHost())
return Status::FromErrorString("Lost debug server connection");
- auto error = LaunchAndConnectToDebugserver(process_info);
- if (error.Fail()) {
- const char *error_string = error.AsCString();
- if (error_string == nullptr)
- error_string = "unable to launch " DEBUGSERVER_BASENAME;
- }
- return error;
+ return LaunchAndConnectToDebugserver(process_info);
}
static FileSpec GetDebugserverPath(Platform &platform) {
diff --git a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
index d0d1508e85172..09e1565072180 100644
--- a/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
+++ b/lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
@@ -270,7 +270,6 @@ bool ScriptedThread::CalculateStopInfo() {
uint32_t signal;
llvm::StringRef description;
if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
- signal = LLDB_INVALID_SIGNAL_NUMBER;
return false;
}
data_dict->GetValueForKeyAsString("desc", description);
diff --git a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
index 380986d8afab7..933fffe2f40b9 100644
--- a/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
+++ b/lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
@@ -139,8 +139,10 @@ SymbolVendorELF::CreateInstance(const lldb::ModuleSP &module_sp,
SectionList *module_section_list = module_sp->GetSectionList();
SectionList *objfile_section_list = dsym_objfile_sp->GetSectionList();
- if (!module_section_list || !objfile_section_list)
+ if (!module_section_list || !objfile_section_list) {
+ delete symbol_vendor;
return nullptr;
+ }
static const SectionType g_sections[] = {
eSectionTypeDWARFDebugAbbrev, eSectionTypeDWARFDebugAddr,
diff --git a/lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp b/lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
index c18af06fbdc98..97f4437683a5c 100644
--- a/lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
+++ b/lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
@@ -108,8 +108,10 @@ SymbolVendorWasm::CreateInstance(const lldb::ModuleSP &module_sp,
SectionList *module_section_list = module_sp->GetSectionList();
SectionList *objfile_section_list = sym_objfile_sp->GetSectionList();
- if (!module_section_list || !objfile_section_list)
+ if (!module_section_list || !objfile_section_list) {
+ delete symbol_vendor;
return nullptr;
+ }
static const SectionType g_sections[] = {
eSectionTypeDWARFDebugAbbrev, eSectionTypeDWARFDebugAddr,
diff --git a/lldb/source/Target/LanguageRuntime.cpp b/lldb/source/Target/LanguageRuntime.cpp
index 269d1e017fdf2..e1a1d9caea8fe 100644
--- a/lldb/source/Target/LanguageRuntime.cpp
+++ b/lldb/source/Target/LanguageRuntime.cpp
@@ -218,11 +218,8 @@ LanguageRuntime::LanguageRuntime(Process *process) : Runtime(process) {}
BreakpointPreconditionSP
LanguageRuntime::GetExceptionPrecondition(LanguageType language,
bool throw_bp) {
- LanguageRuntimeCreateInstance create_callback;
for (uint32_t idx = 0;
- (create_callback =
- PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) !=
- nullptr;
+ PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx) != nullptr;
idx++) {
if (auto precondition_callback =
PluginManager::GetLanguageRuntimeGetExceptionPreconditionAtIndex(
@@ -289,12 +286,8 @@ void LanguageRuntime::InitializeCommands(CommandObject *parent) {
if (!parent->IsMultiwordObject())
return;
- LanguageRuntimeCreateInstance create_callback;
-
for (uint32_t idx = 0;
- (create_callback =
- PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) !=
- nullptr;
+ PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx) != nullptr;
++idx) {
if (LanguageRuntimeGetCommandObject command_callback =
PluginManager::GetLanguageRuntimeGetCommandObjectAtIndex(idx)) {
diff --git a/lldb/source/Target/RegisterContextUnwind.cpp b/lldb/source/Target/RegisterContextUnwind.cpp
index 880300d0637fb..46eb4e927c9b3 100644
--- a/lldb/source/Target/RegisterContextUnwind.cpp
+++ b/lldb/source/Target/RegisterContextUnwind.cpp
@@ -2002,7 +2002,6 @@ bool RegisterContextUnwind::ReadFrameAddress(
"Got an invalid CFA register value - reg %s (%d), value 0x%" PRIx64,
cfa_reg.GetName(), cfa_reg.GetAsKind(eRegisterKindLLDB),
cfa_reg_contents);
- cfa_reg_contents = LLDB_INVALID_ADDRESS;
return false;
}
address = cfa_reg_contents + fa.GetOffset();
diff --git a/lldb/source/Target/StackFrame.cpp b/lldb/source/Target/StackFrame.cpp
index d97a814952186..cd7b6db6db59b 100644
--- a/lldb/source/Target/StackFrame.cpp
+++ b/lldb/source/Target/StackFrame.cpp
@@ -632,7 +632,6 @@ ValueObjectSP StackFrame::LegacyGetValueForVariableExpressionPath(
if (!instance_var_name.empty()) {
var_sp = variable_list->FindVariable(ConstString(instance_var_name));
if (var_sp) {
- separator_idx = 0;
if (Type *var_type = var_sp->GetType())
if (auto compiler_type = var_type->GetForwardCompilerType())
if (!compiler_type.IsPointerType())
@@ -735,7 +734,6 @@ ValueObjectSP StackFrame::LegacyGetValueForVariableExpressionPath(
[[fallthrough]];
case '.': {
var_expr = var_expr.drop_front(); // Remove the '.' or '>'
- separator_idx = var_expr.find_first_of(".-[");
ConstString child_name(var_expr.substr(0, var_expr.find_first_of(".-[")));
if (check_ptr_vs_member) {
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
There were a handful of places where clang asserted things that I wasn't as sure of, with my puny human brain, so I left the code as-is. But these were straightforward so I fixed them. |
kastiglione
reviewed
Jul 15, 2025
Comment on lines
-295
to
-297
| (create_callback = | ||
| PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx)) != | ||
| nullptr; |
There was a problem hiding this comment.
I've noticed this unnecessary assignment in the for-loop before, thanks for deleting it.
kastiglione
reviewed
Jul 15, 2025
Comment on lines
+543
to
+544
| [env_dict release]; | ||
| [options release]; |
There was a problem hiding this comment.
I wonder if these objc++ files can switch to using ARC (not that you should deal with that for this PR)
There was a problem hiding this comment.
yeah I thought the same thing, but it's been so long since I touched any Objective-C code, I didn't want to try to modernize it properly.
Michael137
reviewed
Jul 16, 2025
| @@ -2015,8 +2015,6 @@ llvm::Expected<Value> DWARFExpression::Evaluate( | |||
| if (stack.size() < 1) { | |||
| UpdateValueTypeFromLocationDescription(log, dwarf_cu, | |||
| LocationDescriptionKind::Empty); | |||
| // Reset for the next piece. | |||
| dwarf4_location_description_kind = Memory; | |||
There was a problem hiding this comment.
I'm a bit surprised by these ones. Why aren't these needed anymore?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I ran the clang static analyzer over the codebase on macOS, it identified some minor issues but nothing really good. Mostly dead variable assignments and a couple of places where we can leak an object if we early-return, etc.