Composite action

[FabrizioSandri]

Composite Action implementation

In this pull request I will integrate the existing Docker based GitHub action with a composite action. The blog article titled Moving to a composite GitHub Action explains the motivations of this change.

The main reason for this update is that the action can only print the results in the log in text format when using the docker architecture, making it nearly impossible to upload the results as an artifact file and share them with other steps/jobs in the workflow. Using a Composite architecture, on the other hand, allows to use all of the existing actions available in the GitHub Marketplace, such as actions/upload-artifact.

[FabrizioSandri]

In the last few commits I moved the existing docker action into the docker directory. This Docker Action will be triggered by the composite action during the Analyze the package with RcppDeepState stage. All of the docker action's inputs have been included to the composite action as well.

Additionally, after compressing the inputs, the actions/upload-artifact has been added in the stages of this action, because the number of files to be uploaded is large, and compressing them before uploading them is better in terms of speed. Furthermore, the vgcore files created by Valgrind were deleted to minimize the size of the compressed archive.

[FabrizioSandri]

Hi, @randy3k! I had Google meet with @tdhock yesterday and presented him the Action that I am designing for GSOC. I created a first functional prototype of a Docker-based action two weeks ago. However, I discovered several factors that led me to convert from a docker to a composite action. The main reason is that having a composite action allows me to access all of the current GitHub Marketplace actions, such as actions/upload-artifact. So, instead of removing the docker action, I considered moving it to a subdirectory of the repository, let's call it ./docker, and then creating a new action.yml file defining a composite action that triggers the docker one. This is the final structure of the repository:

.
├── action.yml  # this is the composite action
└── docker
    ├── action.yml  # this is the docker action
    └── Dockerfile

This structure has been implemented in this pull request. And the composite action.yml file will trigger the docker action in the second step, as you can see in theese lines of code.

runs:
  using: "composite"
  steps:

    - uses: actions/checkout@v2 
      with:
        repository: FabrizioSandri/RcppDeepState-action
        path: RcppDeepState-action

    - name: Analyze the package with RcppDeepState 
      uses: ./RcppDeepState-action/docker

I wanted to ask you if building a composite action combined with a docker action inside the same repository is a good approach, or having a single composite action in the repository is preferable?

[FabrizioSandri]

The following parameters have been added in the most recent commits:

• fail_ci_if_error: this parameter allows developers to define whether the workflow should fail if at least one error is found by RcppDeepState;
• comment: if set to 'true' the analysis result is reported as a comment in pull requests.

I'm going to test these parameters on the testSAN package.

[github-actions[bot]]

Test 1

Inputs

rbound: -16844752

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 4	read_out_of_bound.cpp : 7	Address 0xffffffffe13cb328 is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 2

Inputs

rbound: -16844752

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 4	read_out_of_bound.cpp : 7	Address 0x193b7de4c is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 3

Inputs

rbound: -16844752

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 4	read_out_of_bound.cpp : 7	Address 0xfffffffef394d1ac is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 4

Inputs

array_size: 1344080079

Logtable

err.kind	message	file.line	address.msg	address.trace
FishyValue	Argument 'size' of function __builtin_vec_new has a fishy (possibly negative) value: -1246227084	use_after_deallocate.cpp : 6	No Address found	No Address Trace found

Test 5

Inputs

array_size: 1344080079

Logtable

err.kind	message	file.line	address.msg	address.trace
FishyValue	Argument 'size' of function __builtin_vec_new has a fishy (possibly negative) value: -1116281767	use_after_deallocate.cpp : 6	No Address found	No Address Trace found

Test 6

Inputs

array_size: 1344080079

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 1	use_after_deallocate.cpp : 8	Address 0xcc38045 is 5 bytes inside a block of size 1,058,509,085 free'd	use_after_deallocate.cpp : 6
InvalidRead	Invalid read of size 1	use_after_deallocate.cpp : 7	Address 0xcc38045 is 5 bytes inside a block of size 1,058,509,085 free'd	use_after_deallocate.cpp : 6

Test 7

Inputs

alloc_size: 1790187482

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 4	use_after_free.cpp : 8	Address 0x59c87068 is 40 bytes inside a block of size 3,101,484,452 free'd	use_after_free.cpp : 6
InvalidRead	Invalid read of size 4	use_after_free.cpp : 7	Address 0x59c87068 is 40 bytes inside a block of size 3,101,484,452 free'd	use_after_free.cpp : 6

