Skip to content

Commit 0e9192b

Browse files
akyrtziakyrtzi
committed
[libclang/depscan] Fix use-after-free issue when using diagnostics of clang_experimental_DepGraph_getDiagnostics
The `StoredDiagnostic`s captured from the `getFileDependencies()` call reference a `SourceManager` object that gets destroyed and is invalid to use for getting diagnostic location info. To address this, capture diagnostics as a serialized diagnostics buffer and "materialize" it for the `clang_experimental_DepGraph_getDiagnostics` call, by re-using existing libclang machinery for loading serialized diagnostics. Also delete `clang_experimental_DependencyScannerWorker_getFileDependencies_v5` since it's unsafe to use to get diagnostics and its functionality is superseeded by `clang_experimental_DependencyScannerWorker_getDepGraph`. rdar://105978877 (cherry picked from commit 397a30d)
1 parent 3409d7d commit 0e9192b

File tree

10 files changed

+99
-80
lines changed

10 files changed

+99
-80
lines changed

‎clang/include/clang-c/Dependencies.h

Lines changed: 5 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -310,18 +310,8 @@ typedef size_t CXModuleLookupOutputCallback(void *Context,
310310
char *Output, size_t MaxLen);
311311

312312
/**
313-
* See \c clang_experimental_DependencyScannerWorker_getFileDependencies_v5.
314-
* Returns diagnostics in an unstructured CXString instead of CXDiagnosticSet.
315-
*/
316-
CINDEX_LINKAGE enum CXErrorCode
317-
clang_experimental_DependencyScannerWorker_getFileDependencies_v4(
318-
CXDependencyScannerWorker Worker, int argc, const char *const *argv,
319-
const char *ModuleName, const char *WorkingDirectory, void *MDCContext,
320-
CXModuleDiscoveredCallback *MDC, void *MLOContext,
321-
CXModuleLookupOutputCallback *MLO, unsigned Options,
322-
CXFileDependenciesList **Out, CXString *error);
323-
324-
/**
313+
* Deprecated, use \c clang_experimental_DependencyScannerWorker_getDepGraph.
314+
*
325315
* Calculates the list of file dependencies for a particular compiler
326316
* invocation.
327317
*
@@ -351,19 +341,18 @@ clang_experimental_DependencyScannerWorker_getFileDependencies_v4(
351341
* \param [out] Out A non-NULL pointer to store the resulting dependencies. The
352342
* output must be freed by calling
353343
* \c clang_experimental_FileDependenciesList_dispose.
354-
* \param [out] OutDiags The diagnostics emitted during scanning. These must be
355-
* always freed by calling \c clang_disposeDiagnosticSet.
344+
* \param [out] error the error string to pass back to client (if any).
356345
*
357346
* \returns \c CXError_Success on success; otherwise a non-zero \c CXErrorCode
358347
* indicating the kind of error.
359348
*/
360349
CINDEX_LINKAGE enum CXErrorCode
361-
clang_experimental_DependencyScannerWorker_getFileDependencies_v5(
350+
clang_experimental_DependencyScannerWorker_getFileDependencies_v4(
362351
CXDependencyScannerWorker Worker, int argc, const char *const *argv,
363352
const char *ModuleName, const char *WorkingDirectory, void *MDCContext,
364353
CXModuleDiscoveredCallback *MDC, void *MLOContext,
365354
CXModuleLookupOutputCallback *MLO, unsigned Options,
366-
CXFileDependenciesList **Out, CXDiagnosticSet *OutDiags);
355+
CXFileDependenciesList **Out, CXString *error);
367356

