Give valgrind-wrapped leak tests a minimum timeout ##r2r - #26471
Merged
Conversation
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.
Description
A
<leak>test derives its per-test budget exactly like a plain cmd test (run.c:1765vs:1328) and is then wrapped in valgrind, so one wall-clock budget covers two execution modes ~40x apart. Measured ondb/leak/open's slowest case (r2 -AA bins/dotnet/HelloWorld.exe): 0.55s native, 22.0s wrapped. The analysis itself is not slow — 285 KB/s vs 393 KB/s forelf/ls— it is just the largest binary indb/leak.Invisible at the 3600s default, which is why it has gone unnoticed. It bites any budget that is sane for the ~17k native tests:
Fix is a floor, not a multiplier: leak tests get at least 120s (5.5x the slowest measured), whatever the budget's source. A multiplier was tried first and rejected — the leak job runs with no
-t(tcc.yml:242), so 3600s x 40 would have removed the reaper rather than widened it.Verified:
-t 0's "no timeout" sentinel survives (a floor only ever raises); a leak test too slow to finish is still reaped, at 120.0s; no change at the default budget; lint and-Wall -Werror -O2clean; full r2r gate vs master 0 regressions.The floor also applies to an explicit per-test
TIMEOUT=— deliberate, since a sub-120s budget cannot be met under valgrind and would only produce a false failure. Happy to make it default-path only if you prefer.