Skip to content

Commit 39a2186

Browse files
anutosh491llvmbot
anutosh491
authored and
llvmbot
committed
[clang-repl] : Fix clang-repl crash with --cuda flag (llvm#136404)
`clang-repl --cuda` was previously crashing with a segmentation fault, instead of reporting a clean error ``` (base) anutosh491@Anutoshs-MacBook-Air bin % ./clang-repl --cuda #0 0x0000000111da4fbc llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x150fbc) #1 0x0000000111da31dc llvm::sys::RunSignalHandlers() (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x14f1dc) #2 0x0000000111da5628 SignalHandler(int) (/opt/local/libexec/llvm-20/lib/libLLVM.dylib+0x151628) #3 0x000000019b242de4 (/usr/lib/system/libsystem_platform.dylib+0x180482de4) #4 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0) #5 0x0000000107f638d0 clang::IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, clang::CompilerInstance&, llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem>, llvm::Error&, std::__1::list<clang::PartialTranslationUnit, std::__1::allocator<clang::PartialTranslationUnit>> const&) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x216b8d0) #6 0x0000000107f6bac8 clang::Interpreter::createWithCUDA(std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>, std::__1::unique_ptr<clang::CompilerInstance, std::__1::default_delete<clang::CompilerInstance>>) (/opt/local/libexec/llvm-20/lib/libclang-cpp.dylib+0x2173ac8) #7 0x000000010206f8a8 main (/opt/local/libexec/llvm-20/bin/clang-repl+0x1000038a8) #8 0x000000019ae8c274 Segmentation fault: 11 ``` The underlying issue was that the `DeviceCompilerInstance` (used for device-side CUDA compilation) was never initialized with a `Sema`, which is required before constructing the `IncrementalCUDADeviceParser`. https://github.com/llvm/llvm-project/blob/89687e6f383b742a3c6542dc673a84d9f82d02de/clang/lib/Interpreter/DeviceOffload.cpp#L32 https://github.com/llvm/llvm-project/blob/89687e6f383b742a3c6542dc673a84d9f82d02de/clang/lib/Interpreter/IncrementalParser.cpp#L31 Unlike the host-side `CompilerInstance` which runs `ExecuteAction` inside the Interpreter constructor (thereby setting up Sema), the device-side CI was passed into the parser uninitialized, leading to an assertion or crash when accessing its internals. To fix this, I refactored the `Interpreter::create` method to include an optional `DeviceCI` parameter. If provided, we know we need to take care of this instance too. Only then do we construct the `IncrementalCUDADeviceParser`. (cherry picked from commit 21fb19f)
1 parent 182e8b7 commit 39a2186

File tree

4 files changed

+68
-51
lines changed

4 files changed

+68
-51
lines changed

‎clang/include/clang/Interpreter/Interpreter.h

+9-4
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ class CXXRecordDecl;
4141
class Decl;
4242
class IncrementalExecutor;
4343
class IncrementalParser;
44+
class IncrementalCUDADeviceParser;
4445

4546
/// Create a pre-configured \c CompilerInstance for incremental processing.
4647
class IncrementalCompilerBuilder {
@@ -93,7 +94,10 @@ class Interpreter {
9394
std::unique_ptr<IncrementalExecutor> IncrExecutor;
9495

9596
// An optional parser for CUDA offloading
96-
std::unique_ptr<IncrementalParser> DeviceParser;
97+
std::unique_ptr<IncrementalCUDADeviceParser> DeviceParser;
98+
99+
// An optional action for CUDA offloading
100+
std::unique_ptr<IncrementalAction> DeviceAct;
97101

98102
/// List containing information about each incrementally parsed piece of code.
99103
std::list<PartialTranslationUnit> PTUs;
@@ -175,10 +179,11 @@ class Interpreter {
175179
llvm::Expected<Expr *> ExtractValueFromExpr(Expr *E);
176180
llvm::Expected<llvm::orc::ExecutorAddr> CompileDtorCall(CXXRecordDecl *CXXRD);
177181

178-
CodeGenerator *getCodeGen() const;
179-
std::unique_ptr<llvm::Module> GenModule();
182+
CodeGenerator *getCodeGen(IncrementalAction *Action = nullptr) const;
183+
std::unique_ptr<llvm::Module> GenModule(IncrementalAction *Action = nullptr);
180184
PartialTranslationUnit &RegisterPTU(TranslationUnitDecl *TU,
181-
std::unique_ptr<llvm::Module> M = {});
185+
std::unique_ptr<llvm::Module> M = {},
186+
IncrementalAction *Action = nullptr);
182187

183188
// A cache for the compiled destructors used to for de-allocation of managed
184189
// clang::Values.

‎clang/lib/Interpreter/DeviceOffload.cpp

+15-30
Original file line numberDiff line numberDiff line change
@@ -31,45 +31,17 @@ IncrementalCUDADeviceParser::IncrementalCUDADeviceParser(
3131
llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs)
3232
: IncrementalParser(*DeviceInstance, Err), PTUs(PTUs), VFS(FS),
3333
CodeGenOpts(HostInstance.getCodeGenOpts()),
34-
TargetOpts(HostInstance.getTargetOpts()) {
34+
TargetOpts(DeviceInstance->getTargetOpts()) {
3535
if (Err)
3636
return;
37-
DeviceCI = std::move(DeviceInstance);
3837
StringRef Arch = TargetOpts.CPU;
3938
if (!Arch.starts_with("sm_") || Arch.substr(3).getAsInteger(10, SMVersion)) {
4039
Err = llvm::joinErrors(std::move(Err), llvm::make_error<llvm::StringError>(
4140
"Invalid CUDA architecture",
4241
llvm::inconvertibleErrorCode()));
4342
return;
4443
}
45-
}
46-
47-
llvm::Expected<TranslationUnitDecl *>
48-
IncrementalCUDADeviceParser::Parse(llvm::StringRef Input) {
49-
auto PTU = IncrementalParser::Parse(Input);
50-
if (!PTU)
51-
return PTU.takeError();
52-
53-
auto PTX = GeneratePTX();
54-
if (!PTX)
55-
return PTX.takeError();
56-
57-
auto Err = GenerateFatbinary();
58-
if (Err)
59-
return std::move(Err);
60-
61-
std::string FatbinFileName =
62-
"/incr_module_" + std::to_string(PTUs.size()) + ".fatbin";
63-
VFS->addFile(FatbinFileName, 0,
64-
llvm::MemoryBuffer::getMemBuffer(
65-
llvm::StringRef(FatbinContent.data(), FatbinContent.size()),
66-
"", false));
67-
68-
CodeGenOpts.CudaGpuBinaryFileName = FatbinFileName;
69-
70-
FatbinContent.clear();
71-
72-
return PTU;
44+
DeviceCI = std::move(DeviceInstance);
7345
}
7446

7547
llvm::Expected<llvm::StringRef> IncrementalCUDADeviceParser::GeneratePTX() {
@@ -172,6 +144,19 @@ llvm::Error IncrementalCUDADeviceParser::GenerateFatbinary() {
172144

173145
FatbinContent.append(PTXCode.begin(), PTXCode.end());
174146

147+
const PartialTranslationUnit &PTU = PTUs.back();
148+
149+
std::string FatbinFileName = "/" + PTU.TheModule->getName().str() + ".fatbin";
150+
151+
VFS->addFile(FatbinFileName, 0,
152+
llvm::MemoryBuffer::getMemBuffer(
153+
llvm::StringRef(FatbinContent.data(), FatbinContent.size()),
154+
"", false));
155+
156+
CodeGenOpts.CudaGpuBinaryFileName = FatbinFileName;
157+
158+
FatbinContent.clear();
159+
175160
return llvm::Error::success();
176161
}
177162

‎clang/lib/Interpreter/DeviceOffload.h

-2
Original file line numberDiff line numberDiff line change
@@ -33,8 +33,6 @@ class IncrementalCUDADeviceParser : public IncrementalParser {
3333
llvm::IntrusiveRefCntPtr<llvm::vfs::InMemoryFileSystem> VFS,
3434
llvm::Error &Err, const std::list<PartialTranslationUnit> &PTUs);
3535

36-
llvm::Expected<TranslationUnitDecl *> Parse(llvm::StringRef Input) override;
37-
3836
// Generate PTX for the last PTU.
3937
llvm::Expected<llvm::StringRef> GeneratePTX();
4038

‎clang/lib/Interpreter/Interpreter.cpp

+44-15
Original file line numberDiff line numberDiff line change
@@ -480,20 +480,34 @@ Interpreter::createWithCUDA(std::unique_ptr<CompilerInstance> CI,
480480
OverlayVFS->pushOverlay(IMVFS);
481481
CI->createFileManager(OverlayVFS);
482482

483-
auto Interp = Interpreter::create(std::move(CI));
484-
if (auto E = Interp.takeError())
485-
return std::move(E);
483+
llvm::Expected<std::unique_ptr<Interpreter>> InterpOrErr =
484+
Interpreter::create(std::move(CI));
485+
if (!InterpOrErr)
486+
return InterpOrErr;
487+
488+
std::unique_ptr<Interpreter> Interp = std::move(*InterpOrErr);
486489

487490
llvm::Error Err = llvm::Error::success();
488-
auto DeviceParser = std::make_unique<IncrementalCUDADeviceParser>(
489-
std::move(DCI), *(*Interp)->getCompilerInstance(), IMVFS, Err,
490-
(*Interp)->PTUs);
491+
llvm::LLVMContext &LLVMCtx = *Interp->TSCtx->getContext();
492+
493+
auto DeviceAct =
494+
std::make_unique<IncrementalAction>(*DCI, LLVMCtx, Err, *Interp);
495+
491496
if (Err)
492497
return std::move(Err);
493498

494-
(*Interp)->DeviceParser = std::move(DeviceParser);
499+
Interp->DeviceAct = std::move(DeviceAct);
500+
501+
DCI->ExecuteAction(*Interp->DeviceAct);
502+
503+
auto DeviceParser = std::make_unique<IncrementalCUDADeviceParser>(
504+
std::move(DCI), *Interp->getCompilerInstance(), IMVFS, Err, Interp->PTUs);
505+
506+
if (Err)
507+
return std::move(Err);
495508

496-
return Interp;
509+
Interp->DeviceParser = std::move(DeviceParser);
510+
return std::move(Interp);
497511
}
498512

499513
const CompilerInstance *Interpreter::getCompilerInstance() const {
@@ -531,15 +545,17 @@ size_t Interpreter::getEffectivePTUSize() const {
531545

532546
PartialTranslationUnit &
533547
Interpreter::RegisterPTU(TranslationUnitDecl *TU,
534-
std::unique_ptr<llvm::Module> M /*={}*/) {
548+
std::unique_ptr<llvm::Module> M /*={}*/,
549+
IncrementalAction *Action) {
535550
PTUs.emplace_back(PartialTranslationUnit());
536551
PartialTranslationUnit &LastPTU = PTUs.back();
537552
LastPTU.TUPart = TU;
538553

539554
if (!M)
540-
M = GenModule();
555+
M = GenModule(Action);
541556

542-
assert((!getCodeGen() || M) && "Must have a llvm::Module at this point");
557+
assert((!getCodeGen(Action) || M) &&
558+
"Must have a llvm::Module at this point");
543559

544560
LastPTU.TheModule = std::move(M);
545561
LLVM_DEBUG(llvm::dbgs() << "compile-ptu " << PTUs.size() - 1
@@ -559,6 +575,16 @@ Interpreter::Parse(llvm::StringRef Code) {
559575
llvm::Expected<TranslationUnitDecl *> DeviceTU = DeviceParser->Parse(Code);
560576
if (auto E = DeviceTU.takeError())
561577
return std::move(E);
578+
579+
RegisterPTU(*DeviceTU, nullptr, DeviceAct.get());
580+
581+
llvm::Expected<llvm::StringRef> PTX = DeviceParser->GeneratePTX();
582+
if (!PTX)
583+
return PTX.takeError();
584+
585+
llvm::Error Err = DeviceParser->GenerateFatbinary();
586+
if (Err)
587+
return std::move(Err);
562588
}
563589

564590
// Tell the interpreter sliently ignore unused expressions since value
@@ -726,9 +752,10 @@ llvm::Error Interpreter::LoadDynamicLibrary(const char *name) {
726752
return llvm::Error::success();
727753
}
728754

729-
std::unique_ptr<llvm::Module> Interpreter::GenModule() {
755+
std::unique_ptr<llvm::Module>
756+
Interpreter::GenModule(IncrementalAction *Action) {
730757
static unsigned ID = 0;
731-
if (CodeGenerator *CG = getCodeGen()) {
758+
if (CodeGenerator *CG = getCodeGen(Action)) {
732759
// Clang's CodeGen is designed to work with a single llvm::Module. In many
733760
// cases for convenience various CodeGen parts have a reference to the
734761
// llvm::Module (TheModule or Module) which does not change when a new
@@ -750,8 +777,10 @@ std::unique_ptr<llvm::Module> Interpreter::GenModule() {
750777
return nullptr;
751778
}
752779

753-
CodeGenerator *Interpreter::getCodeGen() const {
754-
FrontendAction *WrappedAct = Act->getWrapped();
780+
CodeGenerator *Interpreter::getCodeGen(IncrementalAction *Action) const {
781+
if (!Action)
782+
Action = Act.get();
783+
FrontendAction *WrappedAct = Action->getWrapped();
755784
if (!WrappedAct->hasIRSupport())
756785
return nullptr;
757786
return static_cast<CodeGenAction *>(WrappedAct)->getCodeGenerator();

0 commit comments

Comments
 (0)