Test 8

Inputs

alloc_size: 1790187482

Logtable

err.kind	message	file.line	address.msg	address.trace
FishyValue	Argument 'size' of function malloc has a fishy (possibly negative) value: -6179699072	use_after_free.cpp : 6	No Address found	No Address Trace found
InvalidRead	Invalid read of size 4	use_after_free.cpp : 8	Address 0x28 is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 9

Inputs

alloc_size: 1790187482

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidRead	Invalid read of size 4	use_after_free.cpp : 8	Address 0x59c87068 is 40 bytes inside a block of size 4,898,905,888 free'd	use_after_free.cpp : 6
InvalidRead	Invalid read of size 4	use_after_free.cpp : 7	Address 0x59c87068 is 40 bytes inside a block of size 4,898,905,888 free'd	use_after_free.cpp : 6

Test 13

Inputs

wbound: 605794147

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	write_index_outofbound.cpp : 8	Address 0x3a2475f8 is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 14

Inputs

wbound: 605794147

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	write_index_outofbound.cpp : 8	Address 0xffffffffaff4d200 is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 15

Inputs

wbound: 605794147

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	write_index_outofbound.cpp : 8	Address 0x5d9c7fe8 is not stack'd, malloc'd or (recently) free'd	No Address Trace found

Test 16

Inputs

value: -34311738

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	zero_sized_array.cpp : 8	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
InvalidRead	Invalid read of size 4	zero_sized_array.cpp : 9	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
Leak_DefinitelyLost	0 bytes in 1 blocks are definitely lost in loss record 1 of 1,261	zero_sized_array.cpp : 7	No Address found	No Address Trace found

Test 17

Inputs

value: -34311738

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	zero_sized_array.cpp : 8	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
InvalidRead	Invalid read of size 4	zero_sized_array.cpp : 9	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
Leak_DefinitelyLost	0 bytes in 1 blocks are definitely lost in loss record 1 of 1,261	zero_sized_array.cpp : 7	No Address found	No Address Trace found

Test 18

Inputs

value: -34311738

Logtable

err.kind	message	file.line	address.msg	address.trace
InvalidWrite	Invalid write of size 4	zero_sized_array.cpp : 8	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
InvalidRead	Invalid read of size 4	zero_sized_array.cpp : 9	Address 0x783cc70 is 0 bytes after a block of size 0 alloc'd	zero_sized_array.cpp : 7
Leak_DefinitelyLost	0 bytes in 1 blocks are definitely lost in loss record 1 of 1,261	zero_sized_array.cpp : 7	No Address found	No Address Trace found

[tdhock]

very cool rendering of tables in github comment. even cooler if you could hyperlink from items in the file.line/address.trace columns to the corresponding line of code on github, do you think that would be possible? also instead of listing all the tests which generated some valgrind message (which is maybe too much info), it would be preferable to make a table like this https://akhikolla.github.io./packages-folders/BNSL.html which lists, for every function that was tested, what is the first valgrind message that was reported. (should be much smaller and easier for the user to understand)

[randy3k]

@FabrizioSandri All these changes are invisible to users, so either way would be fine. I think moving to composite action is a good idea because it allows more flexible extension. Though when you check out

    - uses: actions/checkout@v2 
      with:
        repository: FabrizioSandri/RcppDeepState-action
        path: RcppDeepState-action

You should be aware that you are always checking out the default branch even when your user calls

- uses: FabrizioSandri/RcppDeepState-action@some-branch

Ideally, you could check out some-branch in the actions/checkout step. However, I don't think it is possible right now and I don't have a good solution for now.

Additionally, to speed up the running time, you could prebuilt a docker image and upload image to https://hub.docker.com/. docker has actions to doing all these automatically.

[FabrizioSandri]

Thank you @randy3k for your quick and detailed response. Uploading the docker image to Docker Hub would be a good improvement for RcppDeepState-action. I observed that a considerable amount of time is spent building the docker container for the action, therefore I'll consider making this upgrade.

[github-actions[bot]]

