UX improvements

[oliverchang]

Confusing call trees?

Looking at e.g.

https://oss-fuzz-introspector.storage.googleapis.com/jsoncpp/inspector-report/20220307/calltree_view_0.html

Why would calltree idx: 00001 be red but [calltree idx: 00002] be green?

Link to call tree from tables

The "call tree index" column should have a link to the actual call tree. Additionally, the "call tree index" value itself seems largely irrelevant (and potentially confusing) to users so we can just hide that number and just show a link to the call tree.

What does blocked nodes mean?

The "blocked nodes" column is also a little confusing as there is no explanation for what a "node" is.

"Functions hit" summary

The way this is presented is a little verbose and confusing, and the numerator used to compute the percentage isn't immediately obvious There is also a missing space after the colon on the first line.

Perhaps a better, more consistent way to present this information is:

Covered functions: 11 
Reachable functions: 56
Percentage of reachable functions covered: 19.64% 

Project functions overview.

The current table is very wide, and some columns (e.g. Arg count, Fuzzer reach count) seem like they can be omitted. If we want to retain fuzzer reach count, then maybe that can go into the dropdown text (View list (N)) for "Reached by fuzzers").

Some columns need more explanation as well. Such as "I Count, "BB Count" etc. These need a hover over text to explain what they are.

[oliverchang]

@AdamKorcz @DavidKorczynski @Navidem WDYT?

[DavidKorczynski]

Why would calltree idx: 00001 be red but [calltree idx: 00002] be green?

NB: I didn't investigate the specific example (will do) but there is a general explanation to this problem:

This can happen because runtime coverage is not correlated with control-flow: a function may be executed once during runtime extraction, but this does not mean all places that reference the function are executed. We make no efforts to analyse if coverage makes sense from a control-flow perspective as it brings other issues (see below).

For example, consider this fuzzer:

void C() {
  printf("Hello");
}

void D() {
  printf("D");
  C();  
} 

void A(size_t arg1, size_t arg2) {
  if (arg1 != arg2) {
    D();
  }
}

void B(size_t arg1, size_t arg2) {
  if (arg1 != arg2) {
    C();
  }
}

int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) {
  A(size, size);
  B(size, 3);
}

In this case D will never be reached, but C will be reached. Thus, when we visualise the calltree of the application C will be green but D will be red, despite C being a child of D (B too calls into C, which is what makes it green).

Fuzz-introspector used to have a heuristic that would analyse the control-flow and for each red node in the calltree would make the children of that node red as well. I decided to remove this as it brought some other issues, e.g. when looking at the coverage report and correlating with the calltree sometimes you would end up looking at code that was covered in the coverage report but the calltree would say it was red. As such, I switched it back to being fully verbose in the sense that the calltree shows control-flow and we simply overlay it with the runtime coverage data without doing any analysis on it.

[DavidKorczynski]

Some columns need more explanation as well. Such as "I Count, "BB Count" etc. These need a hover over text to explain what they are.

#63

[Navidem]

Thanks @oliverchang for your detailed feedback.

The "blocked nodes" column is also a little confusing as there is no explanation for what a "node" is.

My understanding is that at the current state blocked nodes are call-sites, we may find a better way to represent it.

Perhaps a better, more consistent way to present this information is:

This sounds easier to understand!

Project functions overview

We agree that this is a very verbose table at the moment, we plan to go through it and prune any duplicated and probably less informative fields. Examples can include "Git Link", number of fuzzer reached, Args etc.

[DavidKorczynski]

If we want to retain fuzzer reach count, then maybe that can go into the dropdown text (View list (N)) for "Reached by fuzzers").

Keeping fuzzer reach count is valuable from a perspective of rapidly sorting and getting all functions that have 0 reach count. We could perhaps merge it with the dropdown column but we should try to maintain the ability to sort to find non-reachable functions., e.g. by putting the count as the first character in the dropdown.

Alternatively we should split the table into multiple tables, perhaps one of which is solely focused on identifying good targets.

[DavidKorczynski]

"Functions hit" summary

The way this is presented is a little verbose and confusing, and the numerator used to compute the percentage isn't immediately obvious There is also a missing space after the colon on the first line.

Perhaps a better, more consistent way to present this information is:

Covered functions: 11 
Reachable functions: 56
Percentage of reachable functions covered: 19.64% 

Agreed: #69

[oliverchang]

Thanks a lot @DavidKorczynski for all the explanations!

Following up on this particular issue again:

Fuzz-introspector used to have a heuristic that would analyse the control-flow and for each red node in the calltree would make the children of that node red as well. I decided to remove this as it brought some other issues, e.g. when looking at the coverage report and correlating with the calltree sometimes you would end up looking at code that was covered in the coverage report but the calltree would say it was red. As such, I switched it back to being fully verbose in the sense that the calltree shows control-flow and we simply overlay it with the runtime coverage data without doing any analysis on it.

I still think this heuristic is useful. It seems rather confusing that the calltree (which shows control flow) would be coloured in a way that doesn't take the control flow into account.

Re starting with the coverage report and looking at the calltree and seeing that it's red -- it doesn't seem like too big of a downside given than a user can just look for other callsites where the function shows up as green.

Are there other complications this this?

[DavidKorczynski]

I think all of the cases are valid, in the sense that all cases tell something interesting about how a given project is being fuzzed. Thus, I think we should embrace them all and allow the user to switch between coloring in the UI. #112

[oliverchang]

I believe everything has been addressed apart from:

• adding a link to the call tree from the blocker table for the call idx column.
• reducing the # of columns for projects function overview. (e.g. moving the arg counts, fuzzers reach count into the respective columns).

Thoughts on the remaining parts here?

[DavidKorczynski]

• adding a link to the call tree from the blocker table for the call idx column.

@AdamKorcz could you fix this?