Skip to content

Commit 9ae41f0

Browse files
[lldb][Darwin] revert change to lang_opts.BuiltinHeadersInSystemModules (#145864)
Revert the changes made in the following PRs as they are causing bot failures: - #145744 - #144913
1 parent d3fd792 commit 9ae41f0

File tree

7 files changed

+140
-27
lines changed

7 files changed

+140
-27
lines changed

lldb/include/lldb/Target/Platform.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -458,6 +458,22 @@ class Platform : public PluginInterface {
458458
LLVM_PRETTY_FUNCTION, GetName()));
459459
}
460460

461+
/// Returns the full path of the most appropriate SDK for the
462+
/// specified 'module'. This function gets this path by parsing
463+
/// debug-info (see \ref `GetSDKPathFromDebugInfo`).
464+
///
465+
/// \param[in] module Module whose debug-info to parse for
466+
/// which SDK it was compiled against.
467+
///
468+
/// \returns If successful, returns the full path to an
469+
/// Xcode SDK.
470+
virtual llvm::Expected<std::string>
471+
ResolveSDKPathFromDebugInfo(Module &module) {
472+
return llvm::createStringError(
473+
llvm::formatv("{0} not implemented for '{1}' platform.",
474+
LLVM_PRETTY_FUNCTION, GetName()));
475+
}
476+
461477
/// Search CU for the SDK path the CUs was compiled against.
462478
///
463479
/// \param[in] unit The CU

