Skip to content

4100 booster abort rewrite on subject vars #4101

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Apr 1, 2025

Conversation

jberthold
Copy link
Member

Fixes #4100

During rewriting, booster was reporting the rule match as "failed" when trying to match a non-variable in the rule to a variable in the subject (introduced by a refactoring mid 2024).
This produces unsound results because the subject variable could be constrained to have a value that would match the rule term, enabling the application of this rule.
The result in booster should therefore be aborted as long as we don't allow for introducing constraints like this to the rule application.

The new code adds the subject variable to a set of "indeterminate" match pairs. Code has been kept separate so we can later extend the branching mechanism by the collected constraints on subject variables.

jberthold and others added 3 commits March 31, 2025 12:06
As demonstrated in #4100, booster should not report `stuck` when attempting to rewrite a term with variables that are required in a rule match.
Specifically, a subject variable may lead to branching with the rule content matches ending up as path conditions in the different branches.

Previously, booster would discard such cases as match failures and report `stuck` since no rule was applied.
This PR changes the behaviour to aborting the rewrite when a subject variable needs to be matched by anything but a rule variable.
@jberthold
Copy link
Member Author

kevm tests

Test 4100-booster-abort-rewrite-on-subject-vars time master-5fb452a51 time (4100-booster-abort-rewrite-on-subject-vars/master-5fb452a51) time
benchmarks/ecrecoverloop00-sig0-invalid-spec.k 62.01 85.6 0.7244158878504673
erc20/ds/transferFrom-failure-1-d-spec.k 61.17 76.44 0.8002354788069074
mcd-structured/end-subuu-pass-spec.k 62.23 75.79 0.8210845758015568
benchmarks/structarg00-spec.k 55.64 65.67 0.8472666362113598
examples/erc20-spec.md 138.31 161.56 0.85609061648923
test_wmul_weakly_increasing_positive-uint256-uint256-0-spec.k 69.37 80.72 0.8593904856293361
benchmarks/storagevar03-spec.k 48.14 55.6 0.8658273381294964
mcd-structured/end-pack-pass-rough-spec.k 115.68 131.68 0.8784933171324423
erc20/ds/transferFrom-failure-1-c-spec.k 103.47 114.42 0.904299947561615
kontrol/test-countertest-testincrement-0-spec.k 92.34 101.36 0.9110102604577743
benchmarks/ecrecover00-sigvalid-spec.k 60.0 64.06 0.9366219169528567
benchmarks/storagevar00-spec.k 51.47 54.14 0.9506834133727373
mcd/vow-fess-fail-rough-spec.k 92.73 97.08 0.9551915945611867
mcd/flipper-addu48u48-fail-rough-spec.k 51.59 53.57 0.9630390143737166
benchmarks/ecrecoverloop00-sig1-invalid-spec.k 90.72 94.04 0.9646958740961292
mcd/dsvalue-peek-pass-rough-spec.k 65.02 62.49 1.0404864778364538
mcd-structured/flopper-kick-pass-rough-spec.k 84.77 81.44 1.0408889980353635
mcd/flopper-tick-pass-rough-spec.k 155.22 148.91 1.042374588677725
TOTAL 1459.88 1604.57 0.9098263086060441

kontrol tests

(most of the longer tests are skipped in the new setting)
The extreme differences were caused by noise on the test machine.

