Page MenuHomePhabricator

[analyzer] Move `TaintBugVisitor` from `GenericTaintChecker.cpp` to `BugReporterVisitors.h`.
ClosedPublic

Authored by MTC on Apr 16 2018, 5:15 AM.

Details

Summary

TaintBugVisitor is a universal visitor, and many checkers rely on it, such as ArrayBoundCheckerV2.cpp, DivZeroChecker.cpp and VLASizeChecker.cpp. Moving TaintBugVisitor to BugReporterVisitors.h enables other checker can also track where tainted value came from.

Diff Detail

Repository
rC Clang

Event Timeline

MTC created this revision.Apr 16 2018, 5:15 AM

I'm new to the taint visitor, but I am quite confused by your change description.

and many checkers rely on it

How can other checkers rely on it if it's private to the taint checker?

Also, it's probably to explicitly include BugReporterVisitors.h in the checker file then.

MTC added a comment.Apr 22 2018, 6:54 AM

I'm new to the taint visitor, but I am quite confused by your change description.

and many checkers rely on it

How can other checkers rely on it if it's private to the taint checker?

Thanks for your review, george! TaintBugVisitor is an utility to add extra information to illustrate where the taint information originated from. There are several checkers use taint information, e.g. ArrayBoundCheckerV2.cpp, in some cases it will report a warning, like warning: Out of bound memory access (index is tainted). If TaintBugVisitor moves to BugReporterVisitors.h, ArrayBoundCheckerV2 can add extra notes like Taint originated here to the report by adding TaintBugVisitor.

Also, it's probably to explicitly include BugReporterVisitors.h in the checker file then.

If these checkers want to add Taint originated here using TaintBugVisitor, it is necessary to explicitly include BugReporterVisitors.h in following patch.

george.karpenkov accepted this revision.Apr 22 2018, 7:42 AM

can add extra notes like

Ah right, than it's "can rely on" rather than "rely on".

it is necessary to explicitly include

I've meant adding the include to GenericTaintChecker.cpp.

LGTM otherwise.

This revision is now accepted and ready to land.Apr 22 2018, 7:42 AM
This revision was automatically updated to reflect the committed changes.