Repository navigation
Conversation
Valgrind never passes all of its own regression tests, and which tests fail depends on the OS, gdb/debuginfo, and sometimes the CPU. check-regtest.sh runs "make test", extracts the failing tests, and fails only on a failure not listed in regtest-expected/<os>.txt (or if the tests did not run). Each error from the post-test checks is its own entry. "?" marks a test that may or may not fail; --update regenerates the list. New CI jobs regtest_ubuntu_26_04 and regtest_rocky_10 run it with --report-only and upload the observed failures, so that the expected lists can be bootstrapped from CI; remove --report-only once they are committed. Also add "make check-regtest". .gitignore: ignore check-regtest.sh output and 45 test binaries from tests added in the Valgrind 3.23 merge; remove a corrupted line that joined two entries (both are also listed separately).
Fix review findings: wrong codespecs#79 example and weak rationale in the ordering section; bfd.h version mismatch and zstd risks; tarball signature verification; --merge=diff3 produces conflict markers, not .rej files; build-product check before generating patches; memcheck files removed upstream; memcheck/Makefile.am changes; make git-update's memcheck output. Use check-regtest.sh instead of comparing regtest logs by eye, and depend on preparatory work (stale upstream files removed, regtest in CI, fixed make git-update, DynComp IROp scan, bfd.h versions recorded). Add a check that a merge leaves no file that upstream deleted, and fix the .gitignore instructions for new test drivers.
The readelf and Valgrind upgrade plans depend on this work: removing stale upstream files, enforcing regtest expected failures in CI, fixing make git-update and generate-patch-files.sh, scanning DynComp for unhandled IROps, updating the merge doc, and recording bfd.h versions.
…RF PRs codespecs#89-codespecs#95 Both upgrades should wait for them: codespecs#89, codespecs#90, codespecs#91, and codespecs#95 change dwarf.c (and codespecs#90/codespecs#95 change mc_main.c and mc_include.h), and codespecs#85-codespecs#88 change dyncomp_translate.c. codespecs#85's --dyncomp-unhandled-ops=warn and codespecs#87's explicit unary handling also change how to find unhandled IROps.
…red failures Also, in the upgrade plans, return to $INV/fjalar before the test steps, and say how to check whether a CI-only regtest failure is an upstream failure. Addresses CodeRabbit review comments.
Since C23 bool is a keyword. Also bool wasn't actually used.
tc17_sembar.c:45:14: error: both 'long' and '_Bool' in declaration specifiers
45 | typedef long bool;
| ^~~~
tc17_sembar.c:45:1: warning: useless type name in empty declaration
45 | typedef long bool;
| ^~~~~~~
…local
Since C23 thread_local is a keyword (thread storage duration).
swapcontext.c:23:16: error: expected '{' before 'thread_local'
23 | typedef struct thread_local {
| ^~~~~~~~~~~~
swapcontext.c:23:16: warning: 'thread_local' is not at beginning of declaration [-Wold-style-declaration]
swapcontext.c:23:16: error: 'thread_local' used with 'typedef'
swapcontext.c:26:3: warning: data definition has no type or storage class
26 | } thread_local_t;
| ^~~~~~~~~~~~~~
GCC15 will turn this warning into an error:
bug234814.c: In function 'main':
bug234814.c:20:18: error: assignment to '__sighandler_t' {aka 'void (*)(int)'} from incompatible pointer type 'void (*)(void)' [-Wincompatible-pointer-types]
20 | sa.sa_handler = mysigbus;
| ^
"make regtest" runs "make check", which validates the manual with xmllint if it is installed. That needs the DocBook DTD and failed in the Rocky Linux CI container, so no test ran.
…l request plan-pre-upgrade.md and the other edits to the upgrade plans move to branch upgrade-plans. This pull request keeps the plan edits that use check-regtest.sh.
…pecs#96 no longer has Drop the plan text that other pull requests now carry (codespecs#98, codespecs#99, codespecs#101) or that codespecs#103 makes obsolete (make git-update).
|
Warning Review limit reached
This review includes 11 billable files and costs up to $2.75. Or wait 37 minutes for your next included review. View limit details
Comment |
This was referenced Oct 17, 2026
Valgrind plan: - Add a "Scope" section (the three merges, and Fjalar's main changes to Valgrind core). - Why one jump; fall back to a hop at 3.25.1. - Work in a new clone; use ./test-kvasir.sh, not make daikon-test, which ignores $DAIKONDIR and builds ../daikon/fjalar. - host_generic_reg_alloc3.c already has Fjalar's spill-area change; keep it, and add it to the doc's "Valgrind modifications" appendix. - Shadow-state alignment and guest-register offsets to recheck. - Hunk-by-hunk checkoff of the mc_translate.c part of memcheck.patch. - Mechanical Iop_ coverage check, including ops that neither Memcheck nor DynComp mentions. - What to look for in NEWS; autotools requirements of the CI images. - No goal-file changes in the Kvasir tests; a plain Memcheck run. - .gitignore has no "Ignore generated test drivers" section. - What to add to the merge doc; commit message. - An alternative 3-way merge for Coregrind, using git add -A -f so that tracked files matching .gitignore are not dropped. - More risks. Readelf plan: - Correct the #if 0 counts (39 and 29); list consumers of the Binutils-derived code; list codespecs#80. - Merge dwarf.c, dwarf.h, and readelf.c with git merge-file, and exclude them from binutils.patch so that they are not patched twice; fallback hops. - Do not add new features. - Work in a new clone; ./test-kvasir.sh, not make daikon-test. - Warning-list baseline; reference diffs for dwarf.h and elfcomm.c. - Checklist of upgrade-sensitive areas; DWARF 4 versus 5 comparison; --fjalar-debug-dump; harvest_* count; ldd / nm -u. - What to add to the merge doc, including removing the stale lines-notbetween sentence; commit message.
- Valgrind plan: build before the baselines, because check-regtest.sh needs a built tree; save the build log with pipefail. - Readelf plan: drop "make clean", which fails in a fresh clone before valgrind/Makefile exists; use pipefail so that a failed build stops. - 3-way merge: require that "git status --porcelain --ignored valgrind" prints nothing, because "git add -A -f" adds ignored files. - Valgrind plan: note that steps 3-6 and 10 still describe patch and make git-update until codespecs#99 and codespecs#103 land. - plan-pre-upgrade.md: codespecs#102 depends on nothing (land it before codespecs#96); item 6 is done. - Readelf plan: record ldd and nm -u output in the baseline, for the later comparison. - Use "land", not "merge", for pull requests. - Readelf plan: combine the two harvest_* bullets. - Valgrind plan: say to install gdb and glibc debuginfo before the regtest baseline commands, not after them. - Rewrap long lines.
Also leave the .gitignore bullet and the baseline-in-a-copy wording to codespecs#101, and drop the note that steps 3-6 and 10 are out of date, because codespecs#99 and codespecs#103 update those steps.
… with git apply - check-regtest.sh: do not discard "post check failed" lines seen before the summary. - Valgrind plan: use apply-patch-files.sh instead of patch and make git-update; move $INV/fjalar aside before linking it; correct the post_regtest_checks entry. - Readelf plan: replace the ldd / nm -u check, which cannot detect anything for a statically linked tool, with an nm check for zstd, SFrame, and debuginfod code. - Pre-upgrade plan: update the reason for check-regtest.sh.
…t_checks output, add --observed, build tests in parallel; fix plan doc references
… all; bound post_regtest_checks output; add --observed; check --log exists; build test programs in parallel
…e --observed and honor --report-only when tests did not run; --update edits in place and keeps file permissions; usage error for missing option arguments; skip xmllint in upstream baseline
…ing failed; full command in the "To create it" hint; LC_ALL=C; reject empty option arguments
…ader hint names the right file; document missing-file failure; normalize whitespace in one place; CI uploads failing tests' .out and .diff files
…g; fail with --report-only if the tests did not run; handle missing VERSION_ID and /etc/os-release; resolve symbolic links in FJALARDIR; ignore failing tests' .out and .diff files
# Conflicts: # check-regtest.sh # plan-upgrade-readelf.md # plan-upgrade-valgrind.md
…tracked files; run the Kvasir baseline once; note that codespecs#105 contains codespecs#96 and codespecs#96 contains codespecs#102; describe check-regtest.sh's post-test-check log and --report-only
…as check-regtest.sh does
# Conflicts: # Makefile
# Conflicts: # Makefile
…h's options; do not overwrite Fjalar's observed failures Without -s --no-print-directory, check-regtest.sh reads make's messages in test-post.log as errors of the post-test checks. Without --observed, the upstream run overwrites regtest-observed-<os>.txt from step 1.
# Conflicts: # plan-upgrade-valgrind.md
…ts of pull requests; remove the 3-way-merge alternative The text that codespecs#98, codespecs#99, codespecs#101, codespecs#103, and codespecs#113 change in the plans is left as in codespecs#96, so that those pull requests' versions apply when they land. plan-pre-upgrade.md points to those pull requests rather than restating them. The "Alternative: a 3-way merge for Coregrind" section is removed, because codespecs#103's apply-patch-files.sh performs the same 3-way merge with git apply --3way.
This branch has not been deployed
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
1 participant
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.
This pull request depends on #96, and it contains its commits. Please land #96 first. After that, this pull request's diff shows only its own change.
It changes no code. It does not depend on the other pull requests that change the plans (#98, #99, #101, #103, #113): wherever they change the plans, this pull request leaves #96's text, so that their text applies when they land. The only conflicts with them are those that #96 already has: in steps 3 and 10 of
plan-upgrade-valgrind.md, and in.gitignore. It can land at any time after #96.This was split out of #96, which contains the regtest CI and the plan edits that use
check-regtest.sh.Changes
plan-pre-upgrade.md(new): the preparatory work that both upgrades depend on, one item per topic. Items done by other pull requests only name them: regtest in CI (Run Valgrind's regression tests in CI, against expected failures #96, with the remaining steps to bootstrap and enforce the expected-failure lists after it lands),git diffpatch files applied withgit apply --3way(Valgrind merge: create patches with git diff and apply them with git apply #103), input checks ingenerate-patch-files.sh(generate-patch-files.sh: check inputs; fail on errors #99), merge-doc updates (Merge doc: clarify steps that were easy to get wrong #101), and removing stale upstream files (Remove 32 upstream files that upstream Valgrind deleted #98). Two items are described in full: a scan for DynComp panics on unhandled IROps (item 4), and thebfd.hversion in each CI container (item 6; the oldest is Binutils 2.41, in the Rocky Linux 10 image).plan-upgrade-readelf.md: do the preparation first; why to upgrade Binutils before Valgrind; the landed (Fix DW_OP_breg* locations on amd64 #80, Fix Fjalar hang on optimized code with location views #89) and open (Trace formal parameters whose location is a register #90, Read DWARF registers through their guest-state offsets #91, Omit a formal parameter whose location contains an unsupported DWARF operation #95) pull requests that changedwarf.c; verifying the tarballs' signatures; commands for the Fjalar-vs-Binutils reference diffs; zstd and the systembfd.hversion;--merge=diff3conflict markers; disabling the#includeof a header whose code is disabled, and removing headers that upstream deleted; absolute paths in commands. Also: mergedwarf.c,dwarf.h, andreadelf.cwithgit merge-file --diff3, excluded frombinutils.patchso that they are not patched twice; don't add new features; the consumers of the Binutils-derived code and the#if 0counts; work in a new clone, and use./test-kvasir.shrather thanmake daikon-test(which ignores$DAIKONDIR); a warning-list baseline; a checklist of upgrade-sensitive areas (DWARF 5, debuginfod,readelfglobals,dwarf.hstructures); DWARF 4 versus DWARF 5,--fjalar-debug-dump,harvest_*-count, andnmchecks; what to add to the merge doc.plan-upgrade-valgrind.md: do the preparation first; the Fjalar changes since the 3.23 merge (Backport upstream Valgrind fix for$(nproc)in auxprogs/Makefile.am #84's backport, which the merge should drop; DynComp: treat the remaining vector shifts as shifts #88; Declare Fjalar's two core functions locally instead of including pub_core headers #97); the open DynComp and Memcheck-derived pull requests to land first, and aghcommand that lists the open pull requests that change files undervalgrind/; using--dyncomp-unhandled-ops=warn(DynComp: add --dyncomp-unhandled-ops=panic|warn #85) to find unhandled operations; re-running the item-4 programs after the merge; installinggdband the glibc debuginfo package before the upstream baseline. Also: a "Scope" section; why one jump, with a fallback hop at 3.25.1; work in a new clone, and use./test-kvasir.shrather thanmake daikon-test; a build-log baseline;host_generic_reg_alloc3.calready carries Fjalar's spill-area change, which must be kept; shadow-state alignment and guest-register offsets to recheck; a hunk-by-hunk checkoff ofmc_translate.c; a mechanicalIop_coverage check; what to look for in NEWS; what to add to the merge doc.The plans do not describe a separate 3-way merge in a scratch repository: #103's
apply-patch-files.shmerges each file 3-way withgit apply --3way.