function_name	message	file_line	address_trace
rcpp_read_out_of_bound	Invalid read of size 4	read_out_of_bound.cpp:7	No Address Trace found
rcpp_use_after_deallocate	Invalid read of size 1	use_after_deallocate.cpp:8	use_after_deallocate.cpp:6
rcpp_use_after_free	Invalid read of size 4	use_after_free.cpp:8	use_after_free.cpp:6
rcpp_write_index_outofbound	Invalid write of size 4	write_index_outofbound.cpp:8	No Address Trace found
rcpp_zero_sized_array	Invalid write of size 4	zero_sized_array.cpp:8	zero_sized_array.cpp:7

[FabrizioSandri]

@tdhock, I added the hyperlinks from the items to the lines of code in the last few commits. I also modified the table so that it only prints the first error for each function analyzed. The result can be seen in the last comment by github-actions.

I have in plan to add a column containing the inputs of the test in a "collapsed" format, to keep the table size minimal. For example clickable inputs:

Input 1

123

Input 2

456

[github-actions[bot]]

function_name	inputs	message	file_line	address_trace
rcpp_read_out_of_bound	rbound 1423296510	Invalid read of size 4	read_out_of_bound.cpp:7	No Address Trace found
rcpp_use_after_deallocate	array_size 2069105866	Invalid read of size 1	use_after_deallocate.cpp:8	use_after_deallocate.cpp:6
rcpp_use_after_free	alloc_size 475723504	Invalid read of size 4	use_after_free.cpp:8	use_after_free.cpp:6
rcpp_write_index_outofbound	wbound -879321465	Invalid write of size 4	write_index_outofbound.cpp:8	No Address Trace found
rcpp_zero_sized_array	value -1478884570	Invalid write of size 4	zero_sized_array.cpp:8	zero_sized_array.cpp:7

[FabrizioSandri]

In the most recent printed table it is possible to examine the inputs that led to the errors . By clicking on each input value, the corresponding value is showed.

[github-actions[bot]]

function_name	inputs	message	file_line	address_trace
rcpp_read_out_of_bound	rbound -1279349631	Invalid read of size 4	read_out_of_bound.cpp:7	No Address Trace found
rcpp_use_after_deallocate	array_size 2135056896	Invalid read of size 1	use_after_deallocate.cpp:8	use_after_deallocate.cpp:6
rcpp_use_after_free	alloc_size 1818847580	Argument 'size' of function malloc has a fishy (possibly negative) value: -5910388872	use_after_free.cpp:6	No Address Trace found
rcpp_write_index_outofbound	wbound -410987196	Invalid write of size 4	write_index_outofbound.cpp:8	No Address Trace found
rcpp_zero_sized_array	value 83998681	Invalid write of size 4	zero_sized_array.cpp:8	zero_sized_array.cpp:7

[tdhock]

hey @FabrizioSandri that is really great to have the results organized in a table link that with hyper-links, good job. I see that the hyperlinks go to test packages underneath the tests directory in your PR branch https://github.com/FabrizioSandri/RcppDeepState-action/blob/composite-action/tests/testSAN/src/zero_sized_array.cpp#L8, so I'm wondering if the hyper-links will also work if the files are in the standard src/*.cpp place for a package? Also I really like the little triangles for show/hide of the input values, that is super useful for informal inspection of the results. For more formal debugging though it would be more useful to have something like what is linked under the executable.file column on this page https://akhikolla.github.io./packages-folders/SpecsVerification.html, an R code file which the user can just copy and paste into their terminal in order to re-construct the problematic inputs, like this:

