Skip to content

Commit 9eca612

Browse files
authored
[ClangImporter] Avoid use-after-free of clang::DiagnosticOptions after rebranch (#85445)
Upstream LLVM in llvm/llvm-project#139584 changed `DiagnosticOptions` from being a referenced counted object to just be a reference, not owned by the `clang::DiagnosticEngine`. In 0981b71 (part of #82243), the usages of the Swift repository were adapted to the new memory model, but it introduced at least one use-after-free and a potential one around the usage of Clang in the Clang Importer. This commit tries to fix the use-after-free in both cases, by returning a `unique_ptr` to the `clang::DiagnosticOptions`, which makes the lifetime of the `DiagnosticOptions` match the lifetime of the variable that uses it (normally a `CompilerInvocation`). Other cases in 0981b71 should be safe because the lifetime of the `DiagnosticOptions` do not seem to propagate beyond the scope of the functions where they live (but I am not fully sure about the one in `IDETool/CompilerInvocation.cpp` completely). This was causing compiler crashes during the test `Interop/Cxx/stdlib/unsupported-stdlib.swift` which eventually uses `createClangDriver` and tries to emit a diagnostic, which in some cases was reading the memory from `DiagnosticOptions` when it was already out of scope.
1 parent 09c828c commit 9eca612

File tree

5 files changed

+37
-24
lines changed

5 files changed

+37
-24
lines changed

include/swift/ClangImporter/ClangImporter.h

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020
#include "swift/AST/AttrKind.h"
2121
#include "swift/AST/ClangModuleLoader.h"
2222
#include "clang/AST/Attr.h"
23+
#include "clang/Basic/DiagnosticOptions.h"
2324
#include "clang/Basic/Specifiers.h"
2425
#include "llvm/Support/VirtualFileSystem.h"
2526

@@ -213,7 +214,8 @@ class ClangImporter final : public ClangModuleLoader {
213214
std::vector<std::string>
214215
getClangDepScanningInvocationArguments(ASTContext &ctx);
215216

216-
static std::unique_ptr<clang::CompilerInvocation>
217+
static std::pair<std::unique_ptr<clang::CompilerInvocation>,
218+
std::unique_ptr<clang::DiagnosticOptions>>
217219
createClangInvocation(ClangImporter *importer,
218220
const ClangImporterOptions &importerOpts,
219221
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
@@ -223,8 +225,9 @@ class ClangImporter final : public ClangModuleLoader {
223225
///
224226
/// \return a pair of the Clang Driver and the diagnostic engine, which needs
225227
/// to be alive during the use of the Driver.
226-
static std::pair<clang::driver::Driver,
227-
llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>>
228+
static std::tuple<clang::driver::Driver,
229+
llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>,
230+
std::unique_ptr<clang::DiagnosticOptions>>
228231
createClangDriver(
229232
const LangOptions &LangOpts,
230233
const ClangImporterOptions &ClangImporterOpts,

lib/ClangImporter/ClangImporter.cpp

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1284,7 +1284,9 @@ std::optional<std::vector<std::string>> ClangImporter::getClangCC1Arguments(
12841284
return CI->getCC1CommandLine();
12851285
}
12861286

1287-
std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
1287+
std::pair<std::unique_ptr<clang::CompilerInvocation>,
1288+
std::unique_ptr<clang::DiagnosticOptions>>
1289+
ClangImporter::createClangInvocation(
12881290
ClangImporter *importer, const ClangImporterOptions &importerOpts,
12891291
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> VFS,
12901292
const std::vector<std::string> &CC1Args) {
@@ -1298,19 +1300,19 @@ std::unique_ptr<clang::CompilerInvocation> ClangImporter::createClangInvocation(
12981300
// option here is either generated by dependency scanner or just round tripped
12991301
// from `getClangCC1Arguments` so we don't expect it to fail. Use a simple
13001302
// printing diagnostics consumer for debugging any unexpected error.
1301-
clang::DiagnosticOptions diagOpts;
1303+
auto diagOpts = std::make_unique<clang::DiagnosticOptions>();
13021304
clang::DiagnosticsEngine clangDiags(
1303-
new clang::DiagnosticIDs(), diagOpts,
1304-
new clang::TextDiagnosticPrinter(llvm::errs(), diagOpts));
1305+
new clang::DiagnosticIDs(), *diagOpts,
1306+
new clang::TextDiagnosticPrinter(llvm::errs(), *diagOpts));
13051307

13061308
// Finally, use the CC1 command-line and the diagnostic engine
13071309
// to instantiate our Invocation.
13081310
auto CI = std::make_unique<clang::CompilerInvocation>();
13091311
if (!clang::CompilerInvocation::CreateFromArgs(
13101312
*CI, invocationArgs, clangDiags, importerOpts.clangPath.c_str()))
1311-
return nullptr;
1313+
return {nullptr, nullptr};
13121314

1313-
return CI;
1315+
return {std::move(CI), std::move(diagOpts)};
13141316
}
13151317

13161318
std::unique_ptr<ClangImporter> ClangImporter::create(
@@ -1358,8 +1360,9 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
13581360
[] { llvm::errs() << "' '"; });
13591361
llvm::errs() << "'\n";
13601362
}
1361-
importer->Impl.Invocation = createClangInvocation(
1362-
importer.get(), importerOpts, VFS, importer->Impl.ClangArgs);
1363+
std::tie(importer->Impl.Invocation, importer->Impl.DiagnosticOptions) =
1364+
createClangInvocation(importer.get(), importerOpts, VFS,
1365+
importer->Impl.ClangArgs);
13631366
if (!importer->Impl.Invocation)
13641367
return nullptr;
13651368
}
@@ -1435,7 +1438,7 @@ std::unique_ptr<ClangImporter> ClangImporter::create(
14351438
ctx, instance.getVirtualFileSystemPtr(), true);
14361439
if (!swiftTargetClangArgs)
14371440
return nullptr;
1438-
auto swiftTargetClangInvocation = createClangInvocation(
1441+
auto [swiftTargetClangInvocation, clangDiagOpts] = createClangInvocation(
14391442
importer.get(), importerOpts, instance.getVirtualFileSystemPtr(),
14401443
*swiftTargetClangArgs);
14411444
if (!swiftTargetClangInvocation)

lib/ClangImporter/ClangIncludePaths.cpp

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -122,22 +122,23 @@ parseClangDriverArgs(const clang::driver::Driver &clangDriver,
122122
return clangDriver.getOpts().ParseArgs(args, unused1, unused2);
123123
}
124124

125-
std::pair<clang::driver::Driver,
126-
llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>>
125+
std::tuple<clang::driver::Driver,
126+
llvm::IntrusiveRefCntPtr<clang::DiagnosticsEngine>,
127+
std::unique_ptr<clang::DiagnosticOptions>>
127128
ClangImporter::createClangDriver(
128129
const LangOptions &LangOpts, const ClangImporterOptions &ClangImporterOpts,
129130
llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> vfs) {
130131

131132
auto diagVFS = vfs ? vfs : llvm::vfs::getRealFileSystem();
132133

133-
clang::DiagnosticOptions diagOpts;
134+
auto diagOpts = std::make_unique<clang::DiagnosticOptions>();
134135
auto *silentDiagConsumer = new clang::DiagnosticConsumer();
135136
auto clangDiags = clang::CompilerInstance::createDiagnostics(
136-
*diagVFS, diagOpts, silentDiagConsumer);
137+
*diagVFS, *diagOpts, silentDiagConsumer);
137138
clang::driver::Driver clangDriver(ClangImporterOpts.clangPath,
138139
LangOpts.Target.str(), *clangDiags,
139140
"clang LLVM compiler", vfs);
140-
return {std::move(clangDriver), clangDiags};
141+
return {std::move(clangDriver), clangDiags, std::move(diagOpts)};
141142
}
142143

143144
/// Given a list of include paths and a list of file names, finds the first
@@ -208,8 +209,9 @@ getLibcFileMapping(const ASTContext &ctx, StringRef modulemapFileName,
208209
const llvm::Triple &triple = ctx.LangOpts.Target;
209210

210211
// Extract the libc path from Clang driver.
211-
auto [clangDriver, clangDiagEngine] = ClangImporter::createClangDriver(
212-
ctx.LangOpts, ctx.ClangImporterOpts, vfs);
212+
auto [clangDriver, clangDiagEngine, clangDiagOpts] =
213+
ClangImporter::createClangDriver(ctx.LangOpts, ctx.ClangImporterOpts,
214+
vfs);
213215
auto clangDriverArgs = ClangImporter::createClangArgs(
214216
ctx.ClangImporterOpts, ctx.SearchPathOpts, clangDriver);
215217

@@ -287,8 +289,9 @@ static void getLibStdCxxFileMapping(
287289
return;
288290

289291
// Extract the libstdc++ installation path from Clang driver.
290-
auto [clangDriver, clangDiagEngine] = ClangImporter::createClangDriver(
291-
ctx.LangOpts, ctx.ClangImporterOpts, vfs);
292+
auto [clangDriver, clangDiagEngine, clangDiagOpts] =
293+
ClangImporter::createClangDriver(ctx.LangOpts, ctx.ClangImporterOpts,
294+
vfs);
292295
auto clangDriverArgs = ClangImporter::createClangArgs(
293296
ctx.ClangImporterOpts, ctx.SearchPathOpts, clangDriver);
294297

@@ -482,8 +485,9 @@ void GetWindowsFileMappings(
482485
if (!Triple.isWindowsMSVCEnvironment())
483486
return;
484487

485-
auto [Driver, clangDiagEngine] = ClangImporter::createClangDriver(
486-
Context.LangOpts, Context.ClangImporterOpts, driverVFS);
488+
auto [Driver, clangDiagEngine, clangDiagOpts] =
489+
ClangImporter::createClangDriver(Context.LangOpts,
490+
Context.ClangImporterOpts, driverVFS);
487491
const llvm::opt::InputArgList Args = ClangImporter::createClangArgs(
488492
Context.ClangImporterOpts, Context.SearchPathOpts, Driver);
489493
const clang::driver::ToolChain &ToolChain = Driver.getToolChain(Args, Triple);

lib/ClangImporter/ImporterImpl.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,9 @@ class LLVM_LIBRARY_VISIBILITY ClangImporter::Implementation
551551
/// Clang arguments used to create the Clang invocation.
552552
std::vector<std::string> ClangArgs;
553553

554+
/// Clang diagnostic options used to create the Clang invocation.
555+
std::unique_ptr<clang::DiagnosticOptions> DiagnosticOptions;
556+
554557
/// Mapping from Clang swift_attr attribute text to the Swift source file(s)
555558
/// that contain that attribute text.
556559
///

lib/Frontend/CompilerInvocation.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -392,7 +392,7 @@ void CompilerInvocation::computeCXXStdlibOptions() {
392392
LangOpts.PlatformDefaultCXXStdlib = CXXStdlibKind::Msvcprt;
393393
} else if (LangOpts.Target.isOSLinux() || LangOpts.Target.isOSDarwin() ||
394394
LangOpts.Target.isOSFreeBSD()) {
395-
auto [clangDriver, clangDiagEngine] =
395+
auto [clangDriver, clangDiagEngine, clangDiagOpts] =
396396
ClangImporter::createClangDriver(LangOpts, ClangImporterOpts);
397397
auto clangDriverArgs = ClangImporter::createClangArgs(
398398
ClangImporterOpts, SearchPathOpts, clangDriver);

0 commit comments

Comments
 (0)