Skip to content

Commit

Permalink
[lldb] Fix incorrect uses of LLDB_LOG_ERROR
Browse files Browse the repository at this point in the history
Fix incorrect uses of LLDB_LOG_ERROR. The macro doesn't automatically
inject the error in the log message: it merely passes the error as the
first argument to formatv and therefore must be referenced with {0}.

Thanks to Nicholas Allegra for collecting a list of places where the
macro was misused.

rdar://111581655

Differential revision: https://reviews.llvm.org/D154530
  • Loading branch information
JDevlieghere committed Jul 5, 2023
1 parent 1ac3e13 commit e0e36e3
Show file tree
Hide file tree
Showing 15 changed files with 51 additions and 47 deletions.
4 changes: 2 additions & 2 deletions lldb/source/Breakpoint/Watchpoint.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ Watchpoint::Watchpoint(Target &target, lldb::addr_t addr, uint32_t size,
target.GetScratchTypeSystemForLanguage(eLanguageTypeC);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
"Failed to set type.");
"Failed to set type: {0}");
} else {
if (auto ts = *type_system_or_err)
m_type =
ts->GetBuiltinTypeForEncodingAndBitSize(eEncodingUint, 8 * size);
else
LLDB_LOG_ERROR(GetLog(LLDBLog::Watchpoints), std::move(err),
"Failed to set type. Typesystem is no longer live.");
"Failed to set type: Typesystem is no longer live: {0}");
}
}

Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Core/ValueObjectRegister.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ CompilerType ValueObjectRegister::GetCompilerTypeImpl() {
exe_module->GetTypeSystemForLanguage(eLanguageTypeC);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Types), std::move(err),
"Unable to get CompilerType from TypeSystem");
"Unable to get CompilerType from TypeSystem: {0}");
} else {
if (auto ts = *type_system_or_err)
m_compiler_type = ts->GetBuiltinTypeForEncodingAndBitSize(
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/Language/CPlusPlus/BlockPointer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class BlockPointerSyntheticFrontEnd : public SyntheticChildrenFrontEnd {
lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::DataFormatters), std::move(err),
"Failed to get scratch TypeSystemClang");
"Failed to get scratch TypeSystemClang: {0}");
return;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1685,7 +1685,7 @@ AppleObjCRuntimeV2::SharedCacheImageHeaders::CreateSharedCacheImageHeaders(
entsize));
if (auto Err = shared_cache_image_headers->UpdateIfNeeded()) {
LLDB_LOG_ERROR(log, std::move(Err),
"Failed to update SharedCacheImageHeaders");
"Failed to update SharedCacheImageHeaders: {0}");
return nullptr;
}

Expand Down Expand Up @@ -1745,7 +1745,7 @@ bool AppleObjCRuntimeV2::SharedCacheImageHeaders::IsImageLoaded(
if (auto Err = UpdateIfNeeded()) {
Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
LLDB_LOG_ERROR(log, std::move(Err),
"Failed to update SharedCacheImageHeaders");
"Failed to update SharedCacheImageHeaders: {0}");
}
return m_loaded_images.test(image_index);
}
Expand All @@ -1754,7 +1754,7 @@ uint64_t AppleObjCRuntimeV2::SharedCacheImageHeaders::GetVersion() {
if (auto Err = UpdateIfNeeded()) {
Log *log = GetLog(LLDBLog::Process | LLDBLog::Types);
LLDB_LOG_ERROR(log, std::move(Err),
"Failed to update SharedCacheImageHeaders");
"Failed to update SharedCacheImageHeaders: {0}");
}
return m_version;
}
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -729,7 +729,7 @@ bool ProcessKDP::StartAsyncThread() {
"<lldb.process.kdp-remote.async>", [this] { return AsyncThread(); });
if (!async_thread) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Host), async_thread.takeError(),
"failed to launch host thread: {}");
"failed to launch host thread: {0}");
return false;
}
m_async_thread = *async_thread;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3430,7 +3430,7 @@ bool ProcessGDBRemote::StartAsyncThread() {
});
if (!async_thread) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Host), async_thread.takeError(),
"failed to launch host thread: {}");
"failed to launch host thread: {0}");
return false;
}
m_async_thread = *async_thread;
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Plugins/SymbolFile/DWARF/DWARFDebugAranges.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void DWARFDebugAranges::extract(const DWARFDataExtractor &debug_aranges_data) {
Log *log = GetLog(DWARFLog::DebugInfo);
LLDB_LOG_ERROR(log, std::move(error),
"DWARFDebugAranges::extract failed to extract "
".debug_aranges set at offset %#" PRIx64,
".debug_aranges set at offset {1:x}: {0}",
set_offset);
} else {
const uint32_t num_descriptors = set.NumDescriptors();
Expand Down
14 changes: 7 additions & 7 deletions lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -830,7 +830,7 @@ Function *SymbolFileDWARF::ParseFunction(CompileUnit &comp_unit,
auto type_system_or_err = GetTypeSystemForLanguage(GetLanguage(*die.GetCU()));
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to parse function");
"Unable to parse function: {0}");
return nullptr;
}
auto ts = *type_system_or_err;
Expand Down Expand Up @@ -878,7 +878,7 @@ SymbolFileDWARF::ConstructFunctionDemangledName(const DWARFDIE &die) {
auto type_system_or_err = GetTypeSystemForLanguage(GetLanguage(*die.GetCU()));
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to construct demangled name for function");
"Unable to construct demangled name for function: {0}");
return ConstString();
}

