Hi hackers,

With JIT inlining enabled but optimization not, queries using some
contrib functions kill the session at code emission:

   SET jit = on;
   SET jit_above_cost = 0;
   SET jit_inline_above_cost = 0;
   SET jit_optimize_above_cost = -1;
   SELECT page_checksum(get_raw_page('t', 0), 0);
   FATAL:  fatal llvm error: Program used external function
           'get_page_from_raw' which could not be resolved!

Since 089480c0770 extension libraries are built with
-fvisibility=hidden, so get_page_from_raw(), ltree's compare_subnode()
and pgcrypto's mbuf_create_from_data() aren't exported from their
libraries.  The inliner doesn't know that: when it imports
page_checksum_1_9() it leaves get_page_from_raw() as an external call,
expecting it to be resolved at emission time, and it isn't.

When jit_optimize_above_cost is reached too, O3 often inlines the helper
as well and the reference goes away.  With the default settings both
thresholds are 500000, which is probably why nobody has reported this.
But it only takes jit_inline_above_cost being reached without
jit_optimize_above_cost, both of which any user can set, and nothing
guarantees that O3 inlines the helper either.  jit is off by default on
master since 7f8c88c2b87, but not on the back branches.

I found this while moving our test hosts from LLVM 19 to 21.  It isn't
specific to one LLVM version: the same contrib tests fail on Debian 13
with 19.1.7 and 21.1.8, on x86_64 and aarch64, when check-world is run
with

   PG_TEST_INITDB_EXTRA_OPTS='-c jit=on -c jit_above_cost=0
     -c jit_inline_above_cost=0 -c jit_optimize_above_cost=-1'

   pageinspect: page, checksum, oldextversions
   ltree:       ltree
   pgcrypto:    pgp-info  (x86_64 only)

The attached patch makes function_inlinable() refuse a function that
refers to an external function or variable that can't be resolved, using
the same lookup llvm_resolve_symbol() does at emission time. Exported
functions are still inlined as before (int4pl into a plain expression,
citext_eq, hstore_exists); ltree's ltxtq_exec is now left as a call
instead of crashing.  The cost is one dlsym() per external symbol
referenced by an inlining candidate; I haven't measured it separately,
but in the 19 vs 21 runs below the inlining phase didn't get slower.

Compiling the bitcode with -fvisibility=hidden and checking symbol
visibility in the inliner instead wouldn't help bitcode already
installed by extensions, nor cover libraries that restrict their exports
some other way, so I went with the lookup.

The patch adds a JIT case to pageinspect's page test, which fails
without the fix even on a plain "make check", since it sets the jit GUCs
itself.  Without LLVM it passes trivially.  It relies on
get_page_from_raw() staying unexported, and only bites on builds that
install extension bitcode, which today means autoconf builds; meson will
too once CF 5671 (LLVM bitcode for meson builds) is in.

With the patch, check-world has no failures with inlining forced (above)
or with inlining and optimization forced (jit_optimize_above_cost = 0),
on LLVM 19 and 21, x86_64 and aarch64. The jit=off and
JIT-without-inlining modes I ran only on unpatched master, where 19 and
21 gave identical results; the patch only touches function_inlinable(),
which those modes don't reach.

I'd suggest backpatching to 16, where hidden visibility arrived.  It
applies to 16 through 19 with offsets.

One ordering problem: the new test is the first in the tree that turns
JIT on, so on CI's AddressSanitizer job (Linux - Meson 64-bit) it hits
the unrelated ASAN + JIT crash Henson Choi is fixing in [1], and the
backend dies of SIGILL there.  So the test can only go in together with
or after that fix.  If that's going to take a while, I'd suggest that we
split the test out and apply it as a follow-up after [1] lands.

best.

-greg

[1] 
https://postgr.es/m/CAAAe_zA49cHXJ7J4hmeuSa75s%2BeSgCOnK_9Uvq6wUEJ2Bgvdfg%40mail.gmail.com

From d9ed8204b0f06f9afaca6e3266adf58b45f17603 Mon Sep 17 00:00:00 2001
From: Greg Burd <[email protected]>
Date: Tue, 27 Oct 2026 20:59:21 -0400
Subject: [PATCH v2 1/2] jit: Don't inline code that references unexported
 symbols.

