On Sun, Jun 20, 2021 at 10:59 PM Andres Freund <and...@anarazel.de> wrote: > I think this should be part of the earlier loop? Once > LLVMOrcAbsoluteSymbols() is called that owns the reference, so there > doesn't seem to be a reason to increase the refcount only later?
Right, that makes sense. Here's a patch like that. Looking at their release schedule on https://llvm.org/, I see we have a gamble to make. They currently plan to cut RC1 at the end of July, and to release in late September (every second LLVM major release coincides approximately with a PG major release). Option 1: wait until we branch for 14, and then push this to master so that at least seawasp can get back to looking for new problems, and then back-patch only after they release (presumably in time for our November releases). If their API change sticks, PostgreSQL crashes and gives weird results with the initial release of LLVM 13 until our fix comes out. Option 2: get ahead of their release and get this into 14 + August back branch releases based on their current/RC behaviour. If they decide to revert the change before the final release, we'll leak symbol names because we hold an extra reference, until we can fix that. For the last round of changes[1], there was a similar when-to-act question, but that was a doesn't-compile-anymore API change, whereas this is a silent demons-might-fly-out-of-your-nose API change. [1] https://www.postgresql.org/message-id/flat/20201016011244.pmyvr3ee2gbzplq4%40alap3.anarazel.de
From 6bb6eeaead0a80663504bbec7bbb1a32f08cfd92 Mon Sep 17 00:00:00 2001 From: Thomas Munro <thomas.mu...@gmail.com> Date: Mon, 21 Jun 2021 09:41:43 +1200 Subject: [PATCH] Adjust for LLVM 13 API change. LLVM 13 (not yet released, but due out in a few months) has changed the semantics of LLVMOrcAbsoluteSymbols(), so that we now need to bump a reference count on the symbol names we give it to avoid a double-free. Discussion: https://postgr.es/m/CA%2BhUKGLEy8mgtN7BNp0ooFAjUedDTJj5dME7NxLU-m91b85siA%40mail.gmail.com --- src/backend/jit/llvm/llvmjit.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c index 71029a39a9..df691cbf1c 100644 --- a/src/backend/jit/llvm/llvmjit.c +++ b/src/backend/jit/llvm/llvmjit.c @@ -1106,6 +1106,9 @@ llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void *Ctx, { const char *name = LLVMOrcSymbolStringPoolEntryStr(LookupSet[i].Name); +#if LLVM_VERSION_MAJOR > 12 + LLVMOrcRetainSymbolStringPoolEntry(LookupSet[i].Name); +#endif symbols[i].Name = LookupSet[i].Name; symbols[i].Sym.Address = llvm_resolve_symbol(name, NULL); symbols[i].Sym.Flags.GenericFlags = LLVMJITSymbolGenericFlagsExported; -- 2.30.2
