[lldb] Improve error message in ResolveSDKPathFromDebugInfo (NFC) #154607
Conversation
While debugging, I saw a log line of: > Failed to resolve SDK path: Error while searching for SDK (XcodeSDK ''): Unrecognized SDK type: Looking into how this might happen, it seems `ResolveSDKPathFromDebugInfo` appears to (implicitly) assume there's at least one compile unit. This change adds a precondition to return a meaningful error when there are no compile units.
|
@llvm/pr-subscribers-lldb Author: Dave Lee (kastiglione) ChangesWhile debugging, I saw a log line of: > Failed to resolve SDK path: Error while searching for SDK (XcodeSDK ''): Unrecognized SDK type: Looking into how this might happen, it seems Full diff: https://github.com/llvm/llvm-project/pull/154607.diff 1 Files Affected:
diff --git a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
index 1db7bc78013d7..dc8b9437a64e4 100644
--- a/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
+++ b/lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp
@@ -1039,7 +1039,12 @@ ResolveSDKPathFromDebugInfo(lldb_private::Target *target) {
SymbolFile *sym_file = exe_module_sp->GetSymbolFile();
if (!sym_file)
- return llvm::createStringError("Failed to get symbol file from module");
+ return llvm::createStringError("Failed to get symbol file from executable");
+
+ if (sym_file->GetNumCompileUnits() == 0)
+ return llvm::createStringError(
+ "Failed to resolve SDK for target: executable's symbol file has no "
+ "compile units");
XcodeSDK merged_sdk;
for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
|
| "compile units"); | ||
|
|
||
| XcodeSDK merged_sdk; | ||
| for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) { |
There was a problem hiding this comment.
note that merged_sdk will be empty if there are no compile units, but the subsequent code assumes the loop produces a non-empty sdk.
| SymbolFile *sym_file = exe_module_sp->GetSymbolFile(); | ||
| if (!sym_file) | ||
| return llvm::createStringError("Failed to get symbol file from module"); | ||
| return llvm::createStringError("Failed to get symbol file from executable"); |
There was a problem hiding this comment.
Not a big deal if not, but any way we can test this? There are some XcodeSDK unittests which might fit the bill
There was a problem hiding this comment.
I don't see an existing place to test this ResolveSDKPathFromDebugInfo. It's a function used specifically by AddClangModuleCompilationOptions, which doesn't have any existing tests.
There was a problem hiding this comment.
the other place Charles mentioned does have tests: GetSDKPathFromDebugInfo
There was a problem hiding this comment.
@Michael137 I tried to update XcodeSDKModuleTests.cpp but can't figure out how to construct a yaml that represents zero compile units. If you know how to do that, I'll make a follow up test.
|
Should this check also be added to the implementations of GetSDKPathFromDebugInfo? They also loop over the compile units and some callers assume the resolved sdk won't be empty. |
kastiglione
deleted the
lldb-Improve-error-message-in-ResolveSDKPathFromDebugInfo-NFC
branch
4 participants
While debugging, I saw a log line of:
Looking into how this might happen, it seems
ResolveSDKPathFromDebugInfoappears to(implicitly) assume there's at least one compile unit. This change adds a precondition
to return a meaningful error when there are no compile units.
Original: #146062