C#: Introduce BarrierGuards#1807
Conversation
|
|
||
| /** A guard that checks if this expression is non-`null`. */ | ||
| class NullGuard extends DataFlow::BarrierGuards::ValueBarrierGuard { | ||
| NullGuard() { this.getCheckedValue() = any(AbstractValues::NullValue nv | not nv.isNull()) } |
There was a problem hiding this comment.
This won't work as all BarrierGuards must be disjoint. In this case there won't be a difference between NullGuard and AntiNullGuard.
There was a problem hiding this comment.
Good point; BarrierGuards are a bit fragile...
There was a problem hiding this comment.
Indeed they are. Until we get extensible IPA types with an agreed-upon syntax, I don't think there's much to do about that, though.
| override predicate isSanitizer(DataFlow::Node node) { node instanceof Sanitizer } | ||
|
|
||
| override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) { | ||
| guard instanceof BarrierGuard |
There was a problem hiding this comment.
I think this is confusing. I read it as if you were adding all BarrierGuards until I saw that you had defined another abstract class with the same name.
There was a problem hiding this comment.
The difference being that BarrierGuard is an already existing class. Having abstract class Sink extends DataFlow::Node is fine as is quite clear that there's an important distinction between Sink and Node, but doing abstract class BarrierGuard extends DataFlow::BarrierGuard is confusing since then you suddenly have a very important distinction between two classes with identical names.
There was a problem hiding this comment.
(I think my comment was lost, it was: Yeah, but this is the general pattern in C# queries, for extensibility.)
bc9a16b to
5f66b9e
Compare
|
Rebased to resolve conflict in change note. |
5f66b9e to
6e7ef66
Compare
|
Rebased to resolve conflict in change note (again). |
|
Here is the dist-compare report: https://git.semmle.com/gist/tom/75b4bdb0e981aa92f69770f64912116e. |
The equivalent of Java's #1718.