Expand Down Expand Up @@ -1066,7 +1066,7 @@ SymbolFileDWARF::GetTypeUnitSupportFiles(DWARFTypeUnit &tu) {
Log *log = GetLog(DWARFLog::DebugInfo);
LLDB_LOG_ERROR(log, std::move(error),
"SymbolFileDWARF::GetTypeUnitSupportFiles failed to parse "
"the line table prologue");
"the line table prologue: {0}");
};
ElapsedTime elapsed(m_parse_time);
llvm::Error error = prologue.parse(data, &line_table_offset, report, ctx);
Expand Down Expand Up @@ -2131,7 +2131,7 @@ bool SymbolFileDWARF::DeclContextMatchesThisSymbolFile(
decl_ctx_type_system->GetMinimumLanguage(nullptr));
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to match namespace decl using TypeSystem");
"Unable to match namespace decl using TypeSystem: {0}");
return false;
}

Expand Down Expand Up @@ -2977,7 +2977,7 @@ SymbolFileDWARF::FindDefinitionTypeForDWARFDeclContext(const DWARFDIE &die) {
auto type_system_or_err = GetTypeSystemForLanguage(language);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Cannot get TypeSystem for language {}",
"Cannot get TypeSystem for language {1}: {0}",
Language::GetNameForLanguageType(language));
} else {
type_system = *type_system_or_err;
Expand Down Expand Up @@ -3100,7 +3100,7 @@ TypeSP SymbolFileDWARF::ParseType(const SymbolContext &sc, const DWARFDIE &die,
auto type_system_or_err = GetTypeSystemForLanguage(GetLanguage(*die.GetCU()));
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to parse type");
"Unable to parse type: {0}");
return {};
}
auto ts = *type_system_or_err;
Expand Down Expand Up @@ -4162,7 +4162,7 @@ DWARFASTParser *SymbolFileDWARF::GetDWARFParser(DWARFUnit &unit) {
auto type_system_or_err = GetTypeSystem(unit);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get DWARFASTParser");
"Unable to get DWARFASTParser: {0}");
return nullptr;
}
if (auto ts = *type_system_or_err)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ void SymbolFileNativePDB::InitializeObject() {
lldb::eLanguageTypeC_plus_plus);
if (auto err = ts_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Failed to initialize");
"Failed to initialize: {0}");
} else {
if (auto ts = *ts_or_err)
ts->SetSymbolFile(this);
Expand Down
22 changes: 11 additions & 11 deletions lldb/source/Plugins/SymbolFile/PDB/SymbolFilePDB.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ SymbolFilePDB::ParseCompileUnitFunctionForPDBFunc(const PDBSymbolFunc &pdb_func,
auto type_system_or_err = GetTypeSystemForLanguage(lang);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to parse PDBFunc");
"Unable to parse PDBFunc: {0}");
return nullptr;
}

