Skip to content

Commit e271973

Browse files
committed
Fix FunctionTest memory corruption in SwiftCompilerSouces.
A FunctionTest created in Swift source has no persistent storage address. This results in sporadic crashes when running unit tests. Fix the FunctionTest registry to store the value of the test. Eventually, we could probably rig something up with @_rawLayout, but it makes more sense for FunctionTest to be a value type anyway.
1 parent 5e91239 commit e271973

File tree

3 files changed

+16
-21
lines changed

3 files changed

+16
-21
lines changed

include/swift/SIL/Test.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,9 @@ class TestRunner;
120120
/// Tests are instantiated once at program launch. At that time, they are
121121
/// stored in a registry name -> test. When an specify_test instruction
122122
/// naming a particular test is processed, that test is run.
123+
///
124+
/// This is a value type because we have no persistent location in which to
125+
/// store SwiftCompilerSources tests.
123126
class FunctionTest final {
124127
public:
125128
/// Wraps a test lambda.
@@ -190,7 +193,7 @@ class FunctionTest final {
190193
SILFunctionTransform &pass, Dependencies &dependencies);
191194

192195
/// Retrieve the test with named \p name from the global registry.
193-
static FunctionTest *get(StringRef name);
196+
static FunctionTest get(StringRef name);
194197

195198
/// The instance of the TestRunner pass currently running this test. Only
196199
/// non-null when FunctionTest::run is executing.

lib/SIL/Utils/Test.cpp

Lines changed: 10 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ using namespace swift::test;
2525
namespace {
2626

2727
class Registry {
28-
StringMap<FunctionTest *> registeredTests;
28+
StringMap<FunctionTest> registeredTests;
2929
SwiftNativeFunctionTestThunk thunk;
3030

3131
public:
@@ -34,7 +34,7 @@ class Registry {
3434
return registry;
3535
}
3636

37-
void registerFunctionTest(FunctionTest *test, StringRef name) {
37+
void registerFunctionTest(FunctionTest test, StringRef name) {
3838
auto inserted = registeredTests.insert({name, test}).second;
3939
assert(inserted);
4040
(void)inserted;
@@ -46,22 +46,21 @@ class Registry {
4646

4747
SwiftNativeFunctionTestThunk getFunctionTestThunk() { return thunk; }
4848

49-
FunctionTest *getFunctionTest(StringRef name) {
50-
// Avoid creating a new entry here.
51-
auto *res = registeredTests.lookup(name);
52-
if (!res) {
49+
FunctionTest getFunctionTest(StringRef name) {
50+
auto iter = registeredTests.find(name);
51+
if (iter == registeredTests.end()) {
5352
llvm::errs() << "Found no test named " << name << "!\n";
5453
print(llvm::errs());
5554
}
56-
return res;
55+
return iter->getValue();
5756
}
5857

5958
void print(raw_ostream &OS) const {
6059
OS << "test::Registry(" << this << ") with " << registeredTests.size()
6160
<< " entries: {{\n";
6261
for (auto &stringMapEntry : registeredTests) {
6362
OS << "\t" << stringMapEntry.getKey() << " -> "
64-
<< stringMapEntry.getValue() << "\n";
63+
<< &stringMapEntry.getValue() << "\n";
6564
}
6665
OS << "}} test::Registry(" << this << ")\n";
6766
}
@@ -78,21 +77,18 @@ void registerFunctionTestThunk(SwiftNativeFunctionTestThunk thunk) {
7877
FunctionTest::FunctionTest(StringRef name, Invocation invocation)
7978
: invocation(invocation), pass(nullptr), function(nullptr),
8079
dependencies(nullptr) {
81-
Registry::get().registerFunctionTest(this, name);
80+
Registry::get().registerFunctionTest(*this, name);
8281
}
8382
FunctionTest::FunctionTest(StringRef name, NativeSwiftInvocation invocation)
8483
: invocation(invocation), pass(nullptr), function(nullptr),
8584
dependencies(nullptr) {}
8685

8786
void FunctionTest::createNativeSwiftFunctionTest(
8887
StringRef name, NativeSwiftInvocation invocation) {
89-
/// Statically allocate the tests to avoid triggering LSAN's "leak" detection.
90-
static SmallVector<FunctionTest, 4> tests;
91-
auto &test = tests.emplace_back(name, invocation);
92-
Registry::get().registerFunctionTest(&test, name);
88+
Registry::get().registerFunctionTest({name, invocation}, name);
9389
}
9490

95-
FunctionTest *FunctionTest::get(StringRef name) {
91+
FunctionTest FunctionTest::get(StringRef name) {
9692
return Registry::get().getFunctionTest(name);
9793
}
9894

lib/SILOptimizer/UtilityPasses/TestRunner.cpp

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -98,14 +98,10 @@ void TestRunner::printTestLifetime(bool begin, unsigned testIndex,
9898
}
9999

100100
void TestRunner::runTest(StringRef name, Arguments &arguments) {
101-
auto *test = FunctionTest::get(name);
102-
if (!test) {
103-
llvm::outs() << "No test named: " << name << "\n";
104-
assert(false && "Invalid test name");
105-
}
101+
FunctionTest test = FunctionTest::get(name);
106102
auto *function = getFunction();
107103
FunctionTestDependenciesImpl dependencies(this, function);
108-
test->run(*function, arguments, *this, dependencies);
104+
test.run(*function, arguments, *this, dependencies);
109105
}
110106

111107
void TestRunner::run() {

0 commit comments

Comments
 (0)