Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix(runtime-fuzzer): reduce the range of pages num for free syscall #3400

Closed
wants to merge 1 commit into from
Closed

fix(runtime-fuzzer): reduce the range of pages num for free syscall #3400

wants to merge 1 commit into from

Conversation

StackOverflowExcept1on
Copy link
Member

@StackOverflowExcept1on StackOverflowExcept1on commented Oct 11, 2023 •
edited
Loading 

let initial_pages = 2;
// ...
params_config.add_rule(ParamType::Free, (initial_pages..=initial_pages + 35).into());
// range with `initial_pages + 35` causes `free` syscall with page_no > initial_pages
// (leads to frequent falls)

@StackOverflowExcept1on StackOverflowExcept1on added the A0-pleasereview PR is ready to be reviewed by the team label Oct 11, 2023
breathx
breathx approved these changes Oct 11, 2023
View reviewed changes
utils/runtime-fuzzer/src/gear_calls.rs
@@ -426,7 +426,7 @@ fn config(

let mut params_config = SysCallsParamsConfig::default();
params_config.add_rule(ParamType::Alloc, (10..=20).into());
params_config.add_rule(ParamType::Free, (initial_pages..=initial_pages + 35).into());
params_config.add_rule(ParamType::Free, (initial_pages..=initial_pages + 1).into());
Copy link
Member

Choose a reason for hiding this comment

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

mb at least 2-4?

techraed
techraed approved these changes Oct 11, 2023
View reviewed changes
utils/runtime-fuzzer/src/gear_calls.rs
@@ -426,7 +426,7 @@ fn config(

let mut params_config = SysCallsParamsConfig::default();
params_config.add_rule(ParamType::Alloc, (10..=20).into());
params_config.add_rule(ParamType::Free, (initial_pages..=initial_pages + 35).into());
Copy link
Member

Choose a reason for hiding this comment

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

Open please an issue for making more complex tests for free. As current generator produces multiple alloc calls allocating minimum 10 pages, would be nice to free those very possibly unused in current fuzzer pages, instead of freeing pages with active data for a program that mostly uses them. I afraid these changes can lower amount of executions for update_pages_data in journal notes handler.

Copy link
Member

Choose a reason for hiding this comment

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

is it open @StackOverflowExcept1on ?

@StackOverflowExcept1on StackOverflowExcept1on changed the title fix(runtime-fuzzer): reduce the percentage of unreachable instructions fix(runtime-fuzzer): reduce the range of pages num for free syscall Oct 12, 2023
@StackOverflowExcept1on
Copy link
Member Author

As I figured out this PR is unrelated to 3382. The last one in the logs was free syscall, but the runtime error was not shown because the runtime::sandbox logger was disabled.

But in general, we can think about how to reduce the number of incorrect free syscalls, as @SabaunT wrote.

@StackOverflowExcept1on StackOverflowExcept1on added A1-inprogress Issue is in progress or PR draft is not ready to be reviewed and removed A0-pleasereview PR is ready to be reviewed by the team labels Oct 12, 2023
@StackOverflowExcept1on StackOverflowExcept1on mentioned this pull request Oct 16, 2023
Open
@techraed
Copy link
Member

Closed as not solving the problem stated in #3382.

@StackOverflowExcept1on StackOverflowExcept1on deleted the av/fuzzer-config branch December 1, 2023 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NikVolf NikVolf NikVolf left review comments

@breathx breathx breathx approved these changes

@techraed techraed techraed approved these changes

@mertwole mertwole Awaiting requested review from mertwole

Assignees
No one assigned
Labels
A1-inprogress Issue is in progress or PR draft is not ready to be reviewed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@StackOverflowExcept1on @techraed @NikVolf @breathx