Since commit 089480c0770, extension libraries are built with
-fvisibility=hidden, and only symbols marked PGDLLEXPORT are exported.
The JIT inliner treats every function and variable with external
linkage that an inlined function refers to as something it can leave as
an external reference in the JITed module, to be resolved at emission
time.  When an inlined function referenced a non-exported helper in its
own library, e.g. pageinspect's get_page_from_raw() or ltree's
compare_subnode(), emission failed with

  FATAL:  fatal llvm error: Program used external function
  'get_page_from_raw' which could not be resolved!

taking the session down.  This can happen whenever inlining runs.  It is
easiest to reproduce when jit_inline_above_cost is reached but not
jit_optimize_above_cost, as optimization often inlines the helper too,
but nothing guarantees that.  With the default settings both thresholds
are 500000, which is probably why it went unnoticed.  With
jit_above_cost = 0, jit_inline_above_cost = 0 and
jit_optimize_above_cost = -1, the regression tests of pageinspect and
ltree fail this way with LLVM 19 and 21 on x86_64 and aarch64, and
pgcrypto's on x86_64.

Fix by checking, before deciding that a function is inlinable, that
each external symbol it references can be resolved, as
llvm_resolve_symbol() would at emission time.  Functions that don't
pass are not inlined.  Compiling the bitcode with -fvisibility=hidden
and checking symbol visibility instead wouldn't help bitcode already
installed by extensions, nor cover libraries that restrict their
exports some other way.

Add a test to pageinspect that inlines page_checksum(), which fails
without this fix.

Author: Greg Burd <[email protected]>
Backpatch-through: 16
---
 contrib/pageinspect/expected/page.out   | 20 ++++++++++++
 contrib/pageinspect/sql/page.sql        | 16 ++++++++++
 src/backend/jit/llvm/llvmjit_inline.cpp | 41 ++++++++++++++++++++++++-
 3 files changed, 76 insertions(+), 1 deletion(-)

