Skip to content

Commit fe90657

Browse files
committed
[Basic] Avoid reentrant stat collection
Currently `UnifiedStatsReporter::flushTracesAndProfiles` can kick off requests when computing the source ranges for collected entities, which will try to record additional stats about the requests. This currently happens to work without issue, but swiftlang#29289 bumped the counters down slightly such that the vector storing the stats now performs a re-allocation when we do a reentrant stat entry. This then caused a use-after-free as we try to continue iterating over the old buffer. Fix this issue by refusing to record any new stats while we're flushing out the ones we've already recorded.
1 parent fa15449 commit fe90657

File tree

2 files changed

+18
-1
lines changed

2 files changed

+18
-1
lines changed

include/swift/Basic/Statistic.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ class UnifiedStatsReporter {
162162
std::unique_ptr<StatsProfilers> EventProfilers;
163163
std::unique_ptr<StatsProfilers> EntityProfilers;
164164

165+
/// Whether we are currently flushing statistics and should not therefore
166+
/// record any additional stats until we've finished.
167+
bool IsFlushingTracesAndProfiles;
168+
165169
void publishAlwaysOnStatsToLLVM();
166170
void printAlwaysOnStatsAndTimers(raw_ostream &OS);
167171

lib/Basic/Statistic.cpp

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
#include "llvm/Support/Path.h"
2626
#include "llvm/Support/Process.h"
2727
#include "llvm/Support/raw_ostream.h"
28+
#include "llvm/Support/SaveAndRestore.h"
2829
#include <chrono>
2930
#include <limits>
3031

@@ -348,7 +349,8 @@ UnifiedStatsReporter::UnifiedStatsReporter(StringRef ProgramName,
348349
ProgramName, "Running Program")),
349350
SourceMgr(SM),
350351
ClangSourceMgr(CSM),
351-
RecursiveTimers(llvm::make_unique<RecursionSafeTimers>())
352+
RecursiveTimers(llvm::make_unique<RecursionSafeTimers>()),
353+
IsFlushingTracesAndProfiles(false)
352354
{
353355
path::append(StatsFilename, makeStatsFileName(ProgramName, AuxName));
354356
path::append(TraceFilename, makeTraceFileName(ProgramName, AuxName));
@@ -557,6 +559,13 @@ UnifiedStatsReporter::saveAnyFrontendStatsEvents(
557559
bool IsEntry)
558560
{
559561
assert(MainThreadID == std::this_thread::get_id());
562+
563+
// Don't record any new stats if we're currently flushing the ones we've
564+
// already recorded. This can happen when requests get kicked off when
565+
// computing source ranges.
566+
if (IsFlushingTracesAndProfiles)
567+
return;
568+
560569
// First make a note in the recursion-safe timers; these
561570
// are active anytime UnifiedStatsReporter is active.
562571
if (IsEntry) {
@@ -711,6 +720,10 @@ UnifiedStatsReporter::~UnifiedStatsReporter()
711720

712721
void
713722
UnifiedStatsReporter::flushTracesAndProfiles() {
723+
// Note that we're currently flushing statistics and shouldn't record any
724+
// more until we've finished.
725+
llvm::SaveAndRestore<bool> flushing(IsFlushingTracesAndProfiles, true);
726+
714727
if (FrontendStatsEvents && SourceMgr) {
715728
std::error_code EC;
716729
raw_fd_ostream tstream(TraceFilename, EC, fs::F_Append | fs::F_Text);

0 commit comments

Comments
 (0)