[amd-staging-rocgdb-16] ROCgdb cherry picks from origin/amd-staging (2026-08-10) - #252
Conversation
GLIBC 2.42 changed how thread stack guard pages are implemented [2].
In GLIBC 2.41 and earlier, guard pages were set up using mprotect() to
mark guard regions with no permissions. Once configured, guard pages
were visible as separate entries in /proc/PID/maps with no permissions
(i.e. they're inaccessible). In GLIBC 2.42, guard pages are
installed using the kernel's MADV_GUARD_INSTALL mechanism [1], which
marks them at the page table entry (PTE) level within the existing
mapping.
As a consequence, guard pages do not appear as separate entries in
/proc/PID/maps, but remain as part of the containing mapping. Moreover,
thread stacks from multiple mmap() calls may be merged into a single
virtual memory area (VMA) with read and write permissions since there's
no guard page VMA to separate them. These guard pages cannot be
distinguished by examining VMA listings but do return EIO when read
from /proc/PID/mem.
GDB's gcore code reads /proc/PID/smaps to discover memory regions and
creates one BFD section per mapping. (On linux, this is performed in
linux_find_memory_regions_full in linux-tdep.c.) With the old layout,
memory areas with guard pages appeared separately with no permissions,
which were filtered out. Each thread stack became its own section
containing only readable data. With the new layout, using
MADV_GUARD_INSTALL instead of the older mechanism, it's often the case
that thread stacks created with multiple calls to mmap() are exposed
as a single mapping appearing in /proc/PID/smaps with read and write
permissions. Should that happen, GDB's code creates a single section
covering all thread stacks and their guard pages. (Even if each
thread stack appears in its own mapping, the fact remains that there
will be an inaccessible portion of the mapping. When one or more
thread stacks are coalesced into a single mapping, there will be
several inaccessible "holes" representing the guard pages.)
When gcore_copy_callback copies section contents, it reads memory in
1MB (MAX_COPY_BYTES) chunks. If any page in the chunk is a guard page,
the call to target_read_memory() fails. The old code responded by
breaking out of the copy loop, abandoning the entire section. This
prevents correct copying of thread stack data, resulting in core files
with zero-filled thread stacks, resulting in nearly empty backtraces.
Fix this by falling back to page-by-page reading when a 1MB chunk read
fails. Individual pages that cannot be read are filled with zeros,
allowing the remaining readable memory to be captured.
I also considered a simpler change using the value of
FALLBACK_PAGE_SIZE (4096) as the read size instead of MAX_COPY_BYTES
(1MB). This would avoid the fallback logic but would cause up to 256x
more syscalls. The proposed approach also allows meaningful warnings:
we warn only if an entire region is unreadable (indicating a real
problem), whereas per-page reads would make it harder to distinguish
guard page failures from actual errors. Since guard pages are at
offset 0 for downward-growing stacks, a large target_read_memory()
fails early at the first unreadable byte anyway.
With this fix, I see 16 failures resolved in the following test cases:
gdb.ada/task_switch_in_core.exp
gdb.arch/i386-tls-regs.exp
gdb.threads/threadcrash.exp
gdb.threads/tls-core.exp
Looking at just one of these, from gdb.log without the fix, I see:
thread apply 5 backtrace
Thread 5 (LWP 3414829):
#0 0x00007ffff7d1d982 in __syscall_cancel_arch () from /lib64/libc.so.6
#1 0x0000000000000000 in ?? ()
(gdb) FAIL: gdb.threads/threadcrash.exp: test_gcore: thread apply 5 backtrace
And this is what it looks like with the fix in place (some paths have
been shortened):
thread apply 5 backtrace
Thread 5 (Thread 0x7fffeffff6c0 (LWP 1282651) "threadcrash"):
#0 0x00007ffff7d1d982 in __syscall_cancel_arch () from /lib64/libc.so.6
#1 0x00007ffff7d11c3c in __internal_syscall_cancel () from /lib64/libc.so.6
#2 0x00007ffff7d61b62 in clock_nanosleep@GLIBC_2.2.5 () from /lib64/libc.so.6
#3 0x00007ffff7d6db37 in nanosleep () from /lib64/libc.so.6
#4 0x00007ffff7d8008e in sleep () from /lib64/libc.so.6
#5 0x00000000004006a8 in do_syscall_task (location=NORMAL) at threadcrash.c:158
#6 0x0000000000400885 in thread_function (arg=0x404340) at threadcrash.c:277
#7 0x00007ffff7d15464 in start_thread () from /lib64/libc.so.6
#8 0x00007ffff7d985ac in __clone3 () from /lib64/libc.so.6
(gdb) PASS: gdb.threads/threadcrash.exp: test_live_inferior: thread apply 5 backtrace
Regression testing on Fedora 42 (glibc 2.41) shows no new failures.
The v1 patch used SPARSE_BLOCK_SIZE as the fallback size. While it
was the correct size, it's used for an entirely different purpose
elsewhere in this file. This v2 commit introduces the constant
FALLBACK_PAGE_SIZE instead.
References:
[1] Linux commit 662df3e5c376 ("mm: madvise: implement lightweight
guard page mechanism")
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=662df3e5c37666d6ed75c88098699e070a4b35b5
[2] glibc commit a6fbe36b7f31 ("nptl: Add support for setup guard
pages with MADV_GUARD_INSTALL")
https://sourceware.org/git/?p=glibc.git;a=commit;h=a6fbe36b7f31292981422692236465ab56670ea9
Claude Opus 4.5 and GLM 4.7 assisted with the development of this commit.
Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=33855
Approved-By: Tom de Vries <tdevries@suse.de>
(cherry picked from commit c1da013)
This is a followup patch to commit c1da013, titled "gcore: Handle unreadable pages within readable memory regions". In his review of that earlier patch, Tom de Vries recommended using target_auxv_search with AT_PAGESZ to find the page size if it's available; this patch implements that suggestion. As before, a 4k fallback size is used should the search for an AT_PAGESZ value not succeed. Approved-By: Tom de Vries <tdevries@suse.de> (cherry picked from commit 67732d2)
Replace "skippig" with "skipping" in two places. (cherry picked from commit 76ca162)
When checking values against some Tcl variables, reflect it in the "messages" with a '$'. For example, the following output in gdb.sum: PASS: ...: with debug info: kernel(): blockIdx == group_idx will turn into: PASS: ...: with debug info: kernel(): blockIdx == $group_idx (cherry picked from commit b60bafc)
Add a TOOLCHAINS table mapping toolchain identifiers (gnu, llvm) to their compiler executables and labels. Add --toolchain to select which compiler(s) to run; when omitted all toolchains run in definition order. Only the selected toolchains' executables are required on PATH, so a single-toolchain run does not fail because the other toolchain's compilers are absent. amdclang++ is still required when any gdb.rocm test is in scope, since it is needed to compile GPU kernels regardless of the host compiler. Warn when --output-ignore-list-file is combined with a single toolchain, because the generated list will have no Generic entries and will not suppress those failures in a subsequent two-toolchain run. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit f571893)
Add --timing to record each test's wall-clock duration in one-by-one mode and write rocgdb_timing.log grouped by compiler then directory, sorted from longest to shortest running test. The recorded duration is the maximum wall-clock time of a successful run; tests that ran but never passed are shown as N/A. A --timing-log-file option overrides the default output path. The timing log is written before the optional ignore list so that a write failure for the latter does not discard timing data collected across the full run. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit 6ed2097)
In --one-by-one mode, run a known-good HIP program (built once per run from gdb.rocm/simple.cpp) before each test to confirm the GPU still services a trivial kernel. On timeout or non-zero exit, declare the system unreliable, log the probe output, capture dmesg, and abort the run rather than emitting a wall of spurious failures. The probe executable is rebuilt once per invocation (never cached across runs) so a binary left over for a different GPU cannot trigger a false abort. --sanity-check requires --one-by-one and gdb.rocm tests in scope; --sanity-check-timeout (default 10s) tunes the probe wall clock. dmesg capture falls back to non-interactive sudo and warns when unavailable. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit e640d67)
TheRock commit:
013e3cb9928a -> 2ee541083481
Container images:
therock_build_manylinux_x86_64:
sha256:a382085df3ba... -> sha256:8616d086df21...
(cherry picked from commit 5a19fd1)
Commit 3829f5d ("gdb/testsuite: Extend gdb.rocm/runtime-core.exp with piped GPU coredumps") added a "pipe" output iteration to runtime-core.exp, but the test never actually exercised the pipe path due to two bugs: 1. The call site in runtime-core.exp passes use_pipe as the 4th positional argument to rocm_core_find: rocm_core_find $::binfile {} $fault $use_pipe but the proc's 4th parameter is output_file, with use_pipe being the 5th: proc rocm_core_find {binfile {deletefiles {}} {arg ""} \ {output_file "/dev/null"} {use_pipe false}} As a result, $use_pipe was bound to output_file, and use_pipe defaulted to false. The "pipe" iteration silently ran the same code as the "file" iteration. 2. With (1) fixed, the pipe coredump pattern "|tee \$coredir/\$binfile" writes the GPU core to a file named after the binary, but the subsequent search uses "glob gpucore.*" which does not match. The pipe iteration would therefore fail to locate the GPU core and return "", producing UNTESTED. Fix the call site to pass /dev/null explicitly for output_file, and change the pipe pattern to "|tee \$coredir/gpucore.%p" so the glob finds the resulting file (mirroring the non-pipe pattern). (cherry picked from commit 4ce1c89)
Remove 4 gdb.rocm test entries from the Generic ignore list that are no longer failing and were reported as unused. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit 39b21f5)
663d4f8 to
6a44cb1
Compare
The "identify waves" gdb_test_multiple block used two separate patterns anchored with ^ to match the faulty and auxiliary GPU wave threads from "info thread" output. This is unreliable because the entire output can arrive as a single chunk, in which case ^ only anchors to the very start of the buffer, causing both patterns to fail. Even without the anchor, using two competing patterns causes the first-listed pattern to greedily consume past the line intended for the second. Fix this by using a single pattern that matches any AMDGPU Wave line and classifies it in Tcl. The leading "*" in the thread list identifies the selected (faulty) wave, and the quoted thread name identifies the auxiliary wave. This is robust regardless of how the output is chunked on delivery. While at it, remove the now-unused fault_loc variable and simplify the gpusig conditional, and fix a typo in the do_test comment. (cherry picked from commit 65eee17)
The omp-rocm build path in gdb_compile prunes a benign ROCm device linker warning (ld.lld: warning: ... __keep_alive ... local memory global used by non-kernel function) so the OpenMP offload testcases are not wrongly marked UNTESTED. The prune is anchored on the literal "ld.lld: warning:". With newer toolchains (e.g. LLVM 23), ld.lld's --color-diagnostics defaults to "auto" and colorizes when spawned under a pty (as DejaGnu does), inserting ANSI escapes between "ld.lld:" and "warning:" that stop the anchored regexp from matching. The warning then survives, gdb_compile treats it as a build failure, and the testcase is skipped. Pass -Xoffload-linker --no-color-diagnostics on the link step so the device (offload) linker never colorizes its diagnostics. It is added only when linking, so the compile step does not warn about an unused argument; -Wl, would only reach the host linker, not the device ld.lld that emits this warning. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit ff03066)
9c83e56 to
a1be307
Compare
|
Updated. gdb.rocm/runtime-core.exp needed an adjustment due to upstream drift compared to amd-staging-rocgdb-16. |
|
I'll hold off merging this one as we seem to have gdb.rocm/runtime-core.exp showing a very different pattern on gfx90a where the current test would fail: #257 |
alu-exceptions.exp: The precise=off test variant single-steps through
device code looking for a SIGFPE. Single-stepping is not reliable on
optimized device code, same as many other gdb.rocm tests. Add
require no_optimized_code to skip for optimized builds.
hip-catch-errors.cpp/exp: Three issues with -O3:
- The break marker "Break after reference initialization" was a
comment with no associated code. Under optimization the breakpoint
resolved past the end of the block and the program exited before
hitting it. Add asm volatile ("" ::: "memory") to give the
compiler something to anchor the breakpoint to.
- The hiperr_params_ref::no field was kept in a register by the
optimizer and never stored back to memory before the breakpoint,
reading as 0. Make it volatile to force the store.
- The __device_stub__kernel frame is inlined into its caller at -O3
and disappears from the backtrace. Drop it from the required
backtrace pattern; hipLaunchKernel being present is sufficient.
(cherry picked from commit 82df7bd)
|
Updated with a few more picks. Under test. |
It has been pointed out that HIP error "parameters" is not a suitable term. Nothing is being configured, only more information about the error is being handled. (cherry picked from commit 48d3e1a)
The test fails if any optimization is used. The problems are: - the reference values getting removed. - a failed breakpoint on a comment line at the end of a block. - lack of __device_stub__kernel in call stack. Fix these by: - using volatile qualifier. - adding a volatile NOP assembly instruction at the end of the block. - removing __device_stub__kernel from pattern matching. Rest of the pattern with __hipOnError and hipLaunchKernel is good enough. (cherry picked from commit edd6662)
Setting "locspec" during hiperr_catchpoint construction had the undesired side effect of having an "original_location" field in "*stopped" record of MI [1]. This field is reserved for cases when _user_ inserts a breakpoint. This patch, instead, uses a local version of "locspec" during the symbol lookup in "re_set ()" method and then discards it. This way, there's no "original_location" in "*stopped" record. [1] -break-info Co-authored-by: Pedro Alves <pedro@palves.net> (cherry picked from commit 1ca9be7)
Still looks reasonable. If tests are conclusive, we can merge that. |
I need to verify the hip-catch-err cherry-picks, as they were entangled with some shadow stack bits from upstream. |
869a9a1 to
6619205
Compare
Introduce a new MI command to catch HIP runtime errors that takes the form of: -catch-hiperr [ -c <condition> ] [ -t ] Along with it the documentation is updated and new tests are added. Co-authored-by: Pedro Alves <pedro@palves.net> (cherry picked from commit f00ee4b)
On gfx90a, ttmps are not initialized by default, which means the thread name can be absent from the AMDGPU Wave line in "info thread" output. The previous pattern required a quoted thread name field and matched the auxiliary wave by checking for the string "aux_kernel" in that field. Fix this by capturing the rest of the AMDGPU Wave line and using a regexp that matches the function call aux_kernel () in the frame description. (cherry picked from commit 97ad699)
6619205 to
f863c26
Compare
|
Fixed up gdb.rocm/hip-catch-errors.exp, which required an adjustment to the unknown option test pattern. Turns out upstream tweaked things to replace single quotes to double quotes in some messages. |
|
Validated on gfx90a, gfx1201 and gfx942. |
The compiler emits unsigned long instead of int for the firstprivate argument variable (AIROCGDB-649), causing GDB to print the wrong type. The test may pass or fail depending on the surrounding values/memory. (cherry picked from commit d977686)
Adds a workflow intended to run twice daily (03:17 and 15:17 UTC) to bump THEROCK_COMMIT_REF and the therock_build_manylinux_x86_64 container digest in therock-ci-linux.yml. If an open PR labelled 'therock-deps' already exists, the run skips cleanly. The schedule is currently commented out for testing; the workflow can be triggered manually via workflow_dispatch in the meantime. The build container digest is fetched via the OCI Distribution API using Python's stdlib urllib, avoiding any external tool dependency. The TheRock commit is resolved against refs/heads/main explicitly rather than relying on HEAD. Co-Authored-By: Claude <noreply@anthropic.com> (cherry picked from commit e5b006d)
Commits:
cff819c github: add daily TheRock dependency update workflow
f215dd6 gdb.rocm/omp-target-basic: xfail firstprivate value test
f863c26 gdb/testsuite/runtime-core: Fix wave identification for gfx90a
3cdd39d gdb: add -catch-hiperr to the Machine Interface (MI)
4dc955a hiperr_catchpoint: stop setting "locspec" during construction
84ee926 gdb.rocm/hip-catch-errors: make the test optimization tolerant
2960487 catch hiperr: turn "parameters" wordings into "information"
f3e3e92 [gdb/testsuite] Fix gdb.rocm tests for optimized builds
a1be307 gdb/testsuite: disable device ld.lld color diagnostics for omp-rocm
ed5a060 gdb/testsuite/runtime-core: Fix unreliable wave identification
398928e [ROCgdb] Update ignore list: remove unused gdb.rocm entries
cc29299 gdb/testsuite: fix pipe iteration of gdb.rocm/runtime-core.exp
f2e2513 Update TheRock dependencies and container images
0d982f6 test_rocgdb: add --sanity-check GPU health probe
d2f6403 test_rocgdb: add --timing mode
99471be test_rocgdb: add --toolchain mode
7dd7567 gdb.rocm/hip-builtin-variables: add '$' for Tcl vars in messages
8c16d5b gdb, testsuite: fix typo "skippig"
defcf6e gcore: Query auxv for AT_PAGESZ in gcore_copy_callback
435070d gcore: Handle unreadable pages within readable memory regions