368357
/**
369358
* Output of \c clang_experimental_DependencyScannerWorker_getDepGraph.

‎clang/include/clang/Frontend/SerializedDiagnosticReader.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,9 @@ class SerializedDiagnosticReader {
6565
/// Read the diagnostics in \c File
6666
std::error_code readDiagnostics(StringRef File);
6767

68+
/// Read the diagnostics in \c Buffer.
69+
std::error_code readDiagnostics(llvm::MemoryBufferRef Buffer);
70+
6871
private:
6972
enum class Cursor;
7073

‎clang/lib/Frontend/SerializedDiagnosticPrinter.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -608,6 +608,9 @@ void SDiagsWriter::HandleDiagnostic(DiagnosticsEngine::Level DiagLevel,
608608
return;
609609
}
610610

611+
// Call base class to update diagnostic counts.
612+
DiagnosticConsumer::HandleDiagnostic(DiagLevel, Info);
613+
611614
// Enter the block for a non-note diagnostic immediately, rather than waiting
612615
// for beginDiagnostic, in case associated notes are emitted before we get
613616
// there.

‎clang/lib/Frontend/SerializedDiagnosticReader.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,12 @@ std::error_code SerializedDiagnosticReader::readDiagnostics(StringRef File) {
3434
if (!Buffer)
3535
return SDError::CouldNotLoad;
3636

37-
llvm::BitstreamCursor Stream(**Buffer);
37+
return readDiagnostics(**Buffer);
38+
}
39+
40+
std::error_code
41+
SerializedDiagnosticReader::readDiagnostics(llvm::MemoryBufferRef Buffer) {
42+
llvm::BitstreamCursor Stream(Buffer);
3843
Optional<llvm::BitstreamBlockInfo> BlockInfo;
3944

4045
if (Stream.AtEndOfStream())

‎clang/test/Index/Core/scan-deps-with-diags.m

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
// RUN: not c-index-test core --scan-deps %S -output-dir=%t -- \
2+
// RUN: %clang -c %s -o %t/t.o 2> %t.err.txt
3+
// RUN: FileCheck -input-file=%t.err.txt %s
4+
5+
// CHECK: [[@LINE+1]]:10: fatal error: 'not-existent.h' file not found
6+
#include "not-existent.h"

‎clang/tools/c-index-test/core_main.cpp

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -846,7 +846,8 @@ static int scanDeps(ArrayRef<const char *> Args, std::string WorkingDirectory,
846846
llvm::make_scope_exit([&]() { clang_disposeDiagnosticSet(Diags); });
847847
for (unsigned I = 0, N = clang_getNumDiagnosticsInSet(Diags); I < N; ++I) {
848848
CXDiagnostic Diag = clang_getDiagnosticInSet(Diags, I);
849-
CXString Spelling = clang_getDiagnosticSpelling(Diag);
849+
CXString Spelling =
850+
clang_formatDiagnostic(Diag, clang_defaultDiagnosticDisplayOptions());
850851
llvm::errs() << clang_getCString(Spelling) << "\n";
851852
clang_disposeString(Spelling);
852853
clang_disposeDiagnostic(Diag);

‎clang/tools/libclang/CDependencies.cpp

Lines changed: 24 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,13 @@
1313

1414
#include "CASUtils.h"
1515
#include "CXDiagnosticSetDiagnosticConsumer.h"
16+
#include "CXLoadedDiagnostic.h"
1617
#include "CXString.h"
1718

1819
#include "clang-c/Dependencies.h"
1920

2021
#include "clang/Frontend/CompilerInstance.h"
22+
#include "clang/Frontend/SerializedDiagnosticPrinter.h"
2123
#include "clang/Tooling/DependencyScanning/DependencyScanningService.h"
2224
#include "clang/Tooling/DependencyScanning/DependencyScanningTool.h"
2325
#include "clang/Tooling/DependencyScanning/DependencyScanningWorker.h"
@@ -342,49 +344,6 @@ CXErrorCode clang_experimental_DependencyScannerWorker_getFileDependencies_v4(
342344
return Result;
343345
}
344346

345-
CXErrorCode clang_experimental_DependencyScannerWorker_getFileDependencies_v5(
346-
CXDependencyScannerWorker W, int argc, const char *const *argv,
347-
const char *ModuleName, const char *WorkingDirectory, void *MDCContext,
348-
CXModuleDiscoveredCallback *MDC, void *MLOContext,
349-
CXModuleLookupOutputCallback *MLO, unsigned, CXFileDependenciesList **Out,
350-
CXDiagnosticSet *OutDiags) {
351-
OutputLookup OL(MLOContext, MLO);
352-
auto LookupOutputs = [&](const ModuleID &ID, ModuleOutputKind MOK) {
353-
return OL.lookupModuleOutput(ID, MOK);
354-
};
355-
356-
if (!Out)
357-
return CXError_InvalidArguments;
358-
*Out = nullptr;
359-
360-
CXDiagnosticSetDiagnosticConsumer DiagConsumer;
361-
362-
CXErrorCode Result = getFileDependencies(
363-
W, argc, argv, WorkingDirectory, MDC, MDCContext, nullptr, &DiagConsumer,
364-
LookupOutputs, ModuleName ? Optional<StringRef>(ModuleName) : None,
365-
[&](TranslationUnitDeps TU) {
366-
assert(TU.DriverCommandLine.empty());
367-
std::vector<std::string> Modules;
368-
for (const ModuleID &MID : TU.ClangModuleDeps)
369-
Modules.push_back(MID.ModuleName + ":" + MID.ContextHash);
370-
auto *Commands = new CXTranslationUnitCommand[TU.Commands.size()];
371-
for (size_t I = 0, E = TU.Commands.size(); I < E; ++I) {
372-
Commands[I].ContextHash = cxstring::createDup(TU.ID.ContextHash);
373-
Commands[I].FileDeps = cxstring::createSet(TU.FileDeps);
374-
Commands[I].ModuleDeps = cxstring::createSet(Modules);
375-
Commands[I].Executable =
376-
cxstring::createDup(TU.Commands[I].Executable);
377-
Commands[I].BuildArguments =
378-
cxstring::createSet(TU.Commands[I].Arguments);
379-
}
380-
*Out = new CXFileDependenciesList{TU.Commands.size(), Commands};
381-
});
382-
383-
*OutDiags = DiagConsumer.getDiagnosticSet();
384-
385-
return Result;
386-
}
387-
388347
namespace {
389348

390349
struct DependencyScannerWorkerScanSettings {
@@ -438,8 +397,18 @@ struct CStringsManager {
438397

439398
struct DependencyGraph {
440399
TranslationUnitDeps TUDeps;
441-
CXDiagnosticSetDiagnosticConsumer DiagConsumer;
400+
SmallString<256> SerialDiagBuf;
442401
CStringsManager StrMgr{};
402+
403+
CXDiagnosticSet getDiagnosticSet() const {
404+
CXLoadDiag_Error Error;
405+
CXString ErrorString;
406+
CXDiagnosticSet DiagSet = loadCXDiagnosticsFromBuffer(
407+
llvm::MemoryBufferRef(SerialDiagBuf, "<diags>"), &Error, &ErrorString);
408+
assert(Error == CXLoadDiag_None);
409+
clang_disposeString(ErrorString);
410+
return DiagSet;
411+
}
443412
};
444413

445414
struct DependencyGraphModule {
@@ -497,9 +466,18 @@ enum CXErrorCode clang_experimental_DependencyScannerWorker_getDepGraph(
497466

498467
DependencyGraph *DepGraph = new DependencyGraph();
499468

469+
// We capture diagnostics as a serialized diagnostics buffer, so that we don't
470+
// need to keep a valid SourceManager in order to access diagnostic locations.
471+
auto DiagOpts = llvm::makeIntrusiveRefCnt<DiagnosticOptions>();
472+
auto DiagOS =
473+
std::make_unique<llvm::raw_svector_ostream>(DepGraph->SerialDiagBuf);
474+
std::unique_ptr<DiagnosticConsumer> SerialDiagConsumer =
475+
serialized_diags::create("<diagnostics>", DiagOpts.get(),
476+
/*MergeChildRecords=*/false, std::move(DiagOS));
477+
500478
CXErrorCode Result = getFileDependencies(
501479
W, argc, argv, WorkingDirectory, /*MDC=*/nullptr, /*MDCContext=*/nullptr,
502-
/*Error=*/nullptr, &DepGraph->DiagConsumer, LookupOutputs,
480+
/*Error=*/nullptr, SerialDiagConsumer.get(), LookupOutputs,
503481
ModuleName ? Optional<StringRef>(ModuleName) : std::nullopt,
504482
[&](TranslationUnitDeps TU) { DepGraph->TUDeps = std::move(TU); });
505483

