Skip to content

Commit 4e246df

Browse files
[Caching] Fix stack-use-after-scope in CachedDiagnostics
Fix a stack-use-after-scope in the CachedDiagnostics when the DiagnosticInfo entry that is getting deserialized has ChildDiagnostics. There are some fields in the DiagnosticInfo are not owned by the DiagnosticsInfo itself. While the callback during the deserialization ensures those fields are still alive for the top level DiagnosticInfo when it gets used, it is not true for any child diagnostics info inside. rdar://123846525
1 parent 85ea5ff commit 4e246df

File tree

2 files changed

+52
-24
lines changed

2 files changed

+52
-24
lines changed

lib/Frontend/CachedDiagnostics.cpp

Lines changed: 32 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -159,8 +159,14 @@ struct DiagnosticSerializer {
159159
llvm::Expected<DiagnosticInfo::FixIt>
160160
deserializeFixIt(const SerializedFixIt &);
161161

162+
// Stores the temporary data from deserialization.
163+
struct DiagnosticStorage {
164+
SmallVector<DiagnosticInfo, 2> ChildDiag;
165+
SmallVector<CharSourceRange, 2> Ranges;
166+
SmallVector<DiagnosticInfo::FixIt, 2> FixIts;
167+
};
162168
llvm::Error deserializeDiagnosticInfo(const SerializedDiagnosticInfo &,
163-
ReplayFunc);
169+
DiagnosticStorage &, ReplayFunc);
164170

165171
// Deserialize File and return the bufferID in serializing SourceManager.
166172
unsigned deserializeFile(const SerializedFile &File);
@@ -288,8 +294,10 @@ void DiagnosticSerializer::handleDiagnostic(SourceManager &SM,
288294
const DiagnosticInfo &Info,
289295
ReplayFunc Fn) {
290296
DiagInfos.emplace_back(convertDiagnosticInfo(SM, Info));
291-
if (Fn)
292-
cantFail(deserializeDiagnosticInfo(DiagInfos.back(), Fn));
297+
if (Fn) {
298+
DiagnosticStorage Storage;
299+
cantFail(deserializeDiagnosticInfo(DiagInfos.back(), Storage, Fn));
300+
}
293301
}
294302

