From 921efb95e655378ca13806caca675031fb25e754 Mon Sep 17 00:00:00 2001 From: not-matthias Date: Tue, 20 Oct 2026 12:37:07 +0200 Subject: [PATCH] fix(initimg): stabilize Linux startup stack layout Reserve a fixed 16 MiB startup region so argument and environment sizes do not move the initial client stack pointer. Preserve startup data and account for the region separately from the runtime stack limit. --- .gitignore | 1 + CODSPEED-CHANGELOG.md | 5 + coregrind/m_initimg/initimg-linux.c | 117 ++++++++++------ none/tests/linux/Makefile.am | 2 + none/tests/linux/stack_layout.c | 164 +++++++++++++++++++++++ none/tests/linux/stack_layout.stderr.exp | 0 none/tests/linux/stack_layout.stdout.exp | 1 + none/tests/linux/stack_layout.vgtest | 2 + 8 files changed, 254 insertions(+), 38 deletions(-) create mode 100644 none/tests/linux/stack_layout.c create mode 100644 none/tests/linux/stack_layout.stderr.exp create mode 100644 none/tests/linux/stack_layout.stdout.exp create mode 100644 none/tests/linux/stack_layout.vgtest diff --git a/.gitignore b/.gitignore index 89e22ed946..fba8ae8f01 100644 --- a/.gitignore +++ b/.gitignore @@ -1929,6 +1929,7 @@ /none/tests/linux/mremap6 /none/tests/linux/open_client /none/tests/linux/pthread-stack +/none/tests/linux/stack_layout /none/tests/linux/stack-overflow /none/tests/linux/getdents_filter diff --git a/CODSPEED-CHANGELOG.md b/CODSPEED-CHANGELOG.md index 7e074a9b15..ae2774a4d0 100644 --- a/CODSPEED-CHANGELOG.md +++ b/CODSPEED-CHANGELOG.md @@ -4,6 +4,11 @@ This file documents changes made to Valgrind for CodSpeed integration, beyond th ## Features +### Fixed Linux startup stack layout + +Reduce simulated cache-miss variation from argument and environment sizes +by keeping the Linux initial stack pointer at a fixed offset from the stack top. + ### Skip `.plt.sec` in `--skip-plt` **Feature**: Added support for skipping `.plt.sec` sections when using the `--skip-plt` option. diff --git a/coregrind/m_initimg/initimg-linux.c b/coregrind/m_initimg/initimg-linux.c index 5477075555..eca1a9f17f 100644 --- a/coregrind/m_initimg/initimg-linux.c +++ b/coregrind/m_initimg/initimg-linux.c @@ -312,37 +312,49 @@ static HChar *copy_str(HChar **tab, const HChar *str) /* ---------------------------------------------------------------- - - This sets up the client's initial stack, containing the args, - environment and aux vector. - - The format of the stack is: - - higher address +-----------------+ <- clstack_end - | | - : string table : - | | - +-----------------+ - | AT_NULL | - - - - | auxv | - +-----------------+ - | NULL | - - - - | envp | - +-----------------+ - | NULL | - - - - | argv | - +-----------------+ - | argc | - lower address +-----------------+ <- sp - | undefined | - : : - - Allocate and create the initial client stack. It is allocated down - from clstack_end, which was previously determined by the address - space manager. The returned value is the SP value for the client. + + This sets up the client's initial stack, containing the args, + environment and aux vector. + + The startup image occupies a fixed-size region directly below + clstack_end, so the initial SP does not depend on the number or size + of arguments, environment variables or auxiliary strings. Tables + grow upward from that fixed SP; strings are placed downward from + clstack_end. The runtime stack starts in the page below SP and grows + into the reservation further down. + + The format of the startup image is: + + higher address +-----------------+ <- clstack_end + | | + : string table : + | | + +-----------------+ + | AT_NULL | + - - + | auxv | + +-----------------+ + | NULL | + - - + | envp | + +-----------------+ + | NULL | + - - + | argv | + +-----------------+ + | argc | + +-----------------+ <- sp (fixed) + lower address | runtime stack | + : : + + The startup image has a fixed capacity so that later mappings and the + runtime stack remain at stable addresses. Linux limits the aggregate + argument/environment size to well below this capacity; inputs that do + not fit are rejected instead of silently moving SP. + + Allocate and create the initial client stack. It is allocated down + from clstack_end, which was previously determined by the address + space manager. The returned value is the SP value for the client. The client's auxv is created by copying and modifying our own one. As a side effect of scanning our own auxv, some important bits of @@ -353,6 +365,10 @@ static HChar *copy_str(HChar **tab, const HChar *str) ---------------------------------------------------------------- */ +/* Since Linux 4.13, execve() limits argv/envp strings plus their pointer + arrays to 6 MiB (3/4 of _STK_LIM). */ +#define STARTUP_REGION_SZB (8 * 1024 * 1024) + struct auxv { Word a_type; @@ -487,15 +503,30 @@ Addr setup_client_stack( void* init_sp, if (0) VG_(printf)("stacksize = %u\n", stacksize); - /* client_SP is the client's stack pointer */ - client_SP = clstack_end - stacksize; - client_SP = VG_ROUNDDN(client_SP, 16); /* make stack 16 byte aligned */ + /* client_SP is fixed so that unused startup data cannot change the + addresses used by the client. The tables grow upward from it and + the strings are placed downward from clstack_end. */ + client_SP = clstack_end + 1 - STARTUP_REGION_SZB; + vg_assert(VG_IS_PAGE_ALIGNED(client_SP)); + + if (stacksize > STARTUP_REGION_SZB) { + VG_(printf)("valgrind: " + "The client arguments and environment need %u bytes, " + "exceeding the fixed %u-byte startup region.\n", + stacksize, (unsigned)STARTUP_REGION_SZB); + VG_(printf)("valgrind: " + "Please report this at " + "https://github.com/CodSpeedHQ/valgrind-codspeed/issues.\n\n"); + VG_(exit)(1); + } /* base of the string table (aligned) */ - stringbase = strtab = (HChar *)clstack_end - - VG_ROUNDUP(stringsize, sizeof(int)); + stringbase = strtab = (HChar *)(clstack_end + 1) + - VG_ROUNDUP(stringsize, sizeof(Word)); - clstack_start = VG_PGROUNDDN(client_SP); + /* Keep the page below the fixed SP mapped so that the first stack + writes do not fault before stack growth is set up. */ + clstack_start = client_SP - VKI_PAGE_SIZE; /* The max stack size */ clstack_max_size = VG_PGROUNDUP(clstack_max_size); @@ -966,6 +997,9 @@ Addr setup_client_stack( void* init_sp, /* client_SP is pointing at client's argc/argv */ + vg_assert((Addr)(auxv + 1) <= (Addr)stringbase); + vg_assert((Addr)strtab <= clstack_end + 1); + if (0) VG_(printf)("startup SP = %#lx\n", client_SP); return client_SP; } @@ -1125,11 +1159,18 @@ IIFinaliseImageInfo VG_(ii_create_image)( IICreateImageInfo iicii, if (szB > m16) szB = m16; if (VG_(clo_main_stacksize) > 0) szB = VG_(clo_main_stacksize); if (szB < m1) szB = m1; + if (szB > (SizeT)-1 - STARTUP_REGION_SZB - (VKI_PAGE_SIZE - 1)) { + VG_(printf)("valgrind: " + "The requested --main-stacksize is too large.\n"); + VG_(exit)(1); + } szB = VG_PGROUNDUP(szB); VG_(debugLog)(1, "initimg", "Setup client stack: size will be %lu\n", szB); - iifii.clstack_max_size = szB; + /* The startup image sits above the fixed initial SP, so account + for it separately from the runtime stack limit. */ + iifii.clstack_max_size = szB + STARTUP_REGION_SZB; iifii.initial_client_SP = setup_client_stack( init_sp, env, diff --git a/none/tests/linux/Makefile.am b/none/tests/linux/Makefile.am index 55426e2b26..161d66b681 100644 --- a/none/tests/linux/Makefile.am +++ b/none/tests/linux/Makefile.am @@ -21,6 +21,7 @@ EXTRA_DIST = \ mremap6.stderr.exp mremap6.vgtest \ open_client.stderr.exp open_client.vgtest \ pthread-stack.stderr.exp pthread-stack.vgtest \ + stack_layout.stderr.exp stack_layout.stdout.exp stack_layout.vgtest \ stack-overflow.stderr.exp stack-overflow.vgtest check_PROGRAMS = \ @@ -37,6 +38,7 @@ check_PROGRAMS = \ mremap5 \ mremap6 \ pthread-stack \ + stack_layout \ stack-overflow if HAVE_OPENAT2 diff --git a/none/tests/linux/stack_layout.c b/none/tests/linux/stack_layout.c new file mode 100644 index 0000000000..d9845f9235 --- /dev/null +++ b/none/tests/linux/stack_layout.c @@ -0,0 +1,164 @@ +#include +#include +#include +#include +#include +#include +#include +#include + +enum { PROBE_ARGC = 3, MAX_VALUES = 8, MAX_LENGTH = 4096, NAME_SIZE = 32 }; + +static const struct { + int argc, arglen, envc, envlen; +} cases[] = { + { 0, 0, 0, 0 }, + { 1, 0, 0, 0 }, + { 1, 15, 0, 0 }, + { 1, 16, 0, 0 }, + { 1, MAX_LENGTH, 0, 0 }, + { MAX_VALUES, 0, 0, 0 }, + { 0, 0, 1, 0 }, + { 0, 0, 1, 15 }, + { 0, 0, 1, 16 }, + { 0, 0, 1, MAX_LENGTH }, + { 0, 0, MAX_VALUES, 0 }, + { MAX_VALUES, MAX_LENGTH, MAX_VALUES, MAX_LENGTH }, +}; + +static const char env_prefix[] = "STACK_LAYOUT_"; +static const char executable[] = "./stack_layout"; + +static void fill_value(char *value, int length, int index) +{ + static const char pattern[] = "a = Z"; + int i; + for (i = 0; i < length; i++) + value[i] = pattern[(i + index) % (sizeof(pattern) - 1)]; + value[length] = '\0'; +} + +static void probe(int argc, char **argv, char **envp) +{ + int c = atoi(argv[2]); + int i, found = 0; + char expected[MAX_LENGTH + 1]; + char name[NAME_SIZE]; + uintptr_t initial_sp, start, end, sp_offset; + FILE *maps; + + assert(c >= 0 && c < sizeof(cases) / sizeof(cases[0])); + assert(argc == PROBE_ARGC + cases[c].argc); + assert(strcmp(argv[0], executable) == 0); + assert(argv[argc] == NULL); + for (i = 0; i < cases[c].argc; i++) { + fill_value(expected, cases[c].arglen, i); + assert(strcmp(argv[PROBE_ARGC + i], expected) == 0); + } + assert(getenv("LC_ALL") && strcmp(getenv("LC_ALL"), "C") == 0); + for (i = 0; envp[i]; i++) { + const char *value; + if (strncmp(envp[i], env_prefix, sizeof(env_prefix) - 1) != 0) + continue; + assert(found < cases[c].envc); + snprintf(name, sizeof(name), "%s%d", env_prefix, found); + fill_value(expected, cases[c].envlen, found); + assert(strncmp(envp[i], name, strlen(name)) == 0); + assert(envp[i][strlen(name)] == '='); + assert(strcmp(envp[i] + strlen(name) + 1, expected) == 0); + value = getenv(name); + assert(value && strcmp(value, expected) == 0); + found++; + } + assert(found == cases[c].envc); + + /* Linux starts with argc immediately before argv. libc passes the original + argv to main, so this recovers the entry SP without arch-specific asm. */ + initial_sp = (uintptr_t)argv - sizeof(*argv); + /* ASLR can move the stack top on 32-bit and inner builds. */ + maps = fopen("/proc/self/maps", "r"); + assert(maps); + do { + assert(fscanf(maps, "%" SCNxPTR "-%" SCNxPTR "%*[^\n]\n", + &start, &end) == 2); + } while (initial_sp < start || initial_sp >= end); + fclose(maps); + sp_offset = end - initial_sp; + assert(write(STDOUT_FILENO, &sp_offset, sizeof(sp_offset)) + == sizeof(sp_offset)); +} + +static uintptr_t run_case(unsigned c) +{ + char arg_values[MAX_VALUES][MAX_LENGTH + 1]; + char env_values[MAX_VALUES][NAME_SIZE + MAX_LENGTH + 1]; + char *args[PROBE_ARGC + MAX_VALUES + 1]; + char *env[MAX_VALUES + 2]; + char case_number[NAME_SIZE]; + int i, fds[2], status; + pid_t child; + FILE *output; + uintptr_t sp_offset; + + snprintf(case_number, sizeof(case_number), "%02u", c); + args[0] = (char *)executable; + args[1] = "--probe"; + args[2] = case_number; + for (i = 0; i < cases[c].argc; i++) { + fill_value(arg_values[i], cases[c].arglen, i); + args[PROBE_ARGC + i] = arg_values[i]; + } + args[PROBE_ARGC + cases[c].argc] = NULL; + env[0] = "LC_ALL=C"; + for (i = 0; i < cases[c].envc; i++) { + int prefix_length = snprintf(env_values[i], NAME_SIZE, "%s%d=", + env_prefix, i); + fill_value(env_values[i] + prefix_length, cases[c].envlen, i); + env[i + 1] = env_values[i]; + } + env[cases[c].envc + 1] = NULL; + + assert(pipe(fds) == 0); + child = fork(); + assert(child >= 0); + if (child == 0) { + close(fds[0]); + assert(dup2(fds[1], STDOUT_FILENO) == STDOUT_FILENO); + close(fds[1]); + execve(executable, args, env); + perror("execve"); + _exit(1); + } + close(fds[1]); + output = fdopen(fds[0], "r"); + assert(output); + assert(fread(&sp_offset, sizeof(sp_offset), 1, output) == 1); + assert(fgetc(output) == EOF); + fclose(output); + assert(waitpid(child, &status, 0) == child); + assert(WIFEXITED(status) && WEXITSTATUS(status) == 0); + return sp_offset; +} + +int main(int argc, char **argv, char **envp) +{ + uintptr_t sp_offset; + unsigned c; + + if (argc >= PROBE_ARGC && strcmp(argv[1], "--probe") == 0) { + probe(argc, argv, envp); + return 0; + } + assert(argc == 1); + sp_offset = run_case(0); + for (c = 1; c < sizeof(cases) / sizeof(cases[0]); c++) { + uintptr_t actual = run_case(c); + if (actual != sp_offset) { + fprintf(stderr, "case %u: initial SP offset changed from %#" PRIxPTR + " to %#" PRIxPTR "\n", c, sp_offset, actual); + return 1; + } + } + puts("Initial SP offset is fixed; arguments and environment are unchanged."); + return 0; +} diff --git a/none/tests/linux/stack_layout.stderr.exp b/none/tests/linux/stack_layout.stderr.exp new file mode 100644 index 0000000000..e69de29bb2 diff --git a/none/tests/linux/stack_layout.stdout.exp b/none/tests/linux/stack_layout.stdout.exp new file mode 100644 index 0000000000..0ddda8c79c --- /dev/null +++ b/none/tests/linux/stack_layout.stdout.exp @@ -0,0 +1 @@ +Initial SP offset is fixed; arguments and environment are unchanged. diff --git a/none/tests/linux/stack_layout.vgtest b/none/tests/linux/stack_layout.vgtest new file mode 100644 index 0000000000..7741c302d5 --- /dev/null +++ b/none/tests/linux/stack_layout.vgtest @@ -0,0 +1,2 @@ +prog: stack_layout +vgopts: -q --trace-children=yes