@@ -615,7 +593,7 @@ const char *clang_experimental_DepGraph_getTUContextHash(CXDepGraph Graph) {
615593
}
616594

617595
CXDiagnosticSet clang_experimental_DepGraph_getDiagnostics(CXDepGraph Graph) {
618-
return unwrap(Graph)->DiagConsumer.getDiagnosticSet();
596+
return unwrap(Graph)->getDiagnosticSet();
619597
}
620598

621599
static std::string lookupModuleOutput(const ModuleID &ID, ModuleOutputKind MOK,

‎clang/tools/libclang/CXLoadedDiagnostic.cpp

Lines changed: 38 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -249,31 +249,48 @@ class DiagLoader : serialized_diags::SerializedDiagnosticReader {
249249
}
250250

251251
CXDiagnosticSet load(const char *file);
252+
CXDiagnosticSet load(llvm::MemoryBufferRef Buffer);
253+
254+
CXDiagnosticSet reportError(std::error_code EC);
252255
};
253256
} // end anonymous namespace
254257

255258
CXDiagnosticSet DiagLoader::load(const char *file) {
256259
TopDiags = std::make_unique<CXLoadedDiagnosticSetImpl>();
257260

258261
std::error_code EC = readDiagnostics(file);
259-
if (EC) {
260-
switch (EC.value()) {
261-
case static_cast<int>(serialized_diags::SDError::HandlerFailed):
262-
// We've already reported the problem.
263-
break;
264-
case static_cast<int>(serialized_diags::SDError::CouldNotLoad):
265-
reportBad(CXLoadDiag_CannotLoad, EC.message());
266-
break;
267-
default:
268-
reportInvalidFile(EC.message());
269-
break;
270-
}
271-
return nullptr;
272-
}
262+
if (EC)
263+
return reportError(EC);
264+
265+
return (CXDiagnosticSet)TopDiags.release();
266+
}
267+
268+
CXDiagnosticSet DiagLoader::load(llvm::MemoryBufferRef Buffer) {
269+
TopDiags = std::make_unique<CXLoadedDiagnosticSetImpl>();
270+
271+
std::error_code EC = readDiagnostics(Buffer);
272+
if (EC)
273+
return reportError(EC);
273274

274275
return (CXDiagnosticSet)TopDiags.release();
275276
}
276277

