(master) hw/xfree86/glamor_egl: do not link statically glx library

[cepelinas9000]

In a987fc7 commit glx library is
statically linked which makes all glx library variables dublicated in
glx.so and glamoregl.so files

for context: #3174 (comment)

---

Backport dashboard

Target branch	Backport PR	Status
release/25.2	#3471	🔄 Open
release/25.1	#3472	🔄 Open
release/25.0	#3473	🔄 Open

_{🤖 Backport assessment on behalf of @metux. All three release branches are
vulnerable (identical link_with: [glamor, libxserver_glx] in
hw/xfree86/glamor_egl/meson.build). Note: PR #3213 carries two commits
(merge 6c75731752); each backport includes both the meson fix and its
prerequisite _X_EXPORT header-export commit (9b74a3a7f9a2). On 25.1/25.0
the headers were remapped to the glx/ layout and glx/glxutil.h gained an
explicit #include <X11/Xfuncproto.h> since it no longer includes
glxserver.h there.}

[metux]

is the glx library now becoming a xf86 module or just a shared library ?

[stefan11111]

is the glx library now becoming a xf86 module or just a shared library ?

There is already the glx static library, which all X servers except xf86 an xnest (which doesn't support glx) link statically too.

xf86 uses it's glx extension library (hw/xfree86/dixmods/glxmodule.c) to implement the glx extension, instead of the server linking to it statically.
I don't know why is was originally done like this, but nowadays it's used by nvidia to implement it's own glx library (on some drivers).

[cepelinas9000]

is the glx library now becoming a xf86 module or just a shared library ?

Currently for xfree86 it is module:

xserver/hw/xfree86/dixmods/glxmodule.c

Line 45 in b9d651f

static XF86ModuleVersionInfo VersRec = {

From glamor perspective (and in my view) it should be viewed as shared library - these _X_EXPORT'ed symbols are directly called for correct functionality.

On other xservers (kdrive and etc) - it directly linked as static library. @stefan11111 can comment on this situation much better.

Without these exports we would need to somehow make GlxPushProvider and few other single global functions.

note: then there is bind-now thingy in cases when glamor loaded before glx (we can end up with manual dereferencing X11Libre/xf86-video-amdgpu#62 for glx functions )

[stefan11111]

@cepelinas9000 I'm guessing we crossed when posting the messages, see mine above.

Linking the glx extension is the "clean" way to solve this, but it would probably break the 390 nvidia xf86 driver.

@metux @X11Libre/dev
We could add some glamor_glx_... stubs for the glx functions that glamor uses, and have glamor call the stubs instead of the glx functions directly?
Then, all X servers that use glx, except xf86, cat link statically to the stubs (that just call the glx functions directly since they are linked statically), and xf86 could implement those stubs via our wrapper around dlopen.

[metux]

🤖 Automated review — generated by Claude Code (ship: Constellation) on behalf of @metux. Not a human review.

Review: hw/xfree86/glamor_egl: do not link statically glx library — bot-review-passed

Reviewed the full diff (+22/-4, 6 files). No CI yet (fork PR, workflow-approval gate) — build-verified
locally instead, given this changes real link/runtime-symbol-resolution behavior, not just source.

This completes a previously-documented incomplete fix

AGENTS.md's "static lib linked into two loadable modules" note already flags exactly this class of
bug: PR #3174 removed glamor_egl.c's explicit call to xorgGlxCreateVendor(), but a follow-up
investigation (this workspace's _WORK_/xserver-master security-fix-worktree pass) found via nm
that the underlying duplication was not actually eliminated — hw/xfree86/glamor_egl/meson.build
still had link_with: [glamor, libxserver_glx], so any object file glamor_egl still genuinely
referenced (e.g. __glXScreenInit/__glXsetGetProcAddress for its own screen/extension setup) got
pulled from the static archive and duplicated into libglamoregl.so anyway, alongside the
already-loaded libglx.so copy — same "two loaded modules, two copies of file-scope statics" hazard.
This PR is the actual completion of that fix: drop libxserver_glx from glamor_egl's link_with:
entirely, and _X_EXPORT the handful of GLX-internal functions glamor_egl genuinely needs, so the
shared module resolves them dynamically against the single copy in libglx.so at load time instead
of getting a static copy of its own.

Build-verified the fix actually works (not just "it compiles")

Checked out the branch, built hw/xfree86/Xorg + hw/xfree86/dixmods/libglx.so +
hw/xfree86/glamor_egl/libglamoregl.so (-Dglx=true -Dglamor=true) — links cleanly (a shared module
is allowed undefined symbols at link time, resolved at dlopen; a genuinely-missing/misspelled export
would still show up as U with no matching T anywhere, so this isn't a rubber-stamp). Then nm -D
on both modules:

libglx.so:        T __glXDrawableInit / __glXEnableExtension / __glXInitExtensionEnableBits /
                    GlxPushProvider / __glXScreenInit / __glXsetGetProcAddress   (defined, once)
libglamoregl.so:  U __glXDrawableInit / __glXEnableExtension / __glXInitExtensionEnableBits /
                    GlxPushProvider / __glXScreenInit / __glXsetGetProcAddress   (undefined — no
                    longer duplicated, correctly deferred to libglx.so at load time)

Exactly the intended outcome: one defined copy, one dynamic reference, zero duplication. Load-order
assumption (libglx.so's core GLX extension init happens before glamor's driver-selected
acceleration module loads) matches how every other core-extension/DDX-module symbol dependency
already works in this loader architecture.

Driver-ABI (NVIDIA blob) — safe, additive-only

Every _X_EXPORT here is a new export on a previously-internal-only symbol — nothing existing is
removed, renamed, or resignatured. Can't break any consumer that didn't already depend on the symbol
being absent. No ABI impact.

Advisory (non-blocking)

• Minor: link_with: [glamor ] has a stray trailing space before the bracket — cosmetic only.
• Once merged, worth a one-line follow-up note in AGENTS.md's glx/glamor duplicate-symbol
  paragraph confirming this closes the gap (master) glamor: glamor_egl.c do not call xorgGlxCreateVendor() #3174 left open (that section currently ends on "so
  (master) glamor: glamor_egl.c do not call xorgGlxCreateVendor() #3174's fix didn't fully eliminate cross-module duplication" — this PR is the resolution).

Backport-worthiness — none

This is a build/link-hygiene fix (redundant static duplication, wasted .so size, was not itself
reported as causing a crash), not a security or correctness regression fix. Not a backport
candidate, though maintainer may still want it for cleanliness on release lines that carry the
same link_with: shape — a judgment call, not a required backport.

[stefan11111]

@ONykyf Could you test this patch against the proprietary 390 driver?
I don't have access to hw to test it right now.

[ONykyf]

@ONykyf Could you test this patch against the proprietary 390 driver? I don't have access to hw to test it right now.

I can do it tomorrow.

[stefan11111]

LGTM. Seems to work fine with nvidia 390 too. See: #3174 (comment)

[stefan11111]

I wonder if this breaks when not using lazy-loading. Hopefully not.

[cepelinas9000]

I wonder if this breaks when not using lazy-loading. Hopefully not.

Shouldn't break, i tested on latest Gentoo with bind-now set.

[cepelinas9000]

@metux this one goes with #3174 otherwise in released version will have chance to break using glamor renderer.

[metux]

@stefan11111 @cepelinas9000 do we need to backport this one ?

[cepelinas9000]

@stefan11111 @cepelinas9000 do we need to backport this one ?

Yes, to https://github.com/X11Libre/xserver/commits/release/25.0/ and https://github.com/X11Libre/xserver/commits/release/25.1/