Hi, On 2021-06-19 17:07:51 +1200, Thomas Munro wrote: > On Sat, May 22, 2021 at 12:25 PM Andres Freund <and...@anarazel.de> wrote: > > On 2021-05-21 15:57:01 -0700, Andres Freund wrote: > > > I found the LLVM commit to blame > > > (c8fc5e3ba942057d6c4cdcd1faeae69a28e7b671). > > > Contacting the author and reading the change to see if I can spit the > > > issue myself. > > > > Hrmpf. It's a silent API breakage. The author intended to email us about > > it, but apparently forgot. One now needs to increment a string-pool > > refcount. The reason that didn't trigger a reliable crash is that > > there's a path where the refcount of string-pool entries aren't asserted > > to be above before decrementing the refcount... And that there > > practically never are references to the pool entries after use. > > > > Continuing to discusss whether there's a better way to deal with this. > > Any news?
Nothing really. I'd discussed it a bit with the relevant LLVM maintainer, but we didn't come to a real resolution. He apologized for not giving a proper heads up - he'd planned to send out an email, but somehow lost track of that. Given that any potential solution would be also end up being a versioned ifdef, I think adding something like what you suggest here is the least unreasonable solution. > FWIW this change appears to fix the problem for my system (LLVM 13 > build from a couple of days ago). No more weird results, valgrind > errors gone. I ran the leak checker to see if I now had the opposite > problem, and although there are various leaks reported, I didn't see > obvious intern pool related stacks. > > diff --git a/src/backend/jit/llvm/llvmjit.c b/src/backend/jit/llvm/llvmjit.c > index 71029a39a9..7b09e520f5 100644 > --- a/src/backend/jit/llvm/llvmjit.c > +++ b/src/backend/jit/llvm/llvmjit.c > @@ -1116,6 +1116,11 @@ > llvm_resolve_symbols(LLVMOrcDefinitionGeneratorRef GeneratorObj, void > *Ctx, > if (error != LLVMErrorSuccess) > LLVMOrcDisposeMaterializationUnit(mu); > > +#if LLVM_VERSION_MAJOR > 12 > + for (int i = 0; i < LookupSetSize; i++) > + LLVMOrcRetainSymbolStringPoolEntry(symbols[i].Name); > +#endif > + > pfree(symbols); 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? Greetings, Andres Freund