278+
CXDiagnosticSet DiagLoader::reportError(std::error_code EC) {
279+
assert(EC);
280+
switch (EC.value()) {
281+
case static_cast<int>(serialized_diags::SDError::HandlerFailed):
282+
// We've already reported the problem.
283+
break;
284+
case static_cast<int>(serialized_diags::SDError::CouldNotLoad):
285+
reportBad(CXLoadDiag_CannotLoad, EC.message());
286+
break;
287+
default:
288+
reportInvalidFile(EC.message());
289+
break;
290+
}
291+
return nullptr;
292+
}
293+
277294
std::error_code
278295
DiagLoader::readLocation(const serialized_diags::Location &SDLoc,
279296
CXLoadedDiagnostic::Location &LoadedLoc) {
@@ -420,3 +437,10 @@ CXDiagnosticSet clang_loadDiagnostics(const char *file,
420437
DiagLoader L(error, errorString);
421438
return L.load(file);
422439
}
440+
441+
CXDiagnosticSet clang::loadCXDiagnosticsFromBuffer(llvm::MemoryBufferRef buffer,
442+
enum CXLoadDiag_Error *error,
443+
CXString *errorString) {
444+
DiagLoader L(error, errorString);
445+
return L.load(buffer);
446+
}

‎clang/tools/libclang/CXLoadedDiagnostic.h

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@
1919
#include "clang/Basic/LLVM.h"
2020
#include <vector>
2121

22+
namespace llvm {
23+
class MemoryBufferRef;
24+
}
25+
2226
namespace clang {
2327
class CXLoadedDiagnostic : public CXDiagnosticImpl {
2428
public:
@@ -88,6 +92,13 @@ class CXLoadedDiagnostic : public CXDiagnosticImpl {
8892
unsigned severity;
8993
unsigned category;
9094
};
91-
}
95+
96+
/// Read a serialized diagnostics \p buffer and create a \c CXDiagnosticSet
97+
/// object for the loaded diagnostics.
98+
CXDiagnosticSet loadCXDiagnosticsFromBuffer(llvm::MemoryBufferRef buffer,
99+
enum CXLoadDiag_Error *error,
100+
CXString *errorString);
101+
102+
} // namespace clang
92103

93104
#endif

‎clang/tools/libclang/libclang.map

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -490,7 +490,6 @@ LLVM_16 {
490490
clang_experimental_DependencyScannerServiceOptions_setDependencyMode;
491491
clang_experimental_DependencyScannerServiceOptions_setObjectStore;
492492
clang_experimental_DependencyScannerWorker_getDepGraph;
493-
clang_experimental_DependencyScannerWorker_getFileDependencies_v5;
494493
clang_experimental_DependencyScannerWorkerScanSettings_create;
495494
clang_experimental_DependencyScannerWorkerScanSettings_dispose;
496495
clang_experimental_DepGraph_dispose;

0 commit comments

Comments
 (0)