295303
unsigned DiagnosticSerializer::getFileIDFromBufferID(SourceManager &SM,
@@ -537,7 +545,8 @@ llvm::Error DiagnosticSerializer::deserializeGeneratedFileInfo(
537545
}
538546

539547
llvm::Error DiagnosticSerializer::deserializeDiagnosticInfo(
540-
const SerializedDiagnosticInfo &Info, ReplayFunc callback) {
548+
const SerializedDiagnosticInfo &Info, DiagnosticStorage &Storage,
549+
ReplayFunc callback) {
541550
DiagID ID = (DiagID)Info.ID;
542551
auto Loc = deserializeSourceLoc(Info.Loc);
543552
if (!Loc)
@@ -546,33 +555,32 @@ llvm::Error DiagnosticSerializer::deserializeDiagnosticInfo(
546555
auto BICD = deserializeSourceLoc(Info.BufferIndirectlyCausingDiagnostic);
547556
if (!BICD)
548557
return BICD.takeError();
549-
SmallVector<DiagnosticInfo, 2> ChildDiag;
558+
559+
llvm::TinyPtrVector<DiagnosticInfo *> ChildDiagPtrs;
550560
for (auto &CD : Info.ChildDiagnosticInfo) {
551-
auto E = deserializeDiagnosticInfo(CD, [&](const DiagnosticInfo &Info) {
552-
ChildDiag.emplace_back(Info);
553-
return llvm::Error::success();
554-
});
561+
auto E =
562+
deserializeDiagnosticInfo(CD, Storage, [&](const DiagnosticInfo &Info) {
563+
Storage.ChildDiag.emplace_back(Info);
564+
ChildDiagPtrs.push_back(&Storage.ChildDiag.back());
565+
return llvm::Error::success();
566+
});
555567
if (E)
556568
return E;
557569
}
558-
llvm::TinyPtrVector<DiagnosticInfo*> ChildDiagPtrs;
559-
llvm::for_each(ChildDiag, [&ChildDiagPtrs](DiagnosticInfo &I) {
560-
ChildDiagPtrs.push_back(&I);
561-
});
562-
SmallVector<CharSourceRange, 2> Ranges;
563570
for (auto &R : Info.Ranges) {
564571
auto Range = deserializeSourceRange(R);
565572
if (!Range)
566573
return Range.takeError();
567-
Ranges.emplace_back(*Range);
574+
Storage.Ranges.emplace_back(*Range);
568575
}
569-
SmallVector<DiagnosticInfo::FixIt, 2> FixIts;
576+
auto Ranges = ArrayRef(Storage.Ranges).take_back(Info.Ranges.size());
570577
for (auto &F : Info.FixIts) {
571578
auto FixIt = deserializeFixIt(F);
572579
if (!FixIt)
573580
return FixIt.takeError();
574-
FixIts.emplace_back(*FixIt);
581+
Storage.FixIts.emplace_back(*FixIt);
575582
}
583+
auto FixIts = ArrayRef(Storage.FixIts).take_back(Info.FixIts.size());
576584

577585
DiagnosticInfo DeserializedInfo{ID,
578586
*Loc,
@@ -623,11 +631,13 @@ llvm::Error DiagnosticSerializer::doEmitFromCached(llvm::StringRef Buffer,
623631
}
624632

625633
for (auto &Info : DiagInfos) {
626-
auto E = deserializeDiagnosticInfo(Info, [&](const DiagnosticInfo &Info) {
627-
for (auto *Diag : Diags.getConsumers())
628-
Diag->handleDiagnostic(SrcMgr, Info);
629-
return llvm::Error::success();
630-
});
634+
DiagnosticStorage Storage;
635+
auto E = deserializeDiagnosticInfo(Info, Storage,
636+
[&](const DiagnosticInfo &Info) {
637+
for (auto *Diag : Diags.getConsumers())
638+
Diag->handleDiagnostic(SrcMgr, Info);
639+
return llvm::Error::success();
640+
});
631641
if (E)
632642
return E;
633643
}

test/CAS/cached_diagnostics.swift

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,24 +4,33 @@
44
// RUN: %empty-directory(%t)
55

66
// RUN: %target-swift-frontend -scan-dependencies -module-name Test -O -import-objc-header %S/Inputs/objc.h \
7-
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import -parse-stdlib \
7+
// RUN: -disable-implicit-string-processing-module-import -disable-implicit-concurrency-module-import \
88
// RUN: %s -o %t/deps.json -cache-compile-job -cas-path %t/cas
99

10+
// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json clang:SwiftShims > %t/shim.cmd
11+
// RUN: %swift_frontend_plain @%t/shim.cmd
12+
// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json Swift > %t/swift.cmd
13+
// RUN: %swift_frontend_plain @%t/swift.cmd
14+
1015
// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json bridgingHeader | tail -n +2 > %t/header.cmd
1116
// RUN: %target-swift-frontend @%t/header.cmd -disable-implicit-swift-modules %S/Inputs/objc.h -O -o %t/objc.pch 2>&1 | %FileCheck %s -check-prefix CHECK-BRIDGE
1217
// RUN: %cache-tool -cas-path %t/cas -cache-tool-action print-output-keys -- \
1318
// RUN: %target-swift-frontend @%t/header.cmd -disable-implicit-swift-modules %S/Inputs/objc.h -O -o %t/objc.pch > %t/keys.json
1419
// RUN: %{python} %S/Inputs/ExtractOutputKey.py %t/keys.json %S/Inputs/objc.h > %t/key
1520

21+
// RUN: %{python} %S/Inputs/GenerateExplicitModuleMap.py %t/deps.json > %t/map.json
22+
// RUN: llvm-cas --cas %t/cas --make-blob --data %t/map.json > %t/map.casid
23+
1624
// RUN: %{python} %S/Inputs/BuildCommandExtractor.py %t/deps.json Test > %t/MyApp.cmd
1725
// RUN: echo "\"-disable-implicit-string-processing-module-import\"" >> %t/MyApp.cmd
1826
// RUN: echo "\"-disable-implicit-concurrency-module-import\"" >> %t/MyApp.cmd
1927
// RUN: echo "\"-disable-implicit-swift-modules\"" >> %t/MyApp.cmd
20-
// RUN: echo "\"-parse-stdlib\"" >> %t/MyApp.cmd
2128
// RUN: echo "\"-import-objc-header\"" >> %t/MyApp.cmd
2229
// RUN: echo "\"%t/objc.pch\"" >> %t/MyApp.cmd
2330
// RUN: echo "\"-bridging-header-pch-key\"" >> %t/MyApp.cmd
2431
// RUN: echo "\"@%t/key\"" >> %t/MyApp.cmd
32+
// RUN: echo "\"-explicit-swift-module-map-file\"" >> %t/MyApp.cmd
33+
// RUN: echo "\"@%t/map.casid\"" >> %t/MyApp.cmd
2534

2635
// RUN: %target-swift-frontend -cache-compile-job -module-name Test -O -cas-path %t/cas @%t/MyApp.cmd %s \
2736
// RUN: -emit-module -o %t/test.swiftmodule 2>&1 | %FileCheck %s
@@ -32,6 +41,15 @@
3241

3342
#warning("this is a warning") // expected-warning {{this is a warning}}
3443

44+
enum MyEnum {
45+
case kind
46+
case none
47+
}
48+
49+
let _ : MyEnum? = .none // expected-warning {{assuming you mean 'Optional<MyEnum>.none'; did you mean 'MyEnum.none' instead?}}
50+
// expected-note@-1 {{explicitly specify 'Optional' to silence this warning}} {{19-19=Optional}}
51+
// expected-note@-2 {{use 'MyEnum.none' instead}} {{19-19=MyEnum}}
52+
3553
// CHECK-BRIDGE: warning: warning in bridging header
3654
// CHECK: warning: this is a warning
3755

0 commit comments

Comments
 (0)