38be53aa04
This PR fixes a use-after-free error that happens when `DistinctAttr` instances are created within a `PassManager` running with crash recovery enabled. The root cause is that `DistinctAttr` storage is allocated in a thread_local allocator, which is destroyed when the crash recovery thread joins, invalidating the storage. Moreover, even without crash reproduction disabling multithreading on the context will destroy the context's thread pool, and in turn delete the threadlocal storage. This means a call to `ctx->disableMulthithreading()` breaks the IR. This PR replaces the thread local allocator with a synchronised allocator that's shared between threads. This persists the lifetime of allocated DistinctAttr storage instances to the lifetime of the context. ### Problem Details: The `DistinctAttributeAllocator` uses a `ThreadLocalCache<BumpPtrAllocator>` for lock-free allocation of `DistinctAttr` storage in a multithreaded context. The issue occurs when a `PassManager` is run with crash recovery (`runWithCrashRecovery`), the pass pipeline is executed on a temporary thread spawned by `llvm::CrashRecoveryContext`. Any `DistinctAttr`s created during this execution have their storage allocated in the thread_local cache of this temporary thread. When the thread joins, the thread_local storage is destroyed, freeing the `DistinctAttr`s' memory. If this attribute is accessed later, e.g. when printing, it results in a use-after-free. As mentioned previously, this is also seen after creating some `DistinctAttr`s and then calling `ctx->disableMulthithreading()`. ### Solution `DistinctAttrStorageAllocator` uses a synchronised, shared allocator instead of one wrapped in a `ThreadLocalCache`. The former is what stores the allocator in transient thread_local storage. ### Testing: A C++ unit test has been added to validate this fix. (I was previously reproducing this failure with `mlir-opt` but I can no longer do so and I am unsure why.) ----- Note: This is a 2nd attempt at my previous PR https://github.com/llvm/llvm-project/pull/128566 that was reverted in https://github.com/llvm/llvm-project/pull/133000. I believe I've addressed the TSAN and race condition concerns.
46 lines
1.6 KiB
C++
46 lines
1.6 KiB
C++
//=== DistinctAttributeAllocatorTest.cpp - DistinctAttr storage alloc test ===//
|
|
//
|
|
// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
|
|
// See https://llvm.org/LICENSE.txt for license information.
|
|
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
|
|
//
|
|
//===----------------------------------------------------------------------===//
|
|
|
|
#include "gtest/gtest.h"
|
|
|
|
#include "mlir/IR/Builders.h"
|
|
#include "mlir/IR/BuiltinAttributes.h"
|
|
#include "mlir/IR/MLIRContext.h"
|
|
#include "llvm/Support/CrashRecoveryContext.h"
|
|
#include <thread>
|
|
|
|
using namespace mlir;
|
|
|
|
//
|
|
// Test that a DistinctAttr that is created on a separate thread does
|
|
// not have its storage deleted when the thread joins.
|
|
//
|
|
TEST(DistinctAttributeAllocatorTest, TestAttributeWellFormedAfterThreadJoin) {
|
|
MLIRContext ctx;
|
|
OpBuilder builder(&ctx);
|
|
DistinctAttr attr;
|
|
|
|
std::thread t([&ctx, &attr]() {
|
|
attr = DistinctAttr::create(UnitAttr::get(&ctx));
|
|
ASSERT_TRUE(attr);
|
|
});
|
|
t.join();
|
|
|
|
// If the attribute storage got deleted after the thread joins (which we don't
|
|
// want) then trying to access it triggers an assert in Debug mode, and a
|
|
// crash otherwise. Run this in a CrashRecoveryContext to avoid bringing down
|
|
// the whole test suite if this test fails. Additionally, MSAN and/or TSAN
|
|
// should raise failures here if the attribute storage was deleted.
|
|
llvm::CrashRecoveryContext crc;
|
|
EXPECT_TRUE(crc.RunSafely([attr]() { (void)attr.getAbstractAttribute(); }));
|
|
EXPECT_TRUE(
|
|
crc.RunSafely([attr]() { (void)*cast<Attribute>(attr).getImpl(); }));
|
|
|
|
ASSERT_TRUE(attr);
|
|
}
|