diff --git a/contrib/pageinspect/expected/page.out 
b/contrib/pageinspect/expected/page.out
index fcf19c5ca5a..aa7fb5f18cf 100644
--- a/contrib/pageinspect/expected/page.out
+++ b/contrib/pageinspect/expected/page.out
@@ -286,3 +286,23 @@ select tuple_data_split('test_sequence'::regclass, t_data, 
t_infomask, t_infomas
 (1 row)
 
 drop sequence test_sequence;
+-- JIT inlining must not pull in references to functions that aren't
+-- exported from pageinspect's library, like get_page_from_raw().  Without
+-- JIT support this passes trivially.
+CREATE TEMP TABLE test_jit (a int);
+INSERT INTO test_jit VALUES (1);
+SET jit = on;
+SET jit_above_cost = 0;
+SET jit_inline_above_cost = 0;
+SET jit_optimize_above_cost = -1;
+SELECT page_checksum(get_raw_page('test_jit', 0), 0) IS NOT NULL AS 
jit_inline_test;
+ jit_inline_test 
+-----------------
+ t
+(1 row)
+
+RESET jit;
+RESET jit_above_cost;
+RESET jit_inline_above_cost;
+RESET jit_optimize_above_cost;
+DROP TABLE test_jit;
diff --git a/contrib/pageinspect/sql/page.sql b/contrib/pageinspect/sql/page.sql
index c75fe1147f6..920399249da 100644
--- a/contrib/pageinspect/sql/page.sql
+++ b/contrib/pageinspect/sql/page.sql
@@ -123,3 +123,19 @@ create sequence test_sequence start 72057594037927937;
 select tuple_data_split('test_sequence'::regclass, t_data, t_infomask, 
t_infomask2, t_bits)
   from heap_page_items(get_raw_page('test_sequence', 0));
 drop sequence test_sequence;
+
+-- JIT inlining must not pull in references to functions that aren't
+-- exported from pageinspect's library, like get_page_from_raw().  Without
+-- JIT support this passes trivially.
+CREATE TEMP TABLE test_jit (a int);
+INSERT INTO test_jit VALUES (1);
+SET jit = on;
+SET jit_above_cost = 0;
+SET jit_inline_above_cost = 0;
+SET jit_optimize_above_cost = -1;
+SELECT page_checksum(get_raw_page('test_jit', 0), 0) IS NOT NULL AS 
jit_inline_test;
+RESET jit;
+RESET jit_above_cost;
+RESET jit_inline_above_cost;
+RESET jit_optimize_above_cost;
+DROP TABLE test_jit;
diff --git a/src/backend/jit/llvm/llvmjit_inline.cpp 
b/src/backend/jit/llvm/llvmjit_inline.cpp
index ebc0fe92b73..abab3ef3b0c 100644
--- a/src/backend/jit/llvm/llvmjit_inline.cpp
+++ b/src/backend/jit/llvm/llvmjit_inline.cpp
@@ -41,6 +41,7 @@ extern "C"
 
 #include <llvm-c/Core.h>
 #include <llvm-c/BitReader.h>
+#include <llvm-c/Support.h>
 
 /* Avoid macro clash with LLVM's C++ headers */
 #undef Min
@@ -136,6 +137,7 @@ static void function_references(llvm::Function &F,
                                                                
llvm::SmallPtrSet<llvm::Function *, 8> &referencedFunctions);
 
 static void add_module_to_inline_search_path(InlineSearchPath& path, 
llvm::StringRef modpath);
+static bool symbol_resolvable(const llvm::GlobalValue *gv);
 static llvm::SmallVector<llvm::GlobalValueSummary *, 1>
 summaries_for_guid(const InlineSearchPath& path, llvm::GlobalValue::GUID guid);
 
@@ -625,10 +627,18 @@ function_inlinable(llvm::Function &F,
 
                /*
                 * Never want to inline externally visible vars, cheap enough to
-                * reference.
+                * reference.  But the reference has to be resolvable.
                 */
                if (rv->hasExternalLinkage() || 
rv->hasAvailableExternallyLinkage())
+               {
+                       if (!symbol_resolvable(rv))
+                       {
+                               ilog(DEBUG1, "cannot inline %s due to 
unresolvable variable %s",
+                                        F.getName().data(), 
rv->getName().data());
+                               return false;
+                       }
                        continue;
+               }
 
                /*
                 * If variable is file-local, we need to inline it, to be able 
to
@@ -682,6 +692,17 @@ function_inlinable(llvm::Function &F,
                {
                        llvm::StringRef funcName = 
referencedFunction->getName();
 
+                       /*
+                        * The function might not get inlined, in which case 
the call has
+                        * to be resolvable.
+                        */
+                       if (!symbol_resolvable(referencedFunction))
+                       {
+                               ilog(DEBUG1, "cannot inline %s due to 
unresolvable function %s",
+                                        F.getName().data(), funcName.data());
+                               return false;
+                       }
+
                        /*
                         * Don't bother checking for inlining if remaining cost 
budget is
                         * very small.
@@ -821,6 +842,24 @@ add_module_to_inline_search_path(InlineSearchPath& 
searchpath, llvm::StringRef m
                searchpath.push_back(it->second.get());
 }
 
+/*
+ * Check whether a reference to gv from JITed code can be resolved at
+ * emission time, the way llvm_resolve_symbol() resolves names that aren't
+ * qualified with a module name.
+ *
+ * Not every symbol with external linkage in an extension's bitcode is
+ * exported by its shared library: extension libraries are built with
+ * -fvisibility=hidden, so only symbols marked PGDLLEXPORT are.  A reference
+ * to any other one would make emission fail with a FATAL error.
+ */
+static bool
+symbol_resolvable(const llvm::GlobalValue *gv)
+{
+       std::string 
name(llvm::GlobalValue::dropLLVMManglingEscape(gv->getName()));
+
+       return LLVMSearchForAddressOfSymbol(name.c_str()) != nullptr;
+}
+
 /*
  * Search for all references for functions hashing to guid in the search path,
  * and return them in search path order.
-- 
2.54.0

Reply via email to