[test-suite] Add musttail runtime regression tests for indirect args#410
Draft
xroche wants to merge 2 commits into
Draft
[test-suite] Add musttail runtime regression tests for indirect args#410xroche wants to merge 2 commits into
xroche wants to merge 2 commits into
Conversation
Seven small C programs under SingleSource/Regression/C/musttail/, one per failure mode that surfaced during review of llvm/llvm-project#185094: forwarded indirect arg, computed indirect arg, swapped args, cross-basic-block, stack-spill, sret, and mixed indirect + stack-spill. They share a header that defines a 256-bit bigint, which is the smallest struct guaranteed to land on the indirect-arg path on RISC-V (RV32 and RV64), AArch64, and x86_64. These are runtime counterparts to the asm-FileCheck musttail- indirect-args.ll in llvm/test/CodeGen/RISCV/. An earlier iteration of llvm/llvm-project#185094 produced asm that matched the expected shape and still dangled at runtime; a local QEMU harness caught it where the in-tree asm tests did not. That harness was for the byval path; these are the indirect-arg siblings, ported to the venue llvm-test-suite already maintains for this kind of coverage. The tests are gated on Clang in the new CMakeLists.txt (CMAKE_C_COMPILER_ID STREQUAL "Clang") because GCC does not implement __attribute__((musttail)). One added add_subdirectory line in the parent SingleSource/Regression/C/CMakeLists.txt wires the new subdirectory in. No source-tree changes outside the test suite. Assisted-by: Claude (Anthropic) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Author
|
Pausing this PR. On closer look only 2 of the 7 tests (sret-musttail, mixed-indirect-spill) actually distinguish pre- and post-merge clang. The other 5 pass on both: PR llvm/llvm-project#185094 fixes the backend's That byval-temp is itself a real dangling-pointer bug on RV64 and AArch64 (covered by llvm/llvm-project#190429, #56435, #72555). The Clang fix is at llvm/llvm-project#199351; this PR is on hold until that lands, then I'll add the stack-clobber probe and revisit. Assisted-by: Claude (Anthropic) |
… local sources Five C runtime tests under SingleSource/Regression/C/musttail/ that expose the by-value-parameter-storage class of bug for musttail calls passing struct args indirect. Each test fails on fork-clang on aarch64, riscv64, and s390x at -O2/-O3 (the test-suite's typical opt level), and most fail at -O0 as well; -O1 is generally hidden by optimizer ABI assumptions. - alias-distinct-addresses.c -- C(a, a). Write to x, read y via volatile. If aliased, the write changes y's contents. - triple-alias.c -- C(a, a, a). Same probe for two-of-three aliasing. - local-source.c -- C(local). Local computed from a volatile seed to defeat constant folding. Catches the byval-temp dangle. - mixed-sources.c -- C(local, a). Exercises both phases of the two-phase emit in one call. - swap-alias.c -- C(b, a, a). Combines reorder with aliasing; would catch a fix that handles each separately but mishandles their interaction. The probes use write-then-read-via-volatile so the optimizer cannot reuse prior register values or assume noalias by ABI. Each test returns a distinct rc per failure mode for triage. Companion to llvm/llvm-project#199351.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Twelve C programs under
SingleSource/Regression/C/musttail/for__attribute__((clang::musttail))calls with struct-by-value arguments. The bug class: a tail-called function reads its arguments from caller-frame memory that the tail call has already deallocated. Static checks (assembly, IR shape) look right; only runtime execution diverges.Argument values move through indirect pointers when the struct is too large for register passing. A normal call leaves the caller's frame alive; a musttail deallocates it before the callee runs. If the indirect pointer pointed into that frame, the callee reads freed memory.
The original seven tests target the bug class addressed by llvm/llvm-project#185094 (RISC-V backend): forwarded indirect, computed indirect, swapped args, cross-basic-block musttail, stack-spill, sret, mixed indirect + spill.
The five new tests target the broader class addressed by llvm/llvm-project#199351 (Clang frontend two-phase rewrite). Each uses a write-then-read-via-volatile probe to defeat the optimizer's noalias-by-ABI assumption that hides the bug at -O1:
alias-distinct-addresses--C(a, a): per-slot storage must be distinct.triple-alias-- generalization to three slots.local-source--C(local): the source dangles unless routed through an incoming pointer.mixed-sources--C(local, a): both copy paths in one call.swap-alias--C(b, a, a): reorder combined with aliasing.Cross-arch runtime validation: each new test fails with pre-#199351 clang on aarch64, riscv64, and s390x at -O2 and -O3 (the test-suite's typical optimization levels), and passes with the patched clang.
Gated on Clang (GCC has no
__attribute__((musttail))). No changes outside the test suite.Assisted-by: Claude (Anthropic)