Handle singleton COMDAT functions in i386 musl RTS

This commit is contained in:
Moritz Angermann committed 2026-10-17 09:55:24 +09:00
1 parent 38394fd9b8
commit 03b83dd4df
11 files changed
+407 -1

No files matched your search

+7
View File
@@ -106,6 +106,13 @@ jobs:
run: |
nix eval --impure --json --file scripts/nix/check-musl-iserv-scope.nix \
--apply 'f: f { lib = (builtins.getFlake (toString ./.)).inputs.nixpkgs.lib; }'
nix eval --impure --json --file scripts/nix/check-musl-rts-scope.nix \
--apply 'f: f { lib = (builtins.getFlake (toString ./.)).inputs.nixpkgs.lib; }'
- name: Check scoped ELF COMDAT function coalescing
if: ${{ matrix.system == 'x86_64-linux' && matrix.target == 'linux-static' }}
timeout-minutes: 120
run: nix build .#checks.x86_64-linux.ghc-elf-comdat-functions --no-link -L --max-jobs 1 --timeout "$NIX_BUILD_TIMEOUT"
- name: Check Intel Setup failure selection
if: ${{ matrix.system == 'x86_64-darwin' && matrix.target == 'all' }}
+4
View File
@@ -671,6 +671,10 @@
hydraJobs = packages;
checks = pkgs.lib.optionalAttrs (system == "x86_64-linux") {
ghc-elf-comdat-functions = import ./scripts/nix/check-elf-comdat-functions.nix {
inherit pkgs;
compiler = (compilerSelection pkgs).ghc914-sh;
};
ghc-elf-alignment = import ./scripts/nix/check-elf-alignment.nix {
inherit pkgs;
compiler = (compilerSelection pkgs).ghc914-sh;
+29
View File
@@ -60,6 +60,24 @@ The scope check uses the real Nix module fixed point in the Linux static CI
group, including `pkgs` supplied through `_module.args`. Repeat it with
`nix eval --impure --json --file scripts/nix/check-musl-iserv-scope.nix --apply 'f: f { lib = (builtins.getFlake (toString ./.)).inputs.nixpkgs.lib; }'`.
The i386 musl target RTS also handles duplicate hidden global functions when
both definitions are the whole, sole member of COMDAT groups with the same
signature. This covers GCC's `__x86.get_pc_thunk.*` PIC helpers. Coalescing is
checked against the current symbol owner during discovery and loading; it
does not persist weak-symbol status or suppress ordinary strong conflicts.
Multi-member groups remain outside this patch's scope. The source patch applies
to application and interpreter projects, including profiling. Native compilers
and other cross targets keep their existing RTS sources.
Run the native x86_64 Linux regression with
`nix build .#checks.x86_64-linux.ghc-elf-comdat-functions --no-link -L --max-jobs 1 --timeout 7200`.
It links the same configured RTS sources with and without the patch, verifies
the replacement objects in both linker maps, checks relocations in both load
orders, and rejects strong conflicts and stale archive-owner decisions. Each
probe has a 15-second bound. Module scope is checked separately by
`scripts/nix/check-musl-rts-scope.nix` using the same invocation as the iserv
scope check above.
Windows patches Basement's `INVALID_HANDLE_VALUE` expression to `INT_PTR`, so
hsc2hs receives the signed pointer-sized integer expected by the Haskell API.
The generated constant converts through Haskell `Int` with `Prelude.fromIntegral`;
@@ -359,6 +377,17 @@ records are byte-identical. The changed project skeleton rebuilds auxiliary boot
derivations. An i686/QEMU C control reproduces the old static-loader error and
loads the same `libz.so` with dynamic C linkage.
The newer focused static run `38974180411` at `43262a5c0` built and installed
both normal and profiling i686 interpreters after the private-symbol fix. Its
x86_64 static CLI also built. The i686 `wai-app-static` Template Haskell splice
then failed: the runtime linker rejected duplicate `__x86.get_pc_thunk.bx` and
`.ax` definitions before unresolved lookups, a QEMU target SIGSEGV, and proxy
EOF. The scoped COMDAT candidate cross-compiles both changed C files against
real GHC 9.14 headers with the original warning messages unchanged. Its physical
source producer changes exactly `Linker.c`, `linker/Elf.c`, and `linker/Elf.h`.
The runtime regression and final i686 TH/application builds remain unverified,
including profiling execution.
Intel Darwin reproduced its Setup GC abort and produced the direct-abort trace
used for the info-table diagnosis above. The local renderer regression passes,
while the patched compiler bootstrap and full Intel outputs await CI. Windows
@@ -0,0 +1,95 @@
{ pkgs, compiler }:
assert pkgs.stdenv.hostPlatform.isLinux && pkgs.stdenv.hostPlatform.isx86_64;
let
source = pkgs.applyPatches {
name = "ghc914-rts-single-function-comdat";
src = compiler.configured-src + "/rts";
patches = [ ./ghc914-rts-elf-comdat-functions.patch ];
};
in pkgs.runCommandCC "ghc914-elf-comdat-functions-check" {} ''
set -euo pipefail
mkdir -p "$out"
includes=()
for package in rts rts-headers rts-fs libffi-clib; do
${compiler}/bin/ghc-pkg field "$package" include-dirs --simple-output \
> "$out/$package-include-dirs.txt"
for dir in $(cat "$out/$package-include-dirs.txt"); do
includes+=("-I$dir")
done
done
# Use one consistent set of private headers; installed copies also carry
# pragma-once headers which must not be included as distinct source files.
# The upstream RTS uses GNU C extensions. The fixture itself is assembly.
for variant in baseline patched; do
if [ "$variant" = baseline ]; then tree=${compiler.configured-src}/rts; else tree=${source}; fi
mkdir -p "$out/$variant-objects"
for file in linker/Elf.c Linker.c; do
object=$(basename "$file" .c)
$CC -std=gnu11 -Werror=vla -pedantic -Wall -Wextra -O2 -fPIC \
-mno-avx -mno-avx2 -DCOMPILING_RTS -DINTERNAL_LIBFFI -DFS_NAMESPACE=rts \
-iquote "$tree" -iquote "$tree/linker" \
"''${includes[@]}" -I"$tree" \
-c "$tree/$file" -o "$out/$variant-objects/$object.o" \
2>&1 | tee "$out/$variant-$object-compile.log"
done
done
cp ${./elf-comdat/Probe.hs} Probe.hs
for variant in baseline patched; do
${compiler}/bin/ghc -ignore-dot-ghci -package ghci -j1 -O0 Probe.hs \
"$out/$variant-objects/Elf.o" "$out/$variant-objects/Linker.o" \
-o "$out/$variant" -optl-Wl,-Map="$out/$variant.map"
if grep -E '\.a\((Elf|Linker)\.o\)' "$out/$variant.map"; then
echo 'Original RTS linker object was unexpectedly extracted' >&2
exit 1
fi
done
for variant in a b; do
if [ "$variant" = a ]; then value=17; addend=100; else value=29; addend=200; fi
common=(-DPROBE_VALUE=$value -DPROBE_READER=reader_$variant -DPROBE_ADDEND=$addend)
for kind in comdat strong plain-group different-signature weak; do
flags=(-DGROUP_SIGNATURE=comdat_function)
case "$kind" in
strong) flags+=(-DNO_GROUP) ;;
plain-group) flags+=(-DPLAIN_GROUP) ;;
different-signature) flags=(-DGROUP_SIGNATURE=other_signature -DOTHER_SIGNATURE) ;;
weak) flags+=(-DNO_GROUP -DWEAK_FUNCTION) ;;
esac
$CC -c "''${common[@]}" "''${flags[@]}" ${./elf-comdat/fixture.S} \
-o "$out/$kind-$variant.o"
done
$AR rc "$out/comdat-$variant.a" "$out/comdat-$variant.o"
done
$CC -c -DNO_GROUP -DGROUP_SIGNATURE=comdat_function \
-DPROBE_VALUE=41 -DPROBE_READER=reader_strong -DPROBE_ADDEND=300 \
${./elf-comdat/fixture.S} -o "$out/strong-third.o"
expect_failure() {
name=$1; shift
status=0
timeout 15s "$@" > "$out/$name.log" 2>&1 || status=$?
if [ "$status" -ne 1 ]; then
echo "Expected ordinary failure for $name, got status $status" >&2
exit 1
fi
grep -F 'duplicate definition for symbol' "$out/$name.log"
grep -F 'comdat_function' "$out/$name.log"
}
expect_failure baseline-comdat "$out/baseline" "$out/comdat-a.o" "$out/comdat-b.o"
timeout 15s "$out/patched" "$out/comdat-a.o" "$out/comdat-b.o" | tee "$out/comdat.txt"
grep -Fx '[17,117,217]' "$out/comdat.txt"
timeout 15s "$out/patched" "$out/comdat-b.o" "$out/comdat-a.o" | tee "$out/comdat-reversed.txt"
grep -Fx '[29,129,229]' "$out/comdat-reversed.txt"
for executable in baseline patched; do
timeout 15s "$out/$executable" "$out/weak-a.o" "$out/weak-b.o" | tee "$out/$executable-weak.txt"
grep -Fx '[17,117,217]' "$out/$executable-weak.txt"
done
for kind in strong plain-group; do
expect_failure "$kind" "$out/patched" "$out/$kind-a.o" "$out/$kind-b.o"
done
expect_failure different-signature "$out/patched" "$out/different-signature-a.o" "$out/comdat-b.o"
expect_failure mixed-comdat-first "$out/patched" "$out/comdat-a.o" "$out/strong-b.o"
expect_failure mixed-strong-first "$out/patched" "$out/strong-a.o" "$out/comdat-b.o"
expect_failure archive-owner-change "$out/patched" --archive-conflict \
"$out/comdat-a.a" "$out/comdat-b.a" "$out/strong-third.o"
printf '%s\n' 'PASS: scoped COMDAT coalescing preserves relocations and strong conflicts' \
| tee "$out/result.txt"
''
+1
View File
@@ -25,6 +25,7 @@ let
profiling = lib.mkOption { type = lib.types.bool; };
cabalProjectLocal = lib.mkOption { type = lib.types.lines; default = ""; };
src = lib.mkOption { type = lib.types.anything; default = null; };
modules = lib.mkOption { type = lib.types.listOf lib.types.anything; default = []; };
};
config = {
inherit name builderVersion profiling;
+59
View File
@@ -0,0 +1,59 @@
# Use the pinned lib and the real project module; no compiler or project IFD.
{ lib, projectModule ? import ./musl-rts-project.nix }:
let
apply = { musl ? true, i386 ? true, linux ? true,
compiler ? "ghc914-sh", builderVersion ? 2,
name ? "simplex-chat", profiling ? false }:
let
project = lib.evalModules {
modules = [ projectModule {
options = {
name = lib.mkOption { type = lib.types.str; };
profiling = lib.mkOption { type = lib.types.bool; };
compiler-nix-name = lib.mkOption { type = lib.types.str; };
builderVersion = lib.mkOption { type = lib.types.int; };
modules = lib.mkOption { type = lib.types.listOf lib.types.anything; default = []; };
};
config = {
inherit name profiling builderVersion;
compiler-nix-name = compiler;
# Match cabalProject's module fixed point, not specialArgs.
_module.args.pkgs.stdenv = {
hostPlatform = { isMusl = musl; isx86_32 = i386; };
buildPlatform.isLinux = linux;
};
};
} ];
};
package = lib.evalModules {
modules = [ {
options.packages = lib.mkOption {
type = lib.types.attrsOf (lib.types.submodule {
options.patches = lib.mkOption { type = lib.types.listOf lib.types.path; default = []; };
});
default = {};
};
# Existing patches must remain ordered before this one.
config.packages.rts.patches = [ ./musl-iserv-public-libc.patch ];
} ] ++ project.config.modules;
};
in package.config.packages.rts.patches;
unchanged = [ ./musl-iserv-public-libc.patch ];
selected = unchanged ++ [ ./ghc914-rts-elf-comdat-functions.patch ];
in
assert apply {} == selected;
assert apply { name = "iserv-proxy"; } == selected;
assert apply { profiling = true; } == selected;
assert apply { musl = false; } == unchanged;
assert apply { i386 = false; } == unchanged;
assert apply { linux = false; } == unchanged;
assert apply { compiler = "ghc963"; } == unchanged;
assert apply { compiler = "ghc914-sh-stage1"; } == unchanged;
assert apply { builderVersion = 1; } == unchanged;
{
moduleArgumentFixedPointEvaluated = true;
i386MuslApplicationAndInterpreterCovered = true;
profilingCovered = true;
existingPatchOrderPreserved = true;
otherPlatformsCompilersAndBuildersUnchanged = true;
}
+34
View File
@@ -0,0 +1,34 @@
{-# LANGUAGE ForeignFunctionInterface #-}
module Main (main) where
import Control.Monad (unless)
import Foreign.C.Types (CInt (..))
import Foreign.Ptr (FunPtr, castPtrToFunPtr)
import GHCi.ObjLink (ShouldRetainCAFs (RetainCAFs), initObjLinker, loadArchive, loadObj, lookupSymbol, resolveObjs)
import System.Environment (getArgs)
foreign import ccall "dynamic" callInt :: FunPtr (IO CInt) -> IO CInt
main :: IO ()
main = do
arguments <- getArgs
initObjLinker RetainCAFs
case arguments of
[first, second] -> do
mapM_ loadObj [first, second]
resolve
mapM invoke ["comdat_function", "reader_a", "reader_b"] >>= print
["--archive-conflict", first, second, strong] -> do
-- Discovery initially selects the first archive's COMDAT definition.
-- Loading an ordinary strong definition can replace that unused owner.
-- The second archive must not retain stale permission to coalesce.
mapM_ loadArchive [first, second]
loadObj strong
resolve
invoke "reader_b" >>= print
_ -> fail "expected two objects, or --archive-conflict and three paths"
where
resolve = resolveObjs >>= \ok -> unless ok (fail "resolveObjs returned False")
invoke name = lookupSymbol name >>= maybe
(fail (name <> " not found"))
(callInt . castPtrToFunPtr)
+36
View File
@@ -0,0 +1,36 @@
#if defined(NO_GROUP)
.text
#elif defined(PLAIN_GROUP)
.section .text.comdat_function,"axG",@progbits,GROUP_SIGNATURE
#else
.section .text.comdat_function,"axG",@progbits,GROUP_SIGNATURE,comdat
#endif
#if defined(WEAK_FUNCTION)
.weak comdat_function
#else
.globl comdat_function
#endif
.hidden comdat_function
.type comdat_function,@function
comdat_function:
mov $PROBE_VALUE, %eax
ret
.size comdat_function, .-comdat_function
#if defined(OTHER_SIGNATURE)
.globl other_signature
.set other_signature, comdat_function
#endif
.text
.globl PROBE_READER
.type PROBE_READER,@function
PROBE_READER:
sub $8, %rsp
call comdat_function
add $8, %rsp
add $PROBE_ADDEND, %eax
ret
.size PROBE_READER, .-PROBE_READER
.section .note.GNU-stack,"",@progbits
@@ -0,0 +1,124 @@
--- a/linker/Elf.c
+++ b/linker/Elf.c
@@ -796,6 +796,76 @@
}
#endif
-
+
+/* Recognize only a COMDAT group containing one whole hidden function.
+ * GCC's i386 PIC thunks use this form with a GLOBAL, not WEAK, signature.
+ * This is deliberately not general multi-section COMDAT selection: retaining
+ * duplicate initializer, TLS, data, or partial-function sections is unsafe.
+ */
+static bool
+isSingleFunctionComdat_ELF(ObjectCode *oc, const SymbolName *name)
+{
+ if (oc == NULL || oc->type != STATIC_OBJECT || oc->info == NULL
+ || oc->info->elfHeader == NULL || oc->image == NULL
+ || oc->fileSize <= 0) {
+ return false;
+ }
+
+ const Elf_Shdr *shdr = oc->info->sectionHeader;
+ const Elf_Word shnum = elf_shnum(oc->info->elfHeader);
+ const size_t imageSize = (size_t)oc->fileSize;
+ for (Elf_Word i = 0; i < shnum; i++) {
+ const Elf_Shdr *group = &shdr[i];
+ if (group->sh_type != SHT_GROUP
+ || group->sh_size != 2 * sizeof(Elf_Word)
+ || group->sh_offset > imageSize
+ || group->sh_size > imageSize - group->sh_offset) {
+ continue;
+ }
+ Elf_Word entries[2];
+ memcpy(entries, oc->image + group->sh_offset, sizeof(entries));
+ if (entries[0] != GRP_COMDAT || entries[1] == SHN_UNDEF
+ || entries[1] >= shnum) {
+ continue;
+ }
+ const Elf_Shdr *member = &shdr[entries[1]];
+ const Elf_Word required = SHF_ALLOC | SHF_EXECINSTR | SHF_GROUP;
+ if (member->sh_type != SHT_PROGBITS
+ || (member->sh_flags & required) != required
+ || (member->sh_flags & (SHF_WRITE | SHF_TLS)) != 0) {
+ continue;
+ }
+ for (ElfSymbolTable *table = oc->info->symbolTables;
+ table != NULL; table = table->next) {
+ if (table->index != group->sh_link
+ || group->sh_info >= table->n_symbols) {
+ continue;
+ }
+ const ElfSymbol *signature = &table->symbols[group->sh_info];
+ const Elf_Sym *symbol = signature->elf_sym;
+ if (signature->name != NULL && strcmp(signature->name, name) == 0
+ && ELF_ST_BIND(symbol->st_info) == STB_GLOBAL
+ && ELF_ST_TYPE(symbol->st_info) == STT_FUNC
+ && ELF_ST_VISIBILITY(symbol->st_other) == STV_HIDDEN
+ && symbol->st_shndx < SHN_LORESERVE
+ && symbol->st_shndx == entries[1]
+ && symbol->st_value == 0 && symbol->st_size != 0
+ && symbol->st_size == member->sh_size) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
+bool
+ocIsComdatDuplicate_ELF(ObjectCode *oc, const SymbolName *name)
+{
+ RtsSymbolInfo *existing = lookupStrHashTable(symhash, name);
+ return existing != NULL && existing->owner != oc
+ && isSingleFunctionComdat_ELF(oc, name)
+ && isSingleFunctionComdat_ELF(existing->owner, name);
+}
+
int
ocGetNames_ELF ( ObjectCode* oc )
{
@@ -1234,7 +1304,9 @@
setWeakSymbol(oc, nm);
}
if (!ghciInsertSymbolTable(oc->fileName, symhash,
- nm, symbol->addr, isWeak, sym_type, symbol->elf_sym->st_size, oc)
+ nm, symbol->addr,
+ isWeak || ocIsComdatDuplicate_ELF(oc, nm),
+ sym_type, symbol->elf_sym->st_size, oc)
) {
goto fail;
}
--- a/linker/Elf.h
+++ b/linker/Elf.h
@@ -10,6 +10,7 @@
void ocDeinit_ELF ( ObjectCode* oc );
int ocVerifyImage_ELF ( ObjectCode* oc );
int ocGetNames_ELF ( ObjectCode* oc );
+bool ocIsComdatDuplicate_ELF(ObjectCode *oc, const SymbolName *name);
int ocResolve_ELF ( ObjectCode* oc );
int ocRunInit_ELF ( ObjectCode* oc );
int ocRunFini_ELF ( ObjectCode* oc );
--- a/Linker.c
+++ b/Linker.c
@@ -1645,10 +1645,19 @@
Symbol_t symbol;
for (x = 0; x < oc->n_symbols; x++) {
symbol = oc->symbols[x];
+ bool isWeak = isSymbolWeak(oc, symbol.name);
+#if defined(OBJFORMAT_ELF)
+ /* Archive discovery can change the selected owner. Recheck COMDAT
+ * eligibility now; never persist conditional acceptance as weakness.
+ */
+ if (symbol.name != NULL && !isWeak) {
+ isWeak = ocIsComdatDuplicate_ELF(oc, symbol.name);
+ }
+#endif
if ( symbol.name
&& !ghciInsertSymbolTable(oc->fileName, symhash, symbol.name,
symbol.addr,
- isSymbolWeak(oc, symbol.name),
+ isWeak,
symbol.type, 0, oc)) {
return 0;
}
+1 -1
View File
@@ -4,7 +4,7 @@ prev.lib.optionalAttrs (
) {
haskell-nix = prev.haskell-nix // {
cabalProject' = projectModule: prev.haskell-nix.cabalProject' (
[ ({ config, lib, pkgs, ... }:
[ ./musl-rts-project.nix ({ config, lib, pkgs, ... }:
lib.mkIf (
pkgs.stdenv.hostPlatform.isMusl
&& pkgs.stdenv.buildPlatform.isLinux
+17
View File
@@ -0,0 +1,17 @@
{ config, lib, pkgs, ... }:
lib.mkIf (
pkgs.stdenv.hostPlatform.isMusl
&& pkgs.stdenv.hostPlatform.isx86_32
&& pkgs.stdenv.buildPlatform.isLinux
&& config.compiler-nix-name == "ghc914-sh"
&& config.builderVersion == 2
) {
# Cross projects build their target RTS alongside the boot libraries.
# Cover both application and auxiliary interpreter projects, including
# profiling, while keeping the native compiler and its bootstrap intact.
modules = [
({ lib, ... }: {
packages.rts.patches = lib.mkAfter [ ./ghc914-rts-elf-comdat-functions.patch ];
})
];
}