lldb/include/lldb/Utility/XcodeSDK.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,19 @@ class XcodeSDK {
9393
static bool SDKSupportsModules(Type type, llvm::VersionTuple version);
9494
static bool SDKSupportsModules(Type desired_type, const FileSpec &sdk_path);
9595

96+
/// Returns true if the SDK for the specified triple supports
97+
/// builtin modules in system headers.
98+
///
99+
/// NOTE: should be kept in sync with sdkSupportsBuiltinModules in
100+
/// Toolchains/Darwin.cpp
101+
///
102+
/// FIXME: this function will be removed once LLDB's ClangExpressionParser
103+
/// constructs the compiler instance through the driver/toolchain. See \ref
104+
/// SetupImportStdModuleLangOpts
105+
///
106+
static bool SDKSupportsBuiltinModules(const llvm::Triple &target_triple,
107+
llvm::VersionTuple sdk_version);
108+
96109
/// Return the canonical SDK name, such as "macosx" for the macOS SDK.
97110
static std::string GetCanonicalName(Info info);
98111
/// Return the best-matching SDK type for a specific triple.

lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,49 @@ class ClangDiagnosticManagerAdapter : public clang::DiagnosticConsumer {
319319
StringRef m_filename;
320320
};
321321

322+
/// Returns true if the SDK for the specified triple supports
323+
/// builtin modules in system headers. This is used to decide
324+
/// whether to pass -fbuiltin-headers-in-system-modules to
325+
/// the compiler instance when compiling the `std` module.
326+
static llvm::Expected<bool>
327+
sdkSupportsBuiltinModules(lldb_private::Target &target) {
328+
auto arch_spec = target.GetArchitecture();
329+
auto const &triple = arch_spec.GetTriple();
330+
auto module_sp = target.GetExecutableModule();
331+
if (!module_sp)
332+
return llvm::createStringError("Executable module not found.");
333+
334+
// Get SDK path that the target was compiled against.
335+
auto platform_sp = target.GetPlatform();
336+
if (!platform_sp)
337+
return llvm::createStringError("No Platform plugin found on target.");
338+
339+
auto sdk_or_err = platform_sp->GetSDKPathFromDebugInfo(*module_sp);
340+
if (!sdk_or_err)
341+
return sdk_or_err.takeError();
342+
343+
// Use the SDK path from debug-info to find a local matching SDK directory.
344+
auto sdk_path_or_err =
345+
HostInfo::GetSDKRoot(HostInfo::SDKOptions{std::move(sdk_or_err->first)});
346+
if (!sdk_path_or_err)
347+
return sdk_path_or_err.takeError();
348+
349+
auto VFS = FileSystem::Instance().GetVirtualFileSystem();
350+
if (!VFS)
351+
return llvm::createStringError("No virtual filesystem available.");
352+
353+
// Extract SDK version from the /path/to/some.sdk/SDKSettings.json
354+
auto parsed_or_err = clang::parseDarwinSDKInfo(*VFS, *sdk_path_or_err);
355+
if (!parsed_or_err)
356+
return parsed_or_err.takeError();
357+
358+
auto maybe_sdk = *parsed_or_err;
359+
if (!maybe_sdk)
360+
return llvm::createStringError("Couldn't find Darwin SDK info.");
361+
362+
return XcodeSDK::SDKSupportsBuiltinModules(triple, maybe_sdk->getVersion());
363+
}
364+
322365
static void SetupModuleHeaderPaths(CompilerInstance *compiler,
323366
std::vector<std::string> include_directories,
324367
lldb::TargetSP target_sp) {
@@ -662,6 +705,7 @@ static void SetupLangOpts(CompilerInstance &compiler,
662705

663706
static void SetupImportStdModuleLangOpts(CompilerInstance &compiler,
664707
lldb_private::Target &target) {
708+
Log *log = GetLog(LLDBLog::Expressions);
665709
LangOptions &lang_opts = compiler.getLangOpts();
666710
lang_opts.Modules = true;
667711
// We want to implicitly build modules.
@@ -679,7 +723,12 @@ static void SetupImportStdModuleLangOpts(CompilerInstance &compiler,
679723
lang_opts.GNUKeywords = true;
680724
lang_opts.CPlusPlus11 = true;
681725

682-
lang_opts.BuiltinHeadersInSystemModules = false;
726+
if (auto supported_or_err = sdkSupportsBuiltinModules(target))
727+
lang_opts.BuiltinHeadersInSystemModules = !*supported_or_err;
728+
else
729+
LLDB_LOG_ERROR(log, supported_or_err.takeError(),
730+
"Failed to determine BuiltinHeadersInSystemModules when "
731+
"setting up import-std-module: {0}");
683732

684733
// The Darwin libc expects this macro to be set.
685734
lang_opts.GNUCVersion = 40201;

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.cpp

Lines changed: 31 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -1130,33 +1130,13 @@ void PlatformDarwin::AddClangModuleCompilationOptionsForSDKType(
11301130

11311131
if (target) {
11321132
if (ModuleSP exe_module_sp = target->GetExecutableModule()) {
1133-
SymbolFile *sym_file = exe_module_sp->GetSymbolFile();
1134-
if (!sym_file)
1135-
return;
1136-
1137-
XcodeSDK merged_sdk;
1138-
for (unsigned i = 0; i < sym_file->GetNumCompileUnits(); ++i) {
1139-
if (auto cu_sp = sym_file->GetCompileUnitAtIndex(i)) {
1140-
auto cu_sdk = sym_file->ParseXcodeSDK(*cu_sp);
1141-
merged_sdk.Merge(cu_sdk);
1142-
}
1143-
}
1144-
1145-
// TODO: The result of this loop is almost equivalent to deriving the SDK
1146-
// from the target triple, which would be a lot cheaper.
1147-
1148-
if (FileSystem::Instance().Exists(merged_sdk.GetSysroot())) {
1149-
sysroot_spec = merged_sdk.GetSysroot();
1133+
auto path_or_err = ResolveSDKPathFromDebugInfo(*exe_module_sp);
1134+
if (path_or_err) {
1135+
sysroot_spec = FileSpec(*path_or_err);
11501136
} else {
1151-
auto path_or_err =
1152-
HostInfo::GetSDKRoot(HostInfo::SDKOptions{merged_sdk});
1153-
if (path_or_err) {
1154-
sysroot_spec = FileSpec(*path_or_err);
1155-
} else {
1156-
LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
1157-
path_or_err.takeError(),
1158-
"Failed to resolve SDK path: {0}");
1159-
}
1137+
LLDB_LOG_ERROR(GetLog(LLDBLog::Types | LLDBLog::Host),
1138+
path_or_err.takeError(),
1139+
"Failed to resolve SDK path: {0}");
11601140
}
11611141
}
11621142
}
@@ -1404,6 +1384,31 @@ PlatformDarwin::GetSDKPathFromDebugInfo(Module &module) {
14041384
return std::pair{std::move(merged_sdk), found_mismatch};
14051385
}
14061386

1387+
llvm::Expected<std::string>
1388+
PlatformDarwin::ResolveSDKPathFromDebugInfo(Module &module) {
1389+
auto sdk_or_err = GetSDKPathFromDebugInfo(module);
1390+
if (!sdk_or_err)
1391+
return llvm::createStringError(
1392+
llvm::inconvertibleErrorCode(),
1393+
llvm::formatv("Failed to parse SDK path from debug-info: {0}",
1394+
llvm::toString(sdk_or_err.takeError())));
1395+
1396+
auto [sdk, _] = std::move(*sdk_or_err);
1397+
1398+
if (FileSystem::Instance().Exists(sdk.GetSysroot()))
1399+
return sdk.GetSysroot().GetPath();
1400+
1401+
auto path_or_err = HostInfo::GetSDKRoot(HostInfo::SDKOptions{sdk});
1402+
if (!path_or_err)
1403+
return llvm::createStringError(
1404+
llvm::inconvertibleErrorCode(),
1405+
llvm::formatv("Error while searching for SDK (XcodeSDK '{0}'): {1}",
1406+
sdk.GetString(),
1407+
llvm::toString(path_or_err.takeError())));
1408+
1409+
return path_or_err->str();
1410+
}
1411+
14071412
llvm::Expected<XcodeSDK>
14081413
PlatformDarwin::GetSDKPathFromDebugInfo(CompileUnit &unit) {
14091414
ModuleSP module_sp = unit.CalculateSymbolContextModule();

lldb/source/Plugins/Platform/MacOSX/PlatformDarwin.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ class PlatformDarwin : public PlatformPOSIX {
120120
llvm::Expected<std::pair<XcodeSDK, bool>>
121121
GetSDKPathFromDebugInfo(Module &module) override;
122122

123+
llvm::Expected<std::string>
124+
ResolveSDKPathFromDebugInfo(Module &module) override;
125+
123126
llvm::Expected<XcodeSDK> GetSDKPathFromDebugInfo(CompileUnit &unit) override;
124127

125128
llvm::Expected<std::string>

lldb/source/Utility/XcodeSDK.cpp

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -266,6 +266,27 @@ bool XcodeSDK::SupportsSwift() const {
266266
}
267267
}
268268

269+
bool XcodeSDK::SDKSupportsBuiltinModules(const llvm::Triple &target_triple,
270+
llvm::VersionTuple sdk_version) {
271+
using namespace llvm;
272+
273+
switch (target_triple.getOS()) {
274+
case Triple::OSType::MacOSX:
275+
return sdk_version >= VersionTuple(15U);
276+
case Triple::OSType::IOS:
277+
return sdk_version >= VersionTuple(18U);
278+
case Triple::OSType::TvOS:
279+
return sdk_version >= VersionTuple(18U);
280+
case Triple::OSType::WatchOS:
281+
return sdk_version >= VersionTuple(11U);
282+
case Triple::OSType::XROS:
283+
return sdk_version >= VersionTuple(2U);
284+
default:
285+
// New SDKs support builtin modules from the start.
286+
return true;
287+
}
288+
}
289+
269290
bool XcodeSDK::SDKSupportsModules(XcodeSDK::Type desired_type,
270291
const FileSpec &sdk_path) {
271292
ConstString last_path_component = sdk_path.GetFilename();

lldb/unittests/SymbolFile/DWARF/XcodeSDKModuleTests.cpp

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,9 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_InvalidSDKPath) {
161161

162162
auto platform_sp = Platform::GetHostPlatform();
163163
ASSERT_TRUE(platform_sp);
164+
auto path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module);
165+
EXPECT_FALSE(static_cast<bool>(path_or_err));
166+
llvm::consumeError(path_or_err.takeError());
164167
}
165168

166169
TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
@@ -204,6 +207,9 @@ TEST_F(XcodeSDKModuleTests, TestSDKPathFromDebugInfo_No_DW_AT_APPLE_sdk) {
204207

205208
auto platform_sp = Platform::GetHostPlatform();
206209
ASSERT_TRUE(platform_sp);
210+
auto path_or_err = platform_sp->ResolveSDKPathFromDebugInfo(*module);
211+
EXPECT_FALSE(static_cast<bool>(path_or_err));
212+
llvm::consumeError(path_or_err.takeError());
207213
}
208214

209215
TEST_P(SDKPathParsingMultiparamTests, TestSDKPathFromDebugInfo) {

0 commit comments

Comments
 (0)