diff --git a/.gitignore b/.gitignore index 89e22ed94..fba8ae8f0 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 7e074a9b1..ae2774a4d 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 547707555..eca1a9f17 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 55426e2b2..161d66b68 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 000000000..d9845f923 --- /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 000000000..e69de29bb diff --git a/none/tests/linux/stack_layout.stdout.exp b/none/tests/linux/stack_layout.stdout.exp new file mode 100644 index 000000000..0ddda8c79 --- /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 000000000..7741c302d --- /dev/null +++ b/none/tests/linux/stack_layout.vgtest @@ -0,0 +1,2 @@ +prog: stack_layout +vgopts: -q --trace-children=yes