From e218b6978c4c784e33c99c140281dfc78b77b1dc Mon Sep 17 00:00:00 2001 From: Graeme Geldenhuys Date: Fri, 26 Jun 2026 18:02:51 +0100 Subject: [PATCH] fix(rtl,asm): address pre-landing review findings on the static leaf Pre-landing review fixes, all in the direct-syscall / threads-leaf series: - runtime.thread.static.linux + runtime.start.static.linux: reclaim the per-thread TLS block when clone(2) fails after BuildThreadTLS (previously only the stack mapping was unmapped, leaking one TLS mapping per failed pthread_create). Add a symmetric FreeThreadTLS that recovers the mmap base from the thread pointer using the same alignment math BuildThreadTLS used. - runtime.libc2.linux: system() now forwards `environ` to execve instead of nil, so the spawned /bin/sh sees PATH/HOME/etc. (was running with an empty environment under --static). - runtime.libc2.linux: mkstemp loops getrandom until all 6 bytes are filled rather than ignoring a short read that would weaken the random suffix. - runtime.libc.linux: drop execvp's dead HasSlash branch (both arms called the identical execve); keep the single call + the no-PATH-search note. - runtime.thread.static.linux: correct the stale _clone_thread comment that claimed args pass through callee-saved regs (they are seeded on the child stack and popped). - blaise.assembler.x86_64: in the asm brace-comment strip, splice without trimming mid-scan (a left-trim shifted the string under the scan index) and trim once at the end, so a second brace comment on the same line is still found. - blaise.codegen.native.driver: replace a non-ASCII ellipsis in an added comment with '...' (ASCII-only source rule). Verified: FIXPOINT_OK, NATIVE_FIXPOINT_OK, NATIVE_INTERNAL_OK; self-hosted compiler builds clean (0 spurious lock prefixes); static threads binary runs counter=80000 PASS (deterministic across runs); no unit/codegen/assembler/linker test regressions. --- .../main/pascal/blaise.assembler.x86_64.pas | 7 +++-- .../pascal/blaise.codegen.native.driver.pas | 2 +- .../src/main/pascal/runtime.libc.linux.pas | 26 +++++-------------- .../src/main/pascal/runtime.libc2.linux.pas | 18 ++++++++++--- .../pascal/runtime.start.static.linux.pas | 17 ++++++++++++ .../pascal/runtime.thread.static.linux.pas | 8 +++--- 6 files changed, 47 insertions(+), 31 deletions(-) diff --git a/compiler/src/main/pascal/blaise.assembler.x86_64.pas b/compiler/src/main/pascal/blaise.assembler.x86_64.pas index 2212518..93edb81 100644 --- a/compiler/src/main/pascal/blaise.assembler.x86_64.pas +++ b/compiler/src/main/pascal/blaise.assembler.x86_64.pas @@ -679,14 +679,17 @@ begin Q := P + 1; while (Q < Length(S)) and (S[Q] <> 125) do Q := Q + 1; + { Splice out [P..Q] WITHOUT trimming mid-scan (a left-trim would shift the + string under P); resume from P so a following brace is still found. } if Q < Length(S) then - S := TrimStr(Copy(S, 0, P) + ' ' + Copy(S, Q + 1, Length(S))) + S := Copy(S, 0, P) + ' ' + Copy(S, Q + 1, Length(S)) else - S := TrimStr(Copy(S, 0, P)); + S := Copy(S, 0, P); end else P := P + 1; end; + S := TrimStr(S); if Length(S) = 0 then Exit; { Strip trailing comment, but skip over quoted strings so that '#' diff --git a/compiler/src/main/pascal/blaise.codegen.native.driver.pas b/compiler/src/main/pascal/blaise.codegen.native.driver.pas index 658dce5..2ffa773 100644 --- a/compiler/src/main/pascal/blaise.codegen.native.driver.pas +++ b/compiler/src/main/pascal/blaise.codegen.native.driver.pas @@ -229,7 +229,7 @@ begin try try { --static: freestanding non-PIE ET_EXEC, no libc/PT_INTERP (the kernel - leaf supplies open/read/write/… + _start). Default: dynamic PIE + leaf supplies open/read/write/... + _start). Default: dynamic PIE linked against libc. } Lk.SetDynamic(not AOpts.Static); diff --git a/compiler/src/main/pascal/runtime.libc.linux.pas b/compiler/src/main/pascal/runtime.libc.linux.pas index dbec7ac..9a4a1ef 100644 --- a/compiler/src/main/pascal/runtime.libc.linux.pas +++ b/compiler/src/main/pascal/runtime.libc.linux.pas @@ -123,27 +123,13 @@ begin end; function execvp(File_: PChar; Argv: Pointer): Integer; -var - HasSlash: Boolean; - I: Int64; begin - { If File contains a '/', execve it directly; otherwise a PATH search would be - needed. The RTL only execs absolute/relative program paths (system() builds - "/bin/sh"), so direct execve covers the present call sites; a PATH search can - be added when a bare-name exec appears. } - HasSlash := False; - I := 0; - while (File_[I] and $FF) <> 0 do - begin - if (File_[I] and $FF) = Ord('/') then HasSlash := True; - I := I + 1; - end; - if HasSlash then - Result := execve(File_, Argv, environ) - else - { No PATH search yet - try as-is so the failure is the kernel's ENOENT - rather than a silent wrong result. } - Result := execve(File_, Argv, environ); + { No PATH search yet: execve the name directly. The RTL only execs + absolute/relative program paths (system() builds "/bin/sh"), so this covers + the present call sites. A bare program name resolves only relative to the + CWD and otherwise fails with the kernel's ENOENT (never a silent wrong + result); add a $PATH scan here if a bare-name exec call site appears. } + Result := execve(File_, Argv, environ); end; { Population count of a 1024-bit affinity mask (128 bytes), counting set CPUs. } diff --git a/compiler/src/main/pascal/runtime.libc2.linux.pas b/compiler/src/main/pascal/runtime.libc2.linux.pas index 0fe9c27..8d1e601 100644 --- a/compiler/src/main/pascal/runtime.libc2.linux.pas +++ b/compiler/src/main/pascal/runtime.libc2.linux.pas @@ -223,7 +223,8 @@ function mkstemp(Template: PChar): Integer; const ALPHABET = 'abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789'; var - Len, I, Tries: Integer; + Len, I, Tries, Got: Integer; + N: Int64; RandBuf: array[0..5] of Byte; Fd: Integer; begin @@ -233,7 +234,15 @@ begin Tries := 0; while Tries < 256 do begin - getrandom(@RandBuf[0], 6, 0); + { Fill all 6 bytes; getrandom may short-read, so loop until satisfied rather + than leave stale/zero bytes that would weaken the random suffix. } + Got := 0; + while Got < 6 do + begin + N := getrandom(@RandBuf[Got], 6 - Got, 0); + if N <= 0 then Continue; { EINTR / transient: retry } + Got := Got + Integer(N); + end; for I := 0 to 5 do Template[Len - 6 + I] := Ord(ALPHABET[(RandBuf[I] mod 62)]); Fd := open(Template, O_RDWR or O_CREAT or O_EXCL, 384); { 0600 } @@ -264,8 +273,9 @@ begin Pid := fork(); if Pid = 0 then begin - { Child: replace image; on failure exit 127 like the shell. } - execve(PChar(@ShPath[0]), @Argv[0], nil); + { Child: replace image; on failure exit 127 like the shell. Forward the + process environment so the spawned shell sees PATH/HOME/etc. } + execve(PChar(@ShPath[0]), @Argv[0], environ); _exit(127); end; if Pid < 0 then Exit(-1); diff --git a/compiler/src/main/pascal/runtime.start.static.linux.pas b/compiler/src/main/pascal/runtime.start.static.linux.pas index f6004db..a0a5428 100644 --- a/compiler/src/main/pascal/runtime.start.static.linux.pas +++ b/compiler/src/main/pascal/runtime.start.static.linux.pas @@ -49,6 +49,12 @@ var followed by the TCB self-pointer. Returns nil when the program has no TLS. } function BuildThreadTLS: Pointer; +{ Unmap a TLS block created by BuildThreadTLS, given the thread pointer it + returned. Used to reclaim the block when thread creation fails after the TLS + was built (the offset from TP back to the mmap base is the same alignment math + BuildThreadTLS used, kept here so both sides agree). } +procedure FreeThreadTLS(ATp: Pointer); + implementation type @@ -190,6 +196,17 @@ begin Result := Tp; end; +procedure FreeThreadTLS(ATp: Pointer); +var + Block: Pointer; + BlockSize: Int64; +begin + if (ATp = nil) or (GTlsMemSz = 0) then Exit; + Block := Pointer(PChar(ATp) - AlignUp(GTlsMemSz, GTlsAlign)); + BlockSize := AlignUp(GTlsMemSz, GTlsAlign) + 16; + munmap(Block, BlockSize); +end; + { Call the program's `main(argc, argv)` (emitted by the backend) and return its result. The asm thunk tail-jumps to the bare `main` symbol; argc/argv are already in %edi/%rsi (SysV), exactly what main expects. } diff --git a/compiler/src/main/pascal/runtime.thread.static.linux.pas b/compiler/src/main/pascal/runtime.thread.static.linux.pas index 3219f46..9692dcc 100644 --- a/compiler/src/main/pascal/runtime.thread.static.linux.pas +++ b/compiler/src/main/pascal/runtime.thread.static.linux.pas @@ -178,10 +178,9 @@ end; Args (SysV): %rdi=Flags, %rsi=ChildStack(top), %rdx=ParentTid, %rcx=ChildTid, %r8=Tls, %r9=Entry, [stack]=EntryArg. - The new stack is pre-seeded by the caller with Entry and EntryArg so the child - can reach them after clone (the syscall does not propagate registers reliably - to a freshly cloned stack across all kernels - we read them from the stack). - Here we instead pass them through callee-saved regs that survive the syscall. } + The child stack is pre-seeded (below) with Entry and EntryArg so the child can + reach them after clone without relying on register state surviving across the + syscall: the child pops them off its own stack. } function _clone_thread(Flags: Int64; ChildStack, ParentTid, ChildTid, Tls, Entry, EntryArg: Pointer): Int64; assembler; nostackframe; @@ -252,6 +251,7 @@ begin if Tid < 0 then begin munmap(Region, STACK_SIZE); + FreeThreadTLS(Tls); { reclaim the per-thread TLS block too } Result := 11; { EAGAIN } Exit; end;