testlist <- list(fcst = c(-1.36189834845372e+233, -4.96476445211685e-113,  4.57869030288816e+216, NaN, 1.89588885056273e-206, 6.32464198819322e-269,  2.91658984746689e-123, 3.3908885096056e+28, -1.3519550534275e+233,  -2.92820689366386e+62, -7.82840227744738e+61, -3.11305098229425e-152,  -4.09045746819333e+111, -8.6916912207099e+108, -7.85910685134881e+52,  4.03197128344888e-82, 2.03367438333185e+127, -1.02235149024108e+185,  8.48674365287105e-63, 2.01947218482486e-277, 7.99643146953577e+170,  1.55709139792174e+254, -6.2047033830381e-23, 5.31104280923309e-177,  -1.41628928542658e-308, NA, -1.85766846958224e+44, 4.32503112399815e-264,  6.88768997015808e+85, -4.04372065272737e+77, -7.43332816237665e-150,  -4.38917772188799e-192, 6.84814398130236e+36, 9.38423839164995e-181,  -4.00099710393222e-192, 1.06443240801012e-253, -2.79291659146001e-271,  -3.13553637881162e-256, -1.27173272777544e-08, -Inf, 3.55188869801887e+152,  9.64941048269568e+79, 3.0384132766933e-08, 2.03367438333185e+127,  -1.75583636675576e+21, -1.15077399615719e-06, 4.85899366584286e+169,  -2.03327726380656e+273, 5.6788227081813e+102, 3.64198592742185e+53,  -2.63482765163939e+147, 3.24990012886666e-171, -2.00607987479092e-265,  1.85540129858481e-07, 7.46599090287989e-95, -3.38263399925677e+225,  2.87942402177584e-130, 1.8151329717101e-156, 1.13537971864626e-77,  -2.22667191799216e-35, 9.51209398989272e-182, 4.06044187647137e+77,  -8.97881963882067e-207, -1.01273707846957e-248, 5.11268657733371e-220,  -2.6955423687627e+259, -2.31626966244184e-48, -5.72888630599658e+304,  -3.46641116612178e+171, 3.49067525946871e+103, -1.11582723148929e-290,  -1.86733168046532e-179, 1.11952284688044e+175, -5.77914322724667e+159,  -5.37758857516483e+306, 2.00788060739898e+296, 1.67155702298598e+106,  -1.03223743586675e+99, -1.59724499462991e+86, 3.41238696819031e-217,  -4.63911533434594e+87, -3.05656594404308e-253, 1.06432623771143e+107,  -7.42395886034513e-221, 2.43536245743433e-219, 2.05062141501913e-35,  4.92913573305826e-111, 1.96162466744382e+188, -1.61405022008727e-120,  -45243740827551.3, -2.45776163456198e-245, 4.97485035457822e+233,  -2.88654261956446e+178, 6.69853991187542e-207, 1.95960423271618e+198,  0), obs = c(-1.73058053423204e+50, -1.94554235830854e+170, -1.56240152468541e-54,  -8.25092438108189e-49, -2.08776022078289e+137, -3.50075289634033e+167,  -Inf, -1.08926927986249e-07, 4.31458701911893e-19, -2.56334963334795e-177,  -7.15070139534133e+21, 2.67178959779217e+243, -7.42493611858681e+218,  -Inf, 7.09526066158152e+269, -Inf, -6.8862309889748e-149, -4.07497615916177e+304,  -8.72857902522234e+222, -5.99074538697357e-136, 7.64666971027258e+236,  -1.73722802605537e+80, -8.79529017168655e-221, -1.55801885469436e-241,  8.12257902488912e+214, -4.68949153091161e+181, 1.51134285537189e-39,  -3.47239516751446e-252, -1.39733462791211e-233, NaN, Inf, -23.6346386021491,  0))
result <- do.call(SpecsVerification::auc_cpp,testlist)
str(result)

you can get the output above by passing any R object to the dput function,

> testlist <- list(foo=1:2, bar="baz")
> testlist.Rcode <- capture.output(dput(testlist))
> mylist <- eval(parse(text=testlist.Rcode))
> identical(testlist, mylist)
[1] TRUE

[FabrizioSandri]

Thanks @tdhock!

