Skip to content

Commit da50819

Browse files
Fix puts/cf.assert signature conflict crashing JIT tests (#184)
* Fix puts/cf.assert signature conflict crashing JIT tests with --no-default-lib MLIR's cf.assert-to-LLVM lowering reserves "puts" as void(ptr) when it needs to print an assertion message. Our PrintOp lowering separately declared "puts" as i32(ptr) (matching libc), so whichever declaration landed first caused "redefinition of reserved function 'puts' of different type" and left 'cf.assert' unlegalized, crashing any JIT/AOT test that both printed and hit a lowered cf.assert. Declare it as void(ptr) instead since the return value was never used. Re-enables test-jit-00-async-await and test-jit-00-for-await on Windows now that they pass; test-jit-00-for-await-yield stays disabled since it hits a separate async-generator runtime hang. * Fix JIT teardown process to prevent deadlocks and ensure proper flushing of stdio on Windows * Update TODO comment for async generator and for-await-of tests in CMakeLists.txt * Implement check for presplit coroutine in StringConcatOpLowering to handle heap allocation * Add GC root management functions and integrate with JIT memory mapping
1 parent 2467463 commit da50819

6 files changed

Lines changed: 149 additions & 17 deletions

File tree

‎tslang/include/TypeScript/gcwrapper.h‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,10 @@ void _mlir__GC_free(void *ptr);
1919

2020
size_t _mlir__GC_get_heap_size();
2121

22+
void _mlir__GC_add_roots(void *low, void *highPlus1);
23+
24+
void _mlir__GC_remove_roots(void *low, void *highPlus1);
25+
2226
void _mlir__GC_win32_free_heap();
2327

2428
void *_mlir__GC_malloc_explicitly_typed(size_t size, int64_t descr);

‎tslang/lib/TypeScript/LowerToLLVM.cpp‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -158,8 +158,10 @@ class PrintOpLowering : public TsLlvmPattern<mlir_ts::PrintOp>
158158
auto i8PtrType = th.getPtrType();
159159
auto ptrType = th.getPtrType();
160160

161-
// Get a symbol reference to the printf function, inserting it if necessary.
162-
auto putsFuncOp = ch.getOrInsertFunction("puts", th.getFunctionType(rewriter.getI32Type(), ptrType, false));
161+
// Get a symbol reference to the puts function, inserting it if necessary.
162+
// Note: declared as returning void (not the libc i32) to match the signature MLIR's
163+
// cf.assert-to-LLVM lowering expects when it reserves/looks up "puts" for its own use.
164+
auto putsFuncOp = ch.getOrInsertFunction("puts", th.getFunctionType(th.getVoidType(), ptrType, false));
163165

164166
auto strType = mlir_ts::StringType::get(rewriter.getContext());
165167

@@ -518,6 +520,32 @@ class SetStringLengthOpLowering : public TsLlvmPattern<mlir_ts::SetStringLengthO
518520
}
519521
};
520522

523+
// LLVM's CoroSplit cannot spill variable-size allocas into the coroutine frame
524+
// ("Coroutines cannot handle non static allocas yet"), so buffers inside a
525+
// not-yet-split coroutine (async_execute_fn marked by the MLIR async lowering)
526+
// must be heap-allocated instead.
527+
static bool isInsidePresplitCoroutine(mlir::Operation *op)
528+
{
529+
for (auto *parent = op->getParentOp(); parent; parent = parent->getParentOp())
530+
{
531+
if (auto passthrough = parent->getAttrOfType<mlir::ArrayAttr>("passthrough"))
532+
{
533+
for (auto attr : passthrough)
534+
{
535+
if (auto strAttr = dyn_cast<mlir::StringAttr>(attr))
536+
{
537+
if (strAttr.getValue() == "presplitcoroutine")
538+
{
539+
return true;
540+
}
541+
}
542+
}
543+
}
544+
}
545+
546+
return false;
547+
}
548+
521549
class StringConcatOpLowering : public TsLlvmPattern<mlir_ts::StringConcatOp>
522550
{
523551
public:
@@ -552,7 +580,8 @@ class StringConcatOpLowering : public TsLlvmPattern<mlir_ts::StringConcatOp>
552580
size = rewriter.create<LLVM::AddOp>(loc, llvmIndexType, ValueRange{size, size1.getResult()});
553581
}
554582