Test 4100-booster-abort-rewrite-on-subject-vars time master-5fb452a51 time (4100-booster-abort-rewrite-on-subject-vars/master-5fb452a51) time
HevmTests.prove_revert 10.5 21.6 0.4861111111111111
test_foundry_refute_node 22.44 33.25 0.6748872180451129
ConstructorTest.testFail_constructor() 19.88 29.05 0.6843373493975903
AddrTest.test_addr_true()-trace_options1 18.76 26.73 0.7018331462775907
test_foundry_resume_proof 30.45 41.09 0.7410562180579215
HevmTests.proveFail_all_branches 31.41 38.51 0.8156323032978448
BMCLoopsTest.test_countdown_concrete() 9.2 10.88 0.8455882352941175
ConstructorArgsTest.test_constructor_args() 39.08 45.02 0.8680586406041758
HevmTests.prove_require_assert_false 24.24 27.49 0.8817751909785376
BlockParamsTest.testWarp(uint256)-trace_options2 15.33 17.07 0.8980667838312829
MockCallTestFoundry.testMockGetters() 24.83 26.46 0.9383975812547241
GasTest.testSetGas() 8.43 8.94 0.9429530201342282
InitCodeTest.test_init() 59.93 63.49 0.9439281776657741
PlainPrankTest.test_stopPrank_notExistent() 8.26 8.69 0.950517836593786
PlainPrankTest.test_prank_expectRevert() 13.27 13.92 0.9533045977011494
AssumeTest.test_assume_true(uint256,uint256) 10.32 10.8 0.9555555555555555
Setup2Test.testFail_setup() 20.01 20.83 0.9606337013922229
RollTest.test_roll_setup() 20.69 21.51 0.9618781961878196
FreshCheatcodes.test_int128() 8.87 9.22 0.9620390455531452
BlockParamsTest.testRoll(uint256) 9.24 9.6 0.9625
AssertTest.test_failing_branch(uint256) 32.34 31.13 1.03886925795053
Storage.sol 58.11 55.93 1.0389772930448775
AllowChangesTest.testFailAllowChangesToStorage() 13.06 12.52 1.0431309904153356
PlainPrankTest.test_startPrankWithOrigin_true() 15.19 14.53 1.0454232622161046
StoreTest.testStoreLoadNonExistent() 9.43 8.95 1.053631284916201
StartPrankTestOrigin.test_startprank_origin_setup() 33.41 31.7 1.0539432176656152
PlainPrankTest.test_startPrank_zeroAddress_true() 16.41 15.46 1.0614489003880982
BytesTypeTest.test_bytes32(bytes32) 8.95 8.41 1.0642092746730083
test_foundry_xml_report 16.59 15.49 1.0710135571336346
FreshCheatcodes.test_bool() 9.8 9.13 1.0733844468784228
PrankTestOrigin.test_origin_setup() 24.58 22.88 1.0743006993006994
test_foundry_auto_abstraction 25.98 24.05 1.0802494802494802
FreshCheatcodes.test_freshSymbolicWord() 9.38 8.68 1.0806451612903227
test_foundry_split_node 335.39 309.18 1.0847726243612135
test_foundry_show_with_hex_encoding 6.35 5.74 1.1062717770034842
HevmTests.proveFail_require_assert 24.22 21.85 1.1084668192219678
Setup2Test.test_setup() 10.29 9.13 1.1270536692223436
MethodDisambiguateTest.test_method_call() 9.39 8.04 1.1679104477611941
test_constructor_with_symbolic_args 86.85 70.67 1.2289514645535586
Empty.sol 88.45 69.82 1.266828988828416
test_foundry_merge_nodes 37.59 28.36 1.3254583921015517
BMCBoundTest.testBound() 115.05 79.16 1.4533855482566953
HevmTests.prove_require_assert_true 52.17 34.9 1.494842406876791
test_foundry_merge_loop_heads 98.03 61.96 1.5821497740477728
AccountParamsTest.testDealConcrete()-trace_options0 25.21 14.09 1.7892122072391767
test_foundry_remove_node 18.92 8.43 2.2443653618030845
ConstructorTest.test_constructor() 46.47 17.09 2.719133996489175
InitCodeTest.testFail_init() 70.92 25.74 2.7552447552447554
TOTAL 1703.67 1507.17 1.130376798901252

@jberthold jberthold marked this pull request as ready for review March 31, 2025 11:07
@jberthold jberthold requested a review from tothtamas28 March 31, 2025 11:08
@jberthold jberthold force-pushed the 4100-booster-abort-rewrite-on-subject-vars branch from 5dddebd to 11fb271 Compare March 31, 2025 11:14
@jberthold jberthold merged commit cefb70b into master Apr 1, 2025
6 checks passed
@jberthold jberthold deleted the 4100-booster-abort-rewrite-on-subject-vars branch April 1, 2025 22:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Booster reports Stuck but should branch or abort
2 participants