fuzz-introspector
fuzz-introspector copied to clipboard
Incorrect hyperlink in the calltree
For example, see the call site link of the function igraph_i_parse_real
in the igraph report, https://storage.googleapis.com/oss-fuzz-introspector/igraph/inspector-report/20230226/calltree_view_4.html?scrollToNode=00221
The call site of the aforementioned function in the original igraph library is this: https://github.com/igraph/igraph/blob/6de21b242e764c2a0a6cb7a6ae04aa70262dbc25/src/io/dl-parser.y#L229. The line numbers of functions and call sites are all taken from this file ending with ".y".
However, link in the call tree links to a C file in the coverage report which is converted from the .y file. This creates a discrepancy between all line numbers recorded from the original file and the converted C file. See the coverage report highlighting the incorrect line (this is the link to call cite of igraph_i_parse_real
in the call tree): https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L229.
Could this be a potential bug? Thank you.
Interesting, thanks for reporting!
Am trying to understand what the true callsite should be, is it https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L1845 ?
dl-parser.c
is generated from dl-parser.y
by GNU Bison. The .y
file is written with Bison syntax, but contains mostly C code.
Bison includes #line
preprocessor directives into the output by default. This allows most tools to produce diagnostics that refer back to the .y
file. igraph actually has this turned off by default (for privacy reasons, think file paths hardcoded in the source release), but it has a switch for its CMake-based build system to turn it on, -DFLEX_KEEP_LINE_NUMBERS=ON
, which I used for OSS-fuzz: https://github.com/igraph/igraph/blob/master/fuzzing/build.sh#L23
This way, AddressSanitizer output, as well as compiler warnings, will actually refer to code written by humans (in the .y
source file), rather than the generated .c
files, whose layout depends on the Bison version.
The most useful location to refer to is in the .y
file, line 229:
https://github.com/igraph/igraph/blob/6de21b242e764c2a0a6cb7a6ae04aa70262dbc25/src/io/dl-parser.y#L229
What you link to is also technically correct:
https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L1845
But it is much less useful when working on igraph, as now I need to refer to the C sources generated specifically on the OSS-fuzz infrastructure, and relate them back the .y
file on my system.
Note that it is this line in the generated C file that allows the compiler to refer back to the .y file automatically:
https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L1842
Just going by the name of this ticket, this is another odd behavior, less interesting than how to handle intermediate generated source files which feels like an enhancement issue where certainly if attribution to the authored source is possible it should be supported.
Looking at https://storage.googleapis.com/oss-fuzz-introspector/htslib/inspector-report/20230124/fuzz_report.html and viewing first fuzz-blocker cram_close call site: 03338
The call site link is to the line above (hts_tpool_process_destroy) what is the actual call site of cram_close, cram_write_eof_block.
Thanks for the description @szhorvat
I think there are two issues at play here.
The callsite here https://storage.googleapis.com/oss-fuzz-introspector/igraph/inspector-report/20230226/calltree_view_4.html?scrollToNode=00221 is set to "red" because the line it refers back to "https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L229" which is not covered. This, however, is wrong information because the line number is taken from the *.y file (229) whereas it references the generated *.c file where the corresponding callsite in the coverage report is 1845 (https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/build/src/io/parsers/dl-parser.c.html#L1845). And in the coverage report, line 229 is irrelevant to the callsite.
There are two problems:
- The coverage indication is wrong, because it mingles the line numbers as described above.
- The link should be to *.y file rather than *.c file, because developers deal only with the *.y file and not really deal with the *.c file (which furthermore depends on the Bison version used).
Solution to (1) above should probably be fairly intuitive to come up with -- I'll look at this.
Solution to (2) is a bit different. Currently the callsite pages provide links to files in the code coverage reports, and the *.y file is not part of the code coverage report, if it was it should be here https://storage.googleapis.com/oss-fuzz-coverage/igraph/reports-by-target/20230226/read_dl_fuzzer/linux/src/igraph/src/io/report.html. As such, we need to either (2.1) include a link back to the source repository where the *.y file exists or (2.2) include the *.y file in the code coverage report, and get the coverage reporting utilities to use these. I think (2.1) is easier solvable because it's in our control, whereas (2.2) may not be fully in our control. I think coming up with (2.1) should be possible.