555-
auto allocInStack = op.getAllocInStack().has_value() && op.getAllocInStack().value();
583+
auto allocInStack = op.getAllocInStack().has_value() && op.getAllocInStack().value()
584+
&& !isInsidePresplitCoroutine(op);
556585

557586
mlir::Value newStringValue = allocInStack ? ch.Alloca(i8PtrTy, size, true)
558587
: ch.MemoryAlloc(size);

‎tslang/lib/TypeScriptRuntime/TypeScriptRuntime.def‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ EXPORTS
1515
GC_realloc=_mlir__GC_realloc
1616
GC_free=_mlir__GC_free
1717
GC_get_heap_size=_mlir__GC_get_heap_size
18+
GC_add_roots=_mlir__GC_add_roots
19+
GC_remove_roots=_mlir__GC_remove_roots
1820
GC_malloc_explicitly_typed=_mlir__GC_malloc_explicitly_typed
1921
GC_make_descriptor=_mlir__GC_make_descriptor
2022

‎tslang/lib/TypeScriptRuntime/gc.cpp‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,16 @@ size_t _mlir__GC_get_heap_size()
5858
return GC_get_heap_size();
5959
}
6060

61+
void _mlir__GC_add_roots(void *low, void *highPlus1)
62+
{
63+
GC_add_roots(low, highPlus1);
64+
}
65+
66+
void _mlir__GC_remove_roots(void *low, void *highPlus1)
67+
{
68+
GC_remove_roots(low, highPlus1);
69+
}
70+
6171
void _mlir__GC_win32_free_heap()
6272
{
6373
#ifdef WIN32

‎tslang/test/tester/CMakeLists.txt‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -623,14 +623,11 @@ add_test(NAME test-jit-00-safe-cast-field-access COMMAND test-runner -jit "${PRO
623623
add_test(NAME test-jit-00-safe-cast-bug COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00safe_cast_bug.ts")
624624
add_test(NAME test-jit-00-optional COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00optional.ts")
625625
add_test(NAME test-jit-01-optional COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01optional.ts")
626-
if (NOT(WIN32))
627-
# TODO: crash
628-
#add_test(NAME test-jit-00-async-await COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00async_await.ts")
629-
# TODO: crash
630-
#add_test(NAME test-jit-00-for-await COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_await.ts")
631-
# TODO: in opt mode, error
632-
#add_test(NAME test-jit-00-for-await-yield COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_await_yield.ts")
633-
endif()
626+
add_test(NAME test-jit-00-async-await COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00async_await.ts")
627+
add_test(NAME test-jit-00-for-await COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_await.ts")
628+
# TODO: async generator / for-await-of hangs at runtime under JIT
629+
# error: EXEC : LLVM error : Coroutines cannot handle non static allocas yet
630+
add_test(NAME test-jit-00-for-await-yield COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00for_await_yield.ts")
634631
if (NOT(WIN32))
635632
add_test(NAME test-jit-00-try-catch COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00try_catch.ts")
636633
add_test(NAME test-jit-01-try-catch COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01try_catch.ts")
@@ -739,7 +736,7 @@ add_test(NAME test-jit-244-arraysome COMMAND test-runner -jit "${PROJECT_SOURCE
739736
add_test(NAME test-jit-Grammar_and_types COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/Grammar_and_types.ts")
740737
add_test(NAME test-jit-StackTest COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/00stack_test.ts")
741738
# TODO: find out why it is crashing
742-
#add_test(NAME test-jit-Raytrace COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/raytrace.ts")
739+
add_test(NAME test-jit-Raytrace COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/raytrace.ts")
743740
add_test(NAME test-jit-Path COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/path.ts")
744741
add_test(NAME test-jit-Print-Bug-01 COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/01print-bug.ts")
745742
add_test(NAME test-jit-arrayLiterals COMMAND test-runner -jit "${PROJECT_SOURCE_DIR}/test/tester/tests/arrayLiterals.ts")

‎tslang/tslang/jit.cpp‎

Lines changed: 95 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
#include "mlir/ExecutionEngine/ExecutionEngine.h"
44
#include "mlir/IR/BuiltinOps.h"
55

6+
#include "llvm/ADT/STLExtras.h"
7+
#include "llvm/ExecutionEngine/SectionMemoryManager.h"
68
#include "llvm/Support/DynamicLibrary.h"
9+
#include "llvm/Support/Memory.h"
10+
#include "llvm/Support/Path.h"
711

812
#include <cstdio>
913
#ifdef _WIN32
@@ -92,6 +96,63 @@ int loadLibrary(mlir::SmallString<256> &libPath, llvm::StringMap<void *> &export
9296
return 0;
9397
}
9498

99+
// JIT'd globals live in RTDyld-allocated RW data sections (plain VirtualAlloc/mmap
100+
// memory), which the Boehm GC does not scan: for AOT binaries the loader-mapped
101+
// data segment is registered as a root set automatically, but in the JIT an object
102+
// reachable only from a global (e.g. a static class member) is collected on the
103+
// first GC cycle and its memory recycled, corrupting later accesses. Register every
104+
// RW data section with the GC as a root range. GC_add_roots/GC_remove_roots are
105+
// resolved dynamically from the already-loaded TypeScriptRuntime library, so this
106+
// stays inert when the GC is disabled or not present.
107+
class GCRootsSectionMemoryMapper : public llvm::SectionMemoryManager::MemoryMapper
108+
{
109+
// AllocationPurpose lives in the enclosing SectionMemoryManager, not in the
110+
// MemoryMapper base class, so it is not found by unqualified lookup here
111+
using AllocationPurpose = llvm::SectionMemoryManager::AllocationPurpose;
112+
using GCRootsFn = void (*)(void *, void *);
113+
114+
public:
115+
llvm::sys::MemoryBlock allocateMappedMemory(AllocationPurpose purpose, size_t numBytes,
116+
const llvm::sys::MemoryBlock *const nearBlock, unsigned flags,
117+
std::error_code &errCode) override
118+
{
119+
auto block = llvm::sys::Memory::allocateMappedMemory(numBytes, nearBlock, flags, errCode);
120+
if (purpose == AllocationPurpose::RWData && block.base() != nullptr)
121+
{
122+
if (auto addRoots = reinterpret_cast<GCRootsFn>(
123+
llvm::sys::DynamicLibrary::SearchForAddressOfSymbol("GC_add_roots")))
124+
{
125+
addRoots(block.base(), static_cast<char *>(block.base()) + block.allocatedSize());
126+
gcRootBlocks.push_back(block.base());
127+
}
128+
}
129+
130+
return block;
131+
}
132+
133+
std::error_code protectMappedMemory(const llvm::sys::MemoryBlock &block, unsigned flags) override
134+
{
135+
return llvm::sys::Memory::protectMappedMemory(block, flags);
136+
}
137+
138+
std::error_code releaseMappedMemory(llvm::sys::MemoryBlock &block) override
139+
{
140+
if (llvm::is_contained(gcRootBlocks, block.base()))
141+
{
142+
if (auto removeRoots = reinterpret_cast<GCRootsFn>(
143+
llvm::sys::DynamicLibrary::SearchForAddressOfSymbol("GC_remove_roots")))
144+
{
145+
removeRoots(block.base(), static_cast<char *>(block.base()) + block.allocatedSize());
146+
}
147+
}
148+
149+
return llvm::sys::Memory::releaseMappedMemory(block);
150+
}
151+
152+
private:
153+
mlir::SmallVector<void *> gcRootBlocks;
154+
};
155+
95156
int runJit(int argc, char **argv, mlir::ModuleOp module, CompileOptions &compileOptions)
96157
{
97158
// to avoid false positive memory leak reports in release builds
@@ -135,8 +196,17 @@ int runJit(int argc, char **argv, mlir::ModuleOp module, CompileOptions &compile
135196
#define LIB_NAME "lib"
136197
#define LIB_EXT "so"
137198
#endif
199+
// Only locate a GC runtime when the user has not already supplied one via
200+
// --shared-libs: loading a second TypeScriptRuntime copy from a different path
201+
// (e.g. TSLANG_LIB_PATH) creates two independent GC instances, so roots
202+
// registered in one are invisible to collections running in the other and
203+
// GC-heap pointers cross between the two heaps.
204+
auto hasTypeScriptRuntime = llvm::any_of(clSharedLibs, [](const std::string &libPath) {
205+
return llvm::sys::path::stem(libPath).contains_insensitive("TypeScriptRuntime");
206+
});
207+
138208
std::string pathTypeScriptLib("../lib/" LIB_NAME "TypeScriptRuntime." LIB_EXT);
139-
if (!disableGC.getValue())
209+
if (!disableGC.getValue() && !hasTypeScriptRuntime)
140210
{
141211
auto absPath3 = makeAbsolutePath(mergeWithDefaultLibPath(getTslangLibPath(), LIB_NAME "TypeScriptRuntime." LIB_EXT));
142212
if (absPath3.empty())
@@ -175,7 +245,11 @@ int runJit(int argc, char **argv, mlir::ModuleOp module, CompileOptions &compile
175245
mlir::SmallVector<mlir::StringRef> sharedLibPaths;
176246
sharedLibPaths.append(begin(clSharedLibs), end(clSharedLibs));
177247

248+
// must outlive the engine: sections are released from the engine's destructor
249+
GCRootsSectionMemoryMapper gcRootsMemoryMapper;
250+
178251
mlir::ExecutionEngineOptions engineOptions;
252+
engineOptions.sectionMemoryMapper = &gcRootsMemoryMapper;
179253
engineOptions.transformer = optPipeline;
180254
engineOptions.enableObjectDump = dumpObjectFile;
181255
engineOptions.enableGDBNotificationListener = !enableOpt;
@@ -264,13 +338,29 @@ int runJit(int argc, char **argv, mlir::ModuleOp module, CompileOptions &compile
264338
}
265339

266340
// The JIT program has finished. On Windows/COFF the MLIR/ORC LLJIT platform
267-
// registers process-level atexit glue that faults during teardown, so the
268-
// normal CRT exit path crashes after a successful run. Flush stdio and exit
269-
// the process directly, bypassing the CRT atexit handlers.
341+
// registers process-level atexit glue that faults during teardown (exception
342+
// 0x80000003), so the normal CRT exit path crashes after a successful run.
343+
// ExitProcess is not safe either: it runs DLL_PROCESS_DETACH, and
344+
// TypeScriptRuntime.dll's static AsyncRuntime destructor then waits on its
345+
// thread pool whose worker threads ExitProcess has already terminated,
346+
// deadlocking forever when async work is still pending. TerminateProcess
347+
// skips all teardown, so nothing can crash or deadlock — but it also skips
348+
// the detach-time stdio flush, so flush every CRT the JIT'd code may have
349+
// bound to (ucrtbase buffers its streams separately from our static CRT).
270350
fflush(stdout);
271351
fflush(stderr);
272352
#ifdef _WIN32
273-
ExitProcess(0);
353+
for (auto crtName : {"ucrtbase.dll", "ucrtbased.dll"})
354+
{
355+
if (HMODULE crt = GetModuleHandleA(crtName))
356+
{
357+
if (auto crtFflush = (int(__cdecl *)(FILE *))GetProcAddress(crt, "fflush"))
358+
{
359+
crtFflush(nullptr); // fflush(NULL) flushes all of that CRT's output streams
360+
}
361+
}
362+
}
363+
TerminateProcess(GetCurrentProcess(), 0);
274364
#endif
275365
return 0;
276366
}

0 commit comments

Comments
 (0)