Skip to content

Conversation

@charles-cooper
Copy link
Member

What I did

How I did it

How to verify it

Commit message

Commit message for the final, squashed PR. (Optional, but reviewers will appreciate it! Please see our commit message style guide for what we would ideally like to see in a commit message.)

Description for the changelog

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov
Copy link

codecov bot commented Jun 26, 2025

Codecov Report

Attention: Patch coverage is 70.37037% with 8 lines in your changes missing coverage. Please review.

Project coverage is 93.09%. Comparing base (c1b6130) to head (cfa0d1d).

Files with missing lines Patch % Lines
vyper/venom/context.py 63.15% 5 Missing and 2 partials ⚠️
vyper/venom/basicblock.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4686      +/-   ##
==========================================
+ Coverage   92.99%   93.09%   +0.09%     
==========================================
  Files         131      131              
  Lines       19094    19119      +25     
  Branches     3324     3329       +5     
==========================================
+ Hits        17757    17799      +42     
+ Misses        899      878      -21     
- Partials      438      442       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@charles-cooper
Copy link
Member Author

this seems to be working somewhat, but i get a bunch of OVERRUN messages (in debug mode), which seems to substantially slow down example generation:
Screenshot from 2025-06-26 12-35-20

@cyberthirst could you please look into this?

source = draw(st.sampled_from(reachable_blocks))

# we have already visited it
if source in cfg:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this might be a budget sink - i think that most of reachable blocks will already be inside the cfg. so with increasing number of reachable, the lower the probability of hitting the block which isn't already embedded

i think smth like this might make more sense

diff --git a/tests/functional/venom/test_memory_fuzzer.py b/tests/functional/venom/test_memory_fuzzer.py
index ec696f712..7a4446295 100644
--- a/tests/functional/venom/test_memory_fuzzer.py
+++ b/tests/functional/venom/test_memory_fuzzer.py
@@ -267,11 +267,12 @@ def control_flow_graph(draw, basic_blocks):
     reachable_blocks = [basic_blocks[0]]
 
     while remaining_blocks:
-        source = draw(st.sampled_from(reachable_blocks))
+        # Only sample from reachable blocks that aren't already in cfg
+        unprocessed_reachable = [b for b in reachable_blocks if b not in cfg]
+        if not unprocessed_reachable:
+            break
 
-        # we have already visited it
-        if source in cfg:
-            continue
+        source = draw(st.sampled_from(unprocessed_reachable))
 
         target = draw(st.sampled_from(remaining_blocks))

@cyberthirst
Copy link
Collaborator

we can monkeypatch some of the internal constants: https://hypothesis.readthedocs.io/en/latest/reference/internals.html#hypothesis.internal.conjecture.engine.BUFFER_SIZE

however, i'm still getting overruns, but of a new form: overrun because hit self.max_choices=201

@cyberthirst
Copy link
Collaborator

cyberthirst commented Aug 19, 2025

we can monkeypatch some of the internal constants: https://hypothesis.readthedocs.io/en/latest/reference/internals.html#hypothesis.internal.conjecture.engine.BUFFER_SIZE

however, i'm still getting overruns, but of a new form: overrun because hit self.max_choices=201

this is the number of overruns with/without the BUFFER_SIZE patch. both versions also use the reachability patch. however, the increased size also increases the test's runtime. max_examples=500 (note that the % of overruns is now
relatively low to make it fairly feasible for fuzzing)

Cause Run 1 Run 2
max_choices 51 52
max_length 64 0

@cyberthirst
Copy link
Collaborator

to reduce the choices, we can make the constants MAX_BASIC_BLOCKS, MAX_INSTRUCTIONS_PER_BLOCK smaller - i think 25 bbs is still reasonable. further we can try to batch various choices into one - e.g. instead of multiple boolean choices in one function, we could do one integer choice and then do weighted if statements - like if cnt < 2: ..., if cnt < 5: ..

Comment on lines +30 to +39
from vyper.venom.passes import (
AssignElimination,
CFGNormalization,
DeadStoreElimination,
LoadElimination,
MakeSSA,
MemMergePass,
SimplifyCFGPass,
SingleUseExpansion,
)

Check notice

Code scanning / CodeQL

Unused import Note test

Import of 'DeadStoreElimination' is not used.
Import of 'LoadElimination' is not used.
# First, handle output to allocate variable if needed
output_sym = None
if inst.output and isinstance(inst.output, SymbolicVar):
output_sym = inst.output

Check warning

Code scanning / CodeQL

Variable defined multiple times Warning test

This assignment to 'output_sym' is unnecessary as it is
redefined
before this value is used.
# All blocks except entry (to prevent back edges to entry)
non_entry_blocks = basic_blocks[1:]

# create a spanning tree to ensure all blocks are reachable
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we have back edges so this isn't creating a spanning tree?

other_target, target = target, other_target
block_types[source] = _BranchBB(target1=target, target2=other_target)

# classify remaining blocks that were not handled during spanning
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how can some nodes be missing if we created a "spanning tree"?

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.

2 participants