Report startstack in procfs stat#227
Conversation
Record the initial guest user stack pointer and expose it as field 28 in /proc/self/stat and /proc/self/task/<tid>/stat. Carry the value across fork IPC and bump the private protocol magic so mixed parent/child builds reject the incompatible header layout. Add procfs regression coverage to ensure startstack points inside the reported [stack] VMA. Fix sysprog21#226
There was a problem hiding this comment.
The core change is correct (field 28 lands in the right column in both emitters, fallback keeps 16-byte SP alignment, fork-IPC struct insertion is 8-byte aligned, all four stack-establishing paths set start_stack, and the ELFM->ELFN magic bump rejects mixed builds).
| } | ||
| } | ||
|
|
||
| TEST("procfs: stat startstack is in [stack]"); |
There was a problem hiding this comment.
This test (and the task-stat one below) can pass vacuously. It only asserts startstack lies inside [stack], but proc_start_stack() falls back to stack_top - 16 when g->start_stack == 0, and that fallback is also inside [stack]. So if start_stack propagation ever breaks (left 0 on some path), the membership check still passes and the regression goes undetected. The test does catch a mis-placed stat field (that would read a neighboring literal 0), but not a lost value. Consider asserting the reported value differs from the stack_top-16 fallback, or comparing against an independently derived SP.
| @@ -1678,6 +1679,7 @@ int64_t sys_clone(hv_vcpu_t vcpu, | |||
| .brk_current = g->brk_current, | |||
There was a problem hiding this comment.
This new wire-format field is the riskiest part of the PR, but nothing tests that it survives the fork IPC round-trip. test-fork-ipc-protocol-host.c only static-asserts the magic bumped to ELFN; no test forks a child and reads the child's /proc/self/stat field 28 to confirm start_stack packs/unpacks at the right offset. A fork→child field-28 check would close this.
| } | ||
|
|
||
| if (!strcmp(endp, "/stat")) { | ||
| uint64_t start_stack = proc_start_stack(g); |
There was a problem hiding this comment.
/proc/self/task/<tid>/stat reports the process-wide g->start_stack for every tid, whereas real Linux reports each thread's own stack start. Harmless today (single guest_t, no per-thread stack tracking, the test only exercises the main tid), but a latent gap for CLONE_VM threads reading a non-main tid's field 28.
| TEST("procfs: stat startstack is in [stack]"); | ||
| { | ||
| char statbuf[1024]; | ||
| char mapsbuf[4096]; |
There was a problem hiding this comment.
mapsbuf[4096] truncates silently at bufsz-1. [stack] is typically the last VMA in /proc/self/maps, so if the emitted maps ever exceed 4095 bytes the [stack] line is cut and parse_stack_range returns false → spurious test failure. This fails safe (won't mask a bug), just a latent flakiness source as the emulated region set grows.
| p++; | ||
| while (*p != '\0' && field <= target) { | ||
| char *endp; | ||
| if (field == 3) { |
There was a problem hiding this comment.
parse_proc_stat_field_u64 special-cases field 3 (endp = p+1, no field == target check), so it can't return field 3 and assumes state is exactly one character. Not exercised here (target is always 28), so no live impact — flagging only as a latent constraint if this helper gets reused.
Record the initial guest user stack pointer and expose it as field 28 in /proc/self/stat and /proc/self/task//stat.
Carry the value across fork IPC and bump the private protocol magic so mixed parent/child builds reject the incompatible header layout.
Add procfs regression coverage to ensure startstack points inside the reported [stack] VMA.
Fix #226
Summary by cubic
Expose the initial user stack pointer as startstack (field 28) in /proc/self/stat and /proc/self/task//stat. Persist and validate it across exec and fork; adds tests to ensure it points inside the [stack] VMA. Fixes #226.
New Features
start_stackduring bootstrap and exec, and emit it as field 28 in procfs stat.start_stackthrough fork IPC; bumpFORK_IPC_PROTOCOL_MAGICto ELFN to reject mixed builds.Migration
Written for commit 997776c. Summary will update on new commits.