diff --git a/mcpp.toml b/mcpp.toml index a5584b5..eca2bbc 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] namespace = "mcpplibs" name = "xpkg" -version = "0.0.51" +version = "0.0.52" description = "C++23 reference implementation of the xpkg V2 spec (multi-arch)" license = "Apache-2.0" repo = "https://github.com/openxlings/libxpkg" diff --git a/src/lua-stdlib/xim/libxpkg/elfpatch.lua b/src/lua-stdlib/xim/libxpkg/elfpatch.lua index 4f4814f..87b5729 100644 --- a/src/lua-stdlib/xim/libxpkg/elfpatch.lua +++ b/src/lua-stdlib/xim/libxpkg/elfpatch.lua @@ -1413,4 +1413,178 @@ function M.relocate_build_paths(opt) end +-- ───────────────────────────────────────────────────────────────────── +-- host_link_interposer +-- ───────────────────────────────────────────────────────────────────── +-- +-- A driver vendor library belongs to the HOST: it is a symlink into +-- /usr/lib/..., it must match the host's kernel module, and we cannot put an +-- RPATH on it because it is not our file. So when it is dlopen'd into one of +-- our processes, its own DT_NEEDED entries have three possible fates: +-- +-- 1. the SONAME is already loaded -> reused, automatically ours +-- 2. not loaded, but the search path finds the HOST's copy -> two builds of +-- one library in one process, ABI mixed +-- 3. not loaded and not findable -> the vendor fails to load, no GPU +-- +-- Our loader's built-in search path cannot exist by construction (AD-5), so +-- (3) is the default outcome; `DEVICE_COUNT=0`. The historical fix was to put +-- our libraries on LD_LIBRARY_PATH, which reaches (1)/(2) and also hands them +-- to every OTHER process in the subos -- including host binaries running on +-- the host loader. That is how `xlings subos use` once returned a /bin/bash +-- that died of SIGSEGV before printing a character. +-- +-- This does the same job with a scope of exactly one object. An interposer is +-- a tiny shared object that +-- +-- * takes the vendor's SONAME, so whoever asks for it gets this instead; +-- * NEEDs the real vendor by absolute path, so the vendor still loads and +-- `dlsym` on the handle still finds its entry points (dlsym searches the +-- handle's whole dependency tree); +-- * carries DT_RPATH -- not RUNPATH -- naming our payload closure, and +-- DT_RPATH is transitive along the load chain, so the vendor's own +-- DT_NEEDED resolve there. +-- +-- Nothing is put on any process-global variable. Measured on a real NVIDIA +-- stack (2026-08-06): with LD_LIBRARY_PATH carrying only the host driver +-- directory, GL_RENDERER came back `NVIDIA GeForce RTX 4080/PCIe/SSE2` and the +-- probe read back the pixel it drew. The same subos with the LD_LIBRARY_PATH +-- approach removed and nothing in its place renders on llvmpipe. +-- +-- PRECONDITION, and it is not optional -- also measured: +-- +-- > An object produced here may only be loaded by a consumer whose INTERP +-- > points into OUR payload. Host binaries must keep using the host's own +-- > vendor. +-- +-- Handing one to a host binary fails as +-- `librt.so.1: undefined symbol: __pointer_chk_guard, version GLIBC_PRIVATE` +-- because the RPATH names our glibc while the process's libc is the host's -- +-- the loader/libc split, from the one direction the same-source assertion +-- cannot see. Callers arrange this by pointing only OUR vendor-config files at +-- the interposer; see the recipe. +-- +-- elfpatch.host_link_interposer{ +-- vendor = "/usr/lib/x86_64-linux-gnu/libEGL_nvidia.so.550.144.03", +-- out = path.join(pkginfo.install_dir(), "lib", "libEGL_nvidia.so.0"), +-- soname = "libEGL_nvidia.so.0", -- default: basename of `out` +-- stub = "", -- default: from the +-- -- `interposer-stub` dep +-- libdirs = { ... }, -- default: closure_lib_paths() +-- } +-- +-- `libdirs` defaults to the closure the resolver already computed. Do not pass +-- a hand-written list: R7 -- the dependency table this replaces was written by +-- hand and was missing libm, libdrm, libgbm, libgcc_s and libwayland-*, every +-- one of which was silently coming from the host. +function M.host_link_interposer(opt) + opt = opt or {} + local vendor = opt.vendor + local out = opt.out + if not vendor or vendor == "" then + error("elfpatch.host_link_interposer: `vendor` is required (the " + .. "absolute path of the host's vendor library)") + end + if not out or out == "" then + error("elfpatch.host_link_interposer: `out` is required") + end + if not os.isfile(vendor) then + error("elfpatch.host_link_interposer: vendor not found: " .. vendor + .. " -- the host does not have this driver installed, and an " + .. "interposer pointing at a missing file would load as " + .. "successfully as one pointing at nothing") + end + + local soname = opt.soname + if not soname or soname == "" then soname = path.filename(out) end + + -- The stub. Prebuilt and shipped as a package (AD-12): there is no + -- compiler at install time, and an object cannot be created by patchelf, + -- only edited. + local stub = opt.stub + if not stub or stub == "" then + local pkginfo = _LIBXPKG_MODULES and _LIBXPKG_MODULES["pkginfo"] + if pkginfo and type(pkginfo.tool_payload_dir) == "function" then + local d = pkginfo.tool_payload_dir("interposer-stub") + if d and d ~= "" then + local cand = path.join(d, "lib", "interposer-stub.so") + if os.isfile(cand) then stub = cand end + end + end + end + if not stub or not os.isfile(stub) then + error("elfpatch.host_link_interposer: no stub. Declare a dependency " + .. "on `interposer-stub`, or pass `stub = `. There is no " + .. "compiler at install time and patchelf edits objects rather " + .. "than creating them.") + end + + local libdirs = opt.libdirs + if not libdirs then + local closure = M.closure_lib_paths({}) + libdirs = (type(closure) == "table") and closure or {} + end + if #libdirs == 0 then + error("elfpatch.host_link_interposer: the payload closure is empty, " + .. "so the interposer would resolve the vendor's dependencies " + .. "from the HOST -- which is what it exists to stop. Check " + .. "that the package declares its runtime deps.") + end + + local tool = _find_tool("patchelf") + if not tool then + error("elfpatch.host_link_interposer: patchelf not found") + end + + local outdir = path.directory(out) + if outdir and outdir ~= "" and not os.isdir(outdir) then os.mkdir(outdir) end + os.tryrm(out) + os.cp(stub, out) + + local rpath = table.concat(libdirs, ":") + local steps = { + { "--set-soname " .. _shell_quote(soname), "set-soname" }, + { "--add-needed " .. _shell_quote(vendor), "add-needed" }, + { "--set-rpath " .. _shell_quote(rpath) .. " --force-rpath", "set-rpath" }, + } + for _, s in ipairs(steps) do + if not _exec_ok(_shell_quote(tool.program) .. " " .. s[1] .. " " + .. _shell_quote(out)) then + error("elfpatch.host_link_interposer: patchelf " .. s[2] + .. " failed on " .. out) + end + end + + -- Assert the artifact, not the intent (R4). Three properties, and each one + -- silently absent produces an interposer that loads fine and does nothing: + -- a wrong SONAME is simply never asked for; a missing NEEDED yields an + -- object with no vendor behind it, so dlsym finds no entry point and the + -- caller reports "no device"; DT_RUNPATH instead of DT_RPATH is not + -- transitive, so the vendor's own dependencies fall through to the host. + local dyn = _iorun(_shell_quote(tool.program) .. " --print-soname " + .. _shell_quote(out)) or "" + if not dyn:find(soname, 1, true) then + error("elfpatch.host_link_interposer: SONAME is '" + .. _trim(dyn) .. "', expected '" .. soname + .. "' -- nothing would ever ask for this object") + end + local needed = _iorun(_shell_quote(tool.program) .. " --print-needed " + .. _shell_quote(out)) or "" + if not needed:find(vendor, 1, true) then + error("elfpatch.host_link_interposer: the vendor is not NEEDED by " + .. out .. " -- dlsym would find no entry point, and the caller " + .. "would report no device rather than an error") + end + local got_rpath = _trim(_iorun(_shell_quote(tool.program) .. " --print-rpath " + .. _shell_quote(out)) or "") + if got_rpath == "" then + error("elfpatch.host_link_interposer: no RPATH on " .. out + .. " -- the vendor's dependencies would resolve from the host") + end + + _info(string.format( + "interposer %s -> %s (%d closure dir(s))", soname, vendor, #libdirs)) + return { out = out, soname = soname, vendor = vendor, libdirs = libdirs } +end + return M diff --git a/src/xpkg-loader.cppm b/src/xpkg-loader.cppm index 7b93df2..46cf3d6 100644 --- a/src/xpkg-loader.cppm +++ b/src/xpkg-loader.cppm @@ -263,24 +263,56 @@ PlatformMatrix parse_xpm(lua::State* L, int pkg_idx) { lua::getfield(L, plat_idx, "deps"); if (lua::type(L, -1) == lua::TTABLE) { int deps_idx = lua::gettop(L); - // Detect shape: if rawlen > 0 OR the first numeric key - // exists, treat as array (legacy). Otherwise treat as - // a {runtime = ..., build = ...} table. + // Three shapes, and the third one used to be silently wrong. + // + // deps = { "a", "b" } array → both + // deps = { runtime = {...}, build = {...} } table → split + // deps = { "a", "b", build = {...} } MIXED → see below + // + // The mixed shape is what an author writes when they have a + // list of runtime deps and want to add one build-time tool. + // It parses, it reads exactly like the split form, and the + // old code took the array branch on it: `build` was dropped + // on the floor and the ARRAY entries were copied into + // build_deps instead. Measured 2026-08-06 on + // nvidia-gl-host-link -- `deps.build = {"xim:patchelf"}` + // alongside five runtime deps installed no patchelf, and + // nothing said so. + // + // Now: an author who writes `runtime`/`build` at all has + // opted into the split, so honour it, and fold the array part + // into runtime (which is what a list beside `build = {...}` + // can only mean). Array-only keeps the legacy fan-out + // untouched -- every existing recipe is that shape. int len = static_cast(lua::rawlen(L, deps_idx)); - bool looks_like_array = len > 0; - if (looks_like_array) { + + lua::getfield(L, deps_idx, "runtime"); + bool has_runtime_key = lua::type(L, -1) == lua::TTABLE; + auto rt_key = parse_string_array(lua::gettop(L)); + lua::pop(L, 1); + + lua::getfield(L, deps_idx, "build"); + bool has_build_key = lua::type(L, -1) == lua::TTABLE; + auto bd = parse_string_array(lua::gettop(L)); + lua::pop(L, 1); + + bool split = has_runtime_key || has_build_key; + + if (!split) { auto v = parse_string_array(deps_idx); xpm.runtime_deps[platform] = v; xpm.build_deps[platform] = v; xpm.deps[platform] = v; } else { - lua::getfield(L, deps_idx, "runtime"); - auto rt = parse_string_array(lua::gettop(L)); - lua::pop(L, 1); - - lua::getfield(L, deps_idx, "build"); - auto bd = parse_string_array(lua::gettop(L)); - lua::pop(L, 1); + auto rt = rt_key; + if (len > 0) { + // The array part of a mixed table: runtime deps + // written positionally, with `build` beside them. + auto arr = parse_string_array(deps_idx); + for (auto& d : arr) + if (std::find(rt.begin(), rt.end(), d) == rt.end()) + rt.push_back(d); + } xpm.runtime_deps[platform] = rt; xpm.build_deps[platform] = bd; diff --git a/tests/fixtures/pkgindex/pkgs/d/depsmixed.lua b/tests/fixtures/pkgindex/pkgs/d/depsmixed.lua new file mode 100644 index 0000000..ac525f5 --- /dev/null +++ b/tests/fixtures/pkgindex/pkgs/d/depsmixed.lua @@ -0,0 +1,28 @@ +package = { + spec = "1", + name = "depsmixed", + description = "Mixed-form deps: a positional runtime list beside build = {...}", + licenses = {"MIT"}, + repo = "https://example.com/depsmixed", + type = "package", + archs = {"x86_64"}, + + xpm = { + linux = { + -- The shape an author reaches for when they already have a list + -- of runtime deps and want to add one install-time tool. Before + -- 0.0.52 this took the array branch: `build` was dropped and the + -- array was copied into build_deps instead. + deps = { + "node", + "npm", + build = { "patchelf" }, + }, + ["latest"] = { ref = "3.0.0" }, + ["3.0.0"] = { + url = "https://example.com/depsmixed-3.0.0.tar.gz", + sha256 = "0000000000000000000000000000000000000000000000000000000000000000", + }, + }, + }, +} diff --git a/tests/test_executor.cpp b/tests/test_executor.cpp index 8d91fde..707828d 100644 --- a/tests/test_executor.cpp +++ b/tests/test_executor.cpp @@ -435,6 +435,185 @@ TEST(ExecutorTest, ApplyElfpatchAuto_LinuxUsesPatchelfForElf) { fs::remove_all(temp_dir); } +// A driver vendor library is the host's file: a symlink into /usr/lib, coupled +// to the host's kernel module, and not ours to put an RPATH on. The historical +// answer was to put OUR libraries on LD_LIBRARY_PATH so the vendor could find +// its dependencies -- which also handed them to every other process in the +// subos, including host binaries on the host loader. That is how +// `xlings subos use` once returned a /bin/bash that died of SIGSEGV before +// printing a character. +// +// host_link_interposer does the same job with a scope of exactly one object. +// Measured on a real NVIDIA stack on 2026-08-06: with LD_LIBRARY_PATH carrying +// only the host driver directory, GL_RENDERER came back as the RTX 4080 and +// the probe read back the pixel it drew; the same subos without it renders on +// llvmpipe. +// +// These cover the SHAPE of the produced object. Each of the three assertions +// below exists because its absence produces an interposer that loads perfectly +// and does nothing: a wrong SONAME is never asked for, a missing NEEDED leaves +// dlsym with no entry point (the caller reports "no device", not an error), +// and DT_RUNPATH instead of DT_RPATH is not transitive so the vendor's own +// dependencies fall through to the host. +TEST(ExecutorTest, HostLinkInterposer_ShapeIsAssertedNotAssumed) { +#ifdef _WIN32 + GTEST_SKIP() << "ELF-specific"; +#endif + const fs::path temp_dir = make_temp_dir("libxpkg-interposer-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path libdir = install_dir / "lib"; + const fs::path tools = temp_dir / "tools"; + const fs::path log_path = temp_dir / "tool.log"; + const fs::path pkg_path = temp_dir / "interposer.lua"; + const fs::path stub = temp_dir / "stub.so"; + const fs::path vendor = temp_dir / "libFAKE_vendor.so.550"; + + fs::create_directories(libdir); + fs::create_directories(tools); + + // A fake patchelf that records its arguments and answers the three + // --print-* queries from what it was told to set. That is enough to prove + // the call sequence and that the result is CHECKED; whether real patchelf + // writes a valid ELF is real patchelf's business. + write_executable_script(tools / "patchelf", + "#!/bin/sh\n" + "printf 'patchelf %s\\n' \"$*\" >> \"$ELFPATCH_LOG\"\n" + "case \"$1\" in\n" + " --set-soname) echo \"$2\" > \"$ELFPATCH_STATE.soname\" ;;\n" + " --add-needed) echo \"$2\" >> \"$ELFPATCH_STATE.needed\" ;;\n" + " --set-rpath) echo \"$2\" > \"$ELFPATCH_STATE.rpath\" ;;\n" + " --print-soname) cat \"$ELFPATCH_STATE.soname\" 2>/dev/null ;;\n" + " --print-needed) cat \"$ELFPATCH_STATE.needed\" 2>/dev/null ;;\n" + " --print-rpath) cat \"$ELFPATCH_STATE.rpath\" 2>/dev/null ;;\n" + "esac\n" + "exit 0\n"); + + write_text(stub, "\x7f" "ELF-stub-placeholder\n"); + write_text(vendor, "\x7f" "ELF-vendor-placeholder\n"); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"interposer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.host_link_interposer{\n" + " vendor = \"" + vendor.string() + "\",\n" + " out = \"" + (libdir / "libFAKE_vendor.so.0").string() + "\",\n" + " stub = \"" + stub.string() + "\",\n" + " libdirs = { \"/payload/a/lib\", \"/payload/b/lib64\" },\n" + " }\n" + " return true\n" + "end\n"); + + const std::string original_path = std::getenv("PATH") ? std::getenv("PATH") : ""; + ScopedEnvVar path_env("PATH", tools.string() + ":" + original_path); + ScopedEnvVar log_env("ELFPATCH_LOG", log_path.string()); + ScopedEnvVar st_env("ELFPATCH_STATE", (temp_dir / "state").string()); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto ctx = make_context(install_dir, "linux", tools); + auto hook_result = exec->run_hook(HookType::Install, ctx); + ASSERT_TRUE(hook_result.success) << hook_result.error; + + EXPECT_TRUE(fs::exists(libdir / "libFAKE_vendor.so.0")) + << "the interposer was not produced"; + + std::ifstream lf(log_path); + std::ostringstream lb; lb << lf.rdbuf(); + const std::string log = lb.str(); + + // The SONAME is the vendor's, so whoever asks for it gets this instead. + EXPECT_NE(log.find("--set-soname libFAKE_vendor.so.0"), std::string::npos) + << log; + // The real vendor by ABSOLUTE path -- dlsym searches the handle's whole + // dependency tree, which is how glvnd still reaches the real entry points. + EXPECT_NE(log.find("--add-needed " + vendor.string()), std::string::npos) + << log; + // --force-rpath, not RUNPATH. DT_RPATH is transitive along the load chain + // and DT_RUNPATH is not; that difference is the whole mechanism. + EXPECT_NE(log.find("--set-rpath /payload/a/lib:/payload/b/lib64"), + std::string::npos) << log; + EXPECT_NE(log.find("--force-rpath"), std::string::npos) << log; + + fs::remove_all(temp_dir); +} + +// An empty closure means the vendor's dependencies would resolve from the +// HOST, which is the single thing this function exists to prevent. Failing the +// install is the only outcome that is not a silent success: the interposer +// would load, the GPU would work by accident, and every library it pulled in +// would be the host's. +TEST(ExecutorTest, HostLinkInterposer_RefusesAnEmptyClosure) { +#ifdef _WIN32 + GTEST_SKIP() << "ELF-specific"; +#endif + const fs::path temp_dir = make_temp_dir("libxpkg-interposer-empty-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "interposer.lua"; + const fs::path stub = temp_dir / "stub.so"; + const fs::path vendor = temp_dir / "libFAKE_vendor.so.550"; + fs::create_directories(install_dir); + write_text(stub, "stub\n"); + write_text(vendor, "vendor\n"); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"interposer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.host_link_interposer{\n" + " vendor = \"" + vendor.string() + "\",\n" + " out = \"" + (install_dir / "x.so").string() + "\",\n" + " stub = \"" + stub.string() + "\",\n" + " libdirs = {},\n" + " }\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto r = exec->run_hook(HookType::Install, make_context(install_dir, "linux")); + EXPECT_FALSE(r.success) + << "an empty closure resolves the vendor's dependencies from the host"; + + fs::remove_all(temp_dir); +} + +// A vendor that is not there. An interposer NEEDing a missing file loads +// exactly as successfully as one NEEDing nothing, and the caller reports "no +// device" -- a machine without this driver must fail at install, where the +// message can say so. +TEST(ExecutorTest, HostLinkInterposer_RefusesAMissingVendor) { +#ifdef _WIN32 + GTEST_SKIP() << "ELF-specific"; +#endif + const fs::path temp_dir = make_temp_dir("libxpkg-interposer-novendor-"); + const fs::path install_dir = temp_dir / "install"; + const fs::path pkg_path = temp_dir / "interposer.lua"; + const fs::path stub = temp_dir / "stub.so"; + fs::create_directories(install_dir); + write_text(stub, "stub\n"); + + write_text(pkg_path, + "package = { spec = \"1\", name = \"interposer\", xpm = { linux = { [\"latest\"] = { ref = \"1.0.0\" }, [\"1.0.0\"] = { url = \"https://example.com/d.tar.gz\", sha256 = \"0\" } } } }\n" + "local elfpatch = import(\"xim.libxpkg.elfpatch\")\n" + "function install()\n" + " elfpatch.host_link_interposer{\n" + " vendor = \"" + (temp_dir / "does-not-exist.so").string() + "\",\n" + " out = \"" + (install_dir / "x.so").string() + "\",\n" + " stub = \"" + stub.string() + "\",\n" + " libdirs = { \"/p/lib\" },\n" + " }\n" + " return true\n" + "end\n"); + + auto exec = create_executor(pkg_path); + ASSERT_TRUE(exec.has_value()) << (exec ? "" : exec.error()); + auto r = exec->run_hook(HookType::Install, make_context(install_dir, "linux")); + EXPECT_FALSE(r.success) << "an interposer pointing at a missing vendor"; + + fs::remove_all(temp_dir); +} + // A downloaded prebuilt carries the build machine's absolute paths in its text // files. glibc's recipe knew this and rewrote them, and got all three parts of // the job wrong -- so this is the shared capability that replaces it. diff --git a/tests/test_loader.cpp b/tests/test_loader.cpp index 856b494..bdcc5bd 100644 --- a/tests/test_loader.cpp +++ b/tests/test_loader.cpp @@ -160,6 +160,35 @@ TEST(LoaderTest, LoadPackage_DepsLegacy_FansOutToBoth) { EXPECT_EQ(un->second, expected); } +// Mixed form: a positional runtime list WITH `build = {...}` beside it. +// +// This is the shape that silently lost its build deps before 0.0.52: `deps` +// had a non-empty array part, so the loader took the legacy branch, dropped +// `build`, and copied the array into build_deps. Both halves have to be +// asserted -- checking only that build_deps contains patchelf would still +// pass if the array were also being fanned out into it. +TEST(LoaderTest, LoadPackage_DepsMixed_ArrayIsRuntimeAndBuildSurvives) { + auto result = load_package(PKGINDEX / "pkgs/d/depsmixed.lua"); + ASSERT_TRUE(result.has_value()) << result.error(); + + auto& xpm = result->xpm; + auto rt = xpm.runtime_deps.find("linux"); + auto bd = xpm.build_deps.find("linux"); + auto un = xpm.deps.find("linux"); + ASSERT_NE(rt, xpm.runtime_deps.end()); + ASSERT_NE(bd, xpm.build_deps.end()); + ASSERT_NE(un, xpm.deps.end()); + + std::vector expectedRt{"node", "npm"}; + std::vector expectedBd{"patchelf"}; + EXPECT_EQ(rt->second, expectedRt); + EXPECT_EQ(bd->second, expectedBd) + << "build = {...} beside an array was dropped, or the array leaked into build_deps"; + + std::vector expectedUnion{"node", "npm", "patchelf"}; + EXPECT_EQ(un->second, expectedUnion); +} + // Split form: deps = { runtime = {...}, build = {...} } must keep // the two lists separate, and the legacy `deps` field must hold // their union (preserving insertion order: runtime first, then build).