if the hyper-links will also work if the files are in the standard src/*.cpp place for a package?

Do you mean the case when the file_line column relate to a file placed in a subdirectory of src? With the actual implementation, I assume that the hyperlink refers only to a file in the src folder, and not in its subdirectories. I can solve this problem in two ways:

1. modify the RcppDeepState library's result table creation process such that the file line is concatenated with the absolute path(dir element in the valgrind XML file);
2. find the associated valgrind log file within the action, analyze it, and extract the absolute path.

Regarding the final part, I'm looking for the best way to upload the executable file. My first thought is to upload all of the executable files as a single artifact zip file and link this zip file in the pull request comment. The issue with this is that users must download the entire zip file including all the executable files, also if they only require one executable file. Another option is to use an external service provider to upload each executable file and then link to each of them in the comment.

[tdhock]

Do you mean the case when the file_line column relate to a file placed in a subdirectory of src?

No I meant when the file is src/*.cpp, the typical place. (it seems like in the example table https://github.com/FabrizioSandri/RcppDeepState-action/pull/4#issuecomment-1185742588 the links are to a src directory under the tests directory, which is not typical).

[tdhock]

I'm looking for the best way to upload the executable file.

Can you please just put the text in the comment under another one of those arrows? (probably in another column called "R code" or something like that)

[github-actions[bot]]

function_name	inputs	message	file_line	address_trace	R_code
rcpp_read_out_of_bound	rbound -1141133358	Invalid read of size 4	read_out_of_bound.cpp:7	No Address Trace found	Test code testlist <- list(rbound = -1141133358L) result <- do.call(testSAN::rcpp_read_out_of_bound,testlist)
rcpp_use_after_deallocate	array_size -524755545	Argument 'size' of function __builtin_vec_new has a fishy (possibly negative) value: -1957293569	use_after_deallocate.cpp:6	No Address Trace found	Test code testlist <- list(array_size = -524755545L) result <- do.call(testSAN::rcpp_use_after_deallocate,testlist)
rcpp_use_after_free	alloc_size -43138275	Invalid read of size 4	use_after_free.cpp:8	use_after_free.cpp:6	Test code testlist <- list(alloc_size = -43138275L) result <- do.call(testSAN::rcpp_use_after_free,testlist)
rcpp_write_index_outofbound	wbound -1083758256	Invalid write of size 4	write_index_outofbound.cpp:8	No Address Trace found	Test code testlist <- list(wbound = -1083758256L) result <- do.call(testSAN::rcpp_write_index_outofbound,testlist)
rcpp_zero_sized_array	value -826366193	Invalid write of size 4	zero_sized_array.cpp:8	zero_sized_array.cpp:7	Test code testlist <- list(value = -826366193L) result <- do.call(testSAN::rcpp_zero_sized_array,testlist)

[FabrizioSandri]

so I'm wondering if the hyper-links will also work if the files are in the standard src/*.cpp place for a package?

Yes, they work correctly also when the files are in the standard src/*.cpp folder.

Relative to the repository's root, the hyperlink is given by the concatenation of the action's location parameter with src and finally with the file name. In the specific case of the workflow that is triggered in this pull request(action-test.yaml - line 24), the location parameter is provided to the action with the value /tests/testSAN, so hyperlinks will point to the src folder underneath /tests/testSAN.

I expect that developers would typically keep the location argument set to its default value, which is /. This indicates that the package is kept at the repository's root. In this way, hyperlinks will point to the / + src = /src folder of the repo.

The location parameter's primary purpose is to enable me to test the action I'm creating without actually saving the packages I want to analyze in the repository's root. This argument also enables running this action for several packages located in the same repository, but in different locations.

[FabrizioSandri]

Can you please just put the text in the comment under another one of those arrows? (probably in another column called "R code" or something like that)

I have just added a column named R_code in the commit effdc05. This column contains a clickable triangle arrow, that shows the test code. The result can be seen in the last action's comment.

[FabrizioSandri]

@tdhock, if you agree with me I'll merge and close this pull request and open a new one for the docker-hub integration.

[github-actions[bot]]

function_name	inputs	message	file_line	address_trace	R_code
rcpp_read_out_of_bound	rbound -1591027753	Invalid read of size 4	read_out_of_bound.cpp:7	No Address Trace found	Test code testlist <- list(rbound = -1591027753L) result <- do.call(testSAN::rcpp_read_out_of_bound, testlist)
rcpp_use_after_deallocate	array_size 822188506	Invalid read of size 1	use_after_deallocate.cpp:8	use_after_deallocate.cpp:6	Test code testlist <- list(array_size = 822188506L) result <- do.call(testSAN::rcpp_use_after_deallocate, testlist)
rcpp_use_after_free	alloc_size -2083945869	Argument 'size' of function malloc has a fishy (possibly negative) value: -1116030032	use_after_free.cpp:6	No Address Trace found	Test code testlist <- list(alloc_size = -2083945869L) result <- do.call(testSAN::rcpp_use_after_free, testlist)
rcpp_write_index_outofbound	wbound 669721096	Invalid write of size 4	write_index_outofbound.cpp:8	No Address Trace found	Test code testlist <- list(wbound = 669721096L) result <- do.call(testSAN::rcpp_write_index_outofbound, testlist)
rcpp_zero_sized_array	value -1242593249	Invalid write of size 4	zero_sized_array.cpp:8	zero_sized_array.cpp:7	Test code testlist <- list(value = -1242593249L) result <- do.call(testSAN::rcpp_zero_sized_array, testlist)