Skip to content

[lldb][NFC] Small fixes identified by the clang static analyzer #148773

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.

Sign up for GitHub

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

[lldb][NFC] Small fixes identified by the clang static analyzer #148773

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
cc1e774
[lldb][NFC] Small fixes identified by the clang static analyzer
jasonmolenda Jul 15, 2025
88a1f71
ws fix
jasonmolenda Jul 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions lldb/source/Expression/DWARFExpression.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit surprised by these ones. Why aren't these needed anymore?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a local variable in the DW_OP_piece block, and we return with an error here. It's a dead store.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be that stylistically we want to set dwarf4_location_description_kind in every clause of this DW_OP_piece block, and we should set it to stay consistent.

return llvm::createStringError(
"expression stack needs at least 1 item for DW_OP_bit_piece");
} else {
Expand Down Expand Up @@ -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));
}
Expand Down
6 changes: 5 additions & 1 deletion lldb/source/Host/macosx/objcxx/Host.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand Down Expand Up @@ -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;
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/DynamicLoader/Hexagon-DYLD/HexagonDYLDRendezvous.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
1 change: 0 additions & 1 deletion lldb/source/Plugins/ObjectFile/JSON/ObjectFileJSON.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}

Expand All @@ -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
Expand Down
3 changes: 3 additions & 0 deletions lldb/source/Plugins/Platform/MacOSX/objcxx/PlatformiOSSimulatorCoreSimulatorSupport.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];
Expand Down Expand Up @@ -539,6 +540,8 @@ static Status HandleFileAction(ProcessLaunchInfo &launch_info,
: "unable to launch");
}

[env_dict release];
[options release];
Comment on lines +543 to +544
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if these objc++ files can switch to using ARC (not that you should deal with that for this PR)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

return CoreSimulatorSupport::Process(pid, std::move(error));
}

Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/Platform/POSIX/PlatformPOSIX.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions lldb/source/Plugins/Process/Utility/ARMUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -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;
}

Expand Down
8 changes: 6 additions & 2 deletions lldb/source/Plugins/Process/Utility/NativeRegisterContextDBReg_x86.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions lldb/source/Plugins/Process/elf-core/ThreadElfCore.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
8 changes: 1 addition & 7 deletions lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 2 additions & 4 deletions lldb/source/Plugins/Process/scripted/ScriptedThread.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,8 @@ bool ScriptedThread::CalculateStopInfo() {
case lldb::eStopReasonSignal: {
uint32_t signal;
llvm::StringRef description;
if (!data_dict->GetValueForKeyAsInteger("signal", signal)) {
signal = LLDB_INVALID_SIGNAL_NUMBER;
return false;
}
if (!data_dict->GetValueForKeyAsInteger("signal", signal))
return false;
data_dict->GetValueForKeyAsString("desc", description);
stop_info_sp =
StopInfo::CreateStopReasonWithSignal(*this, signal, description.data());
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Plugins/SymbolVendor/ELF/SymbolVendorELF.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
4 changes: 3 additions & 1 deletion lldb/source/Plugins/SymbolVendor/wasm/SymbolVendorWasm.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
11 changes: 2 additions & 9 deletions lldb/source/Target/LanguageRuntime.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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;
Comment on lines -295 to -297
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've noticed this unnecessary assignment in the for-loop before, thanks for deleting it.

PluginManager::GetLanguageRuntimeCreateCallbackAtIndex(idx) != nullptr;
++idx) {
if (LanguageRuntimeGetCommandObject command_callback =
PluginManager::GetLanguageRuntimeGetCommandObjectAtIndex(idx)) {
Expand Down
1 change: 0 additions & 1 deletion lldb/source/Target/RegisterContextUnwind.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
2 changes: 0 additions & 2 deletions lldb/source/Target/StackFrame.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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) {
Expand Down
Loading