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.

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

Parameters names/values #18

Merged
merged 14 commits into from Aug 29, 2022
Merged

Parameters names/values #18

merged 14 commits into from Aug 29, 2022

Conversation

FabrizioSandri
Copy link
Owner

@FabrizioSandri FabrizioSandri commented Aug 26, 2022

The changes made in this pull request amount to:

  • change the name of the time_limit parameter to max_seconds_per_function;
  • check whether single quotes are required for parameters like booleans and integers inside workflow files;
  • solve the problem that cause the action's log to include a warning message: rm: missing operand;
  • solve the issue that cause a duplicate concatenation of the src path in the hyperlinks of the comment;
  • strengthened the tests on NA values to avoid unexpected errors.

Fixes #16

@github-actions
Copy link

github-actions bot commented Aug 26, 2022

RcppDeepState Report

function name message file line address trace R code
rcpp_read_out_of_bound Invalid read of size 4 read_out_of_bound.cpp:7 No Address Trace found
Test code
testlist <- list(rbound = -1203971441L)
result <- do.call(testSAN:::rcpp_read_out_of_bound, testlist)
rcpp_use_after_deallocate Argument 'size' of function __builtin_vec_new has a fishy (possibly negative) value: -1203971441 use_after_deallocate.cpp:6 No Address Trace found
Test code
testlist <- list(array_size = -1203971441L)
result <- do.call(testSAN:::rcpp_use_after_deallocate, testlist)
rcpp_use_after_free Argument 'size' of function malloc has a fishy (possibly negative) value: -4815885764 use_after_free.cpp:6 No Address Trace found
Test code
testlist <- list(alloc_size = -1203971441L)
result <- do.call(testSAN:::rcpp_use_after_free, testlist)
rcpp_write_index_outofbound Invalid write of size 4 write_index_outofbound.cpp:8 No Address Trace found
Test code
testlist <- list(wbound = -1203971441L)
result <- do.call(testSAN:::rcpp_write_index_outofbound, testlist)
rcpp_zero_sized_array Invalid write of size 4 zero_sized_array.cpp:8 zero_sized_array.cpp:7
Test code
testlist <- list(value = -1203971441L)
result <- do.call(testSAN:::rcpp_zero_sized_array, testlist)

Analyzed functions summary

function name tested inputs inputs with issues
rcpp_read_out_of_bound 3 3
rcpp_use_after_deallocate 3 3
rcpp_use_after_free 3 3
rcpp_use_uninitialized 3 0
rcpp_write_index_outofbound 3 3
rcpp_zero_sized_array 3 3

Report details

  • Report generated by: f6b40ab
  • Inputs generator seed: 5

@tdhock
Copy link
Collaborator

tdhock commented Aug 26, 2022

looks good, thanks!

README.md Outdated
@@ -10,7 +10,7 @@ RcppDeepState is a fuzz testing library made as a composition of three tools: Rc
- **fail_ci_if_error** (default value: `false`) - Specify if CI pipeline should fail when RcppDeepState finds errors;
- **location** (default value: `/`) - Relative path under `$GITHUB_WORKSPACE` that contains the package that needs to be analyzed. Default uses the `/` location relative to `$GITHUB_WORKSPACE`, that is `$GITHUB_WORKSPACE`;
- **seed** (default value: `-1`) - control the randomness of the inputs generated in the fuzzing phase;
- **time_limit** (default value: `5`) - Fuzzing phase's duration in seconds;
- **time_limit_seconds** (default value: `5`) - Fuzzing phase's duration in seconds;
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is the duration for every input or for every function? maybe could be further clarified by changing the name to something like max_seconds_per_function ?

Copy link
Owner Author

Choose a reason for hiding this comment

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

I agree with you; this makes it clearer for the end user the purpose of the parameter.
Updated with commit aba09b7

@@ -10,7 +10,7 @@ RcppDeepState is a fuzz testing library made as a composition of three tools: Rc
- **fail_ci_if_error** (default value: `false`) - Specify if CI pipeline should fail when RcppDeepState finds errors;
- **location** (default value: `/`) - Relative path under `$GITHUB_WORKSPACE` that contains the package that needs to be analyzed. Default uses the `/` location relative to `$GITHUB_WORKSPACE`, that is `$GITHUB_WORKSPACE`;
- **seed** (default value: `-1`) - control the randomness of the inputs generated in the fuzzing phase;
- **time_limit** (default value: `5`) - Fuzzing phase's duration in seconds;
- **time_limit_seconds** (default value: `5`) - Fuzzing phase's duration in seconds;
- **max_inputs** (default value: `3`) - Maximum number of inputs that will be processed by RcppDeepState;
Copy link
Collaborator

Choose a reason for hiding this comment

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

for every function

Copy link
Owner Author

Choose a reason for hiding this comment

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

Updated the README with commit aba09b7

@FabrizioSandri
Copy link
Owner Author

@tdhock, I removed the single quotes from the list of arguments in the workflow file that tests the RcppDeepState-action and discovered that everything still works perfectly without them. I will update the README file to remove the unnecessary quotes.

Tested with commit afd0b37

@FabrizioSandri
Copy link
Owner Author

I updated the workflow files and the README with the most recent commits, as indicated beforehand.

@FabrizioSandri
Copy link
Owner Author

FabrizioSandri commented Aug 29, 2022

With the second most recent commits, I fixed the problem that caused the action's log to include the following warning message:

rm: missing operand
Try 'rm --help' for more information.

This is because before running the xargs command in a pipe, it is necessary to verify if the command on the left side of the pipe succeeded. This is the man page of xargs:

-r, --no-run-if-empty
              If the standard input does not contain any nonblanks, do not run
              the command.  Normally, the command is run once even if there is
              no input.  This option is a GNU extension.

@FabrizioSandri
Copy link
Owner Author

The src prefix may already be present in the file line reference, which might lead to a duplicate concatenation of the src path in the hyperlinks of the comment. To address this I included the usage of the basename function in order to delete all of the path up to and including the last path separator.

@FabrizioSandri
Copy link
Owner Author

With the most recent changes, I strengthened the tests on NA values to avoid unexpected errors. For instance, prior to this update, it was possible for the address trace column to include a value of NA, which would result in the failure of the whole script.

@tdhock
Copy link
Collaborator

tdhock commented Aug 29, 2022

looks great, please merge if you like.

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.

parameter names / value?
2 participants