On Tue, Sep 07, 2021 at 12:27:27PM -0700, Andres Freund wrote: > Hi, > > On 2021-08-18 15:00:59 +0000, Jelte Fennema wrote: > > I ran into some segfaults when using Postgres that was compiled with LLVM > > 7. According to the backtraces these crashes happened during the call to > > llvm_shutdown, during cleanup after another out of memory condition. It > > seems that calls to LLVMOrcDisposeInstance, can crash (at least on LLVM 7) > > when LLVM is left in bad state. I attached the relevant part of the > > stacktrace to this email. > > > With the attached patch these segfaults went away. The patch turns > > llvm_shutdown into a no-op whenever the backend is exiting with an > > error. Based on my understanding of the code this should be totally fine. No > > memory should be leaked, since all memory will be cleaned up anyway once the > > backend exits shortly after. The only reason this cleanup code even seems to > > exist at all is to get useful LLVM profiling data. To me it seems be > > acceptable if the profiling data is incorrect/missing when the backend exits > > with an error. > > I think this is a tad too strong. We should continue to clean up on exit as > long as the error didn't happen while we're already inside llvm > code. Otherwise we loose some ability to find leaks. How about checking in the > error path whether fatal_new_handler_depth is > 0, and skipping cleanup in > that case? Because that's precisely when it should be unsafe to reenter LLVM.
This avoids a crash when compiled with llvm7+clang7 on RH7 on master: python3 -c "import pg; db=pg.DB('dbname=postgres host=/tmp port=5678'); db.query('SET jit_above_cost=0; SET jit_inline_above_cost=-1; SET jit=on; SET client_min_messages=debug'); db.query('begin'); db.query_formatted('SELECT 1 FROM generate_series(1,99)a WHERE a=%s', [1], inline=False);" diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index df691cbf1c..a3869ee700 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -885,6 +885,12 @@ llvm_session_initialize(void) static void llvm_shutdown(int code, Datum arg) { + extern int fatal_new_handler_depth; + + // if (code!=0) + if (fatal_new_handler_depth > 0) + return; + #if LLVM_VERSION_MAJOR > 11 { if (llvm_opt3_orc) diff --git a/src/backend/jit/llvm/llvmjit_error.cpp b/src/backend/jit/llvm/llvmjit_error.cpp index 26bc828875..802dc1b058 100644 --- a/src/backend/jit/llvm/llvmjit_error.cpp +++ b/src/backend/jit/llvm/llvmjit_error.cpp @@ -24,7 +24,7 @@ extern "C" #include "jit/llvmjit.h" -static int fatal_new_handler_depth = 0; +int fatal_new_handler_depth = 0; static std::new_handler old_new_handler = NULL; static void fatal_system_new_handler(void);