Expand Down Expand Up @@ -570,7 +570,7 @@ lldb_private::Type *SymbolFilePDB::ResolveTypeUID(lldb::user_id_t type_uid) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to ResolveTypeUID");
"Unable to ResolveTypeUID: {0}");
return nullptr;
}

Expand Down Expand Up @@ -607,7 +607,7 @@ bool SymbolFilePDB::CompleteType(lldb_private::CompilerType &compiler_type) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get dynamic array info for UID");
"Unable to get dynamic array info for UID: {0}");
return false;
}
auto ts = *type_system_or_err;
Expand All @@ -629,7 +629,7 @@ lldb_private::CompilerDecl SymbolFilePDB::GetDeclForUID(lldb::user_id_t uid) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get decl for UID");
"Unable to get decl for UID: {0}");
return CompilerDecl();
}
auto ts = *type_system_or_err;
Expand Down Expand Up @@ -659,7 +659,7 @@ SymbolFilePDB::GetDeclContextForUID(lldb::user_id_t uid) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get DeclContext for UID");
"Unable to get DeclContext for UID: {0}");
return CompilerDeclContext();
}

Expand Down Expand Up @@ -690,7 +690,7 @@ SymbolFilePDB::GetDeclContextContainingUID(lldb::user_id_t uid) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get DeclContext containing UID");
"Unable to get DeclContext containing UID: {0}");
return CompilerDeclContext();
}

Expand Down Expand Up @@ -720,7 +720,7 @@ void SymbolFilePDB::ParseDeclsForContext(
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to parse decls for context");
"Unable to parse decls for context: {0}");
return;
}

Expand Down Expand Up @@ -1469,7 +1469,7 @@ void SymbolFilePDB::DumpClangAST(Stream &s) {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to dump ClangAST");
"Unable to dump ClangAST: {0}");
return;
}

Expand Down Expand Up @@ -1682,7 +1682,7 @@ PDBASTParser *SymbolFilePDB::GetPDBAstParser() {
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to get PDB AST parser");
"Unable to get PDB AST parser: {0}");
return nullptr;
}

Expand All @@ -1703,7 +1703,7 @@ SymbolFilePDB::FindNamespace(lldb_private::ConstString name,
GetTypeSystemForLanguage(lldb::eLanguageTypeC_plus_plus);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to find namespace {}", name.AsCString());
"Unable to find namespace {1}: {0}", name.AsCString());
return CompilerDeclContext();
}
auto ts = *type_system_or_err;
Expand Down Expand Up @@ -1998,7 +1998,7 @@ bool SymbolFilePDB::DeclContextMatchesThisSymbolFile(
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(
GetLog(LLDBLog::Symbols), std::move(err),
"Unable to determine if DeclContext matches this symbol file");
"Unable to determine if DeclContext matches this symbol file: {0}");
return false;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ lldb::addr_t AppleGetItemInfoHandler::SetupGetItemInfoFunction(
eLanguageTypeObjC, exe_ctx);
if (!utility_fn_or_error) {
LLDB_LOG_ERROR(log, utility_fn_or_error.takeError(),
"Failed to create utility function: {0}.");
"Failed to create utility function: {0}");
}
m_get_item_info_impl_code = std::move(*utility_fn_or_error);
} else {
Expand All @@ -162,13 +162,13 @@ lldb::addr_t AppleGetItemInfoHandler::SetupGetItemInfoFunction(
eLanguageTypeC);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(log, std::move(err),
"Error inserting get-item-info function");
"Error inserting get-item-info function: {0}");
return args_addr;
}
auto ts = *type_system_or_err;
if (!ts)
return args_addr;

CompilerType get_item_info_return_type =
ts->GetBasicTypeFromAST(eBasicTypeVoid)
.GetPointerType();
Expand Down
5 changes: 3 additions & 2 deletions lldb/source/Symbol/Type.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -546,8 +546,9 @@ bool Type::ResolveCompilerType(ResolveState compiler_type_resolve_state) {
auto type_system_or_err =
m_symbol_file->GetTypeSystemForLanguage(eLanguageTypeC);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Symbols), std::move(err),
"Unable to construct void type from TypeSystemClang");
LLDB_LOG_ERROR(
GetLog(LLDBLog::Symbols), std::move(err),
"Unable to construct void type from TypeSystemClang: {0}");
} else {
CompilerType void_compiler_type;
auto ts = *type_system_or_err;
Expand Down
2 changes: 1 addition & 1 deletion lldb/source/Target/StackFrame.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1360,7 +1360,7 @@ lldb::ValueObjectSP StackFrame::GuessValueForAddress(lldb::addr_t addr) {
target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
if (auto err = c_type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Thread), std::move(err),
"Unable to guess value for given address");
"Unable to guess value for given address: {0}");
return ValueObjectSP();
} else {
auto ts = *c_type_system_or_err;
Expand Down
20 changes: 11 additions & 9 deletions lldb/source/Target/Target.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2387,10 +2387,11 @@ Target::GetScratchTypeSystems(bool create_on_demand) {
auto type_system_or_err =
GetScratchTypeSystemForLanguage(language, create_on_demand);
if (!type_system_or_err)
LLDB_LOG_ERROR(GetLog(LLDBLog::Target), type_system_or_err.takeError(),
"Language '{}' has expression support but no scratch type "
"system available",
Language::GetNameForLanguageType(language));
LLDB_LOG_ERROR(
GetLog(LLDBLog::Target), type_system_or_err.takeError(),
"Language '{1}' has expression support but no scratch type "
"system available: {0}",
Language::GetNameForLanguageType(language));
else
if (auto ts = *type_system_or_err)
scratch_type_systems.push_back(ts);
Expand All @@ -2408,17 +2409,18 @@ Target::GetPersistentExpressionStateForLanguage(lldb::LanguageType language) {
auto type_system_or_err = GetScratchTypeSystemForLanguage(language, true);

if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
"Unable to get persistent expression state for language {}",
Language::GetNameForLanguageType(language));
LLDB_LOG_ERROR(
GetLog(LLDBLog::Target), std::move(err),
"Unable to get persistent expression state for language {1}: {0}",
Language::GetNameForLanguageType(language));
return nullptr;
}

if (auto ts = *type_system_or_err)
return ts->GetPersistentExpressionState();

LLDB_LOG(GetLog(LLDBLog::Target),
"Unable to get persistent expression state for language {}",
"Unable to get persistent expression state for language {1}: {0}",
Language::GetNameForLanguageType(language));
return nullptr;
}
Expand Down Expand Up @@ -2617,7 +2619,7 @@ ExpressionResults Target::EvaluateExpression(
auto ts = *type_system_or_err;
if (!ts)
LLDB_LOG_ERROR(GetLog(LLDBLog::Target), std::move(err),
"Scratch type system is no longer live");
"Scratch type system is no longer live: {0}");
else
persistent_var_sp =
ts->GetPersistentExpressionState()->GetVariable(expr);
Expand Down
7 changes: 4 additions & 3 deletions lldb/source/Target/ThreadPlanTracer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Stream *ThreadPlanTracer::GetLogStream() {
Thread &ThreadPlanTracer::GetThread() {
if (m_thread)
return *m_thread;

ThreadSP thread_sp = m_process.GetThreadList().FindThreadByID(m_tid);
m_thread = thread_sp.get();
return *m_thread;
Expand Down Expand Up @@ -107,8 +107,9 @@ TypeFromUser ThreadPlanAssemblyTracer::GetIntPointerType() {
auto type_system_or_err =
target_sp->GetScratchTypeSystemForLanguage(eLanguageTypeC);
if (auto err = type_system_or_err.takeError()) {
LLDB_LOG_ERROR(GetLog(LLDBLog::Types), std::move(err),
"Unable to get integer pointer type from TypeSystem");
LLDB_LOG_ERROR(
GetLog(LLDBLog::Types), std::move(err),
"Unable to get integer pointer type from TypeSystem: {0}");
} else {
if (auto ts = *type_system_or_err)
m_intptr_type = TypeFromUser(ts->GetBuiltinTypeForEncodingAndBitSize(
Expand Down

0 comments on commit e0e36e3

Please sign in to comment.