Summary:
Developers will sometimes write GuardedBy("T.f") with the intended semantics: "guarded by the field f of the object with type T in the current state".
We want to support this to avoid false positives.
Reviewed By: peterogithub
Differential Revision: D4197476
fbshipit-source-id: acd00d9
Summary:
The way interfaces are dealt with led to a false positive,
where tryLock() works OK for a Lock but not for a ReentrantLock.
The solution is just to provide the model.
While I am at it I am adding some more standard tests for Lock and ReentrantLock, which were not present.
Reviewed By: sblackshear
Differential Revision: D4204551
fbshipit-source-id: 9b6de28
Summary: These are dangerous if you are trying to compare a type to a string, and they're also unsightly.
Reviewed By: jvillard
Differential Revision: D4189956
fbshipit-source-id: 14ce127
Summary:
SIOF is only for interactions between objects of non-POD types. Previously the
checker was also reporting for POD types.
Reviewed By: akotulski
Differential Revision: D4197620
fbshipit-source-id: 7c56571
Summary:
Record an abstraction of the bug traces in the tests. The abstraction of a
trace is the sequence of descriptions. In practice, descriptions are either
empty, or of the form "start/end/return from/call to procedure X". They seem
pretty stable.
Motivation: there is nothing testing the traces reported by Infer right now,
even though they are surfaced to developers. For instance, Quandary uses
--issues-txt instead of --issues-tests to make sure the traces do not regress.
This change would make this approach more widespread.
Reviewed By: sblackshear
Differential Revision: D4159597
fbshipit-source-id: 9c83952
Summary:
`make` doesn't delay variable evaluation in targets' dependencies, so
`$(OBJECTS)` was always empty. Including clang.make after having defined
`OBJECTS` fixes it.
Reviewed By: jberdine
Differential Revision: D4159522
fbshipit-source-id: 6925f8a
Summary:
When loading results from a json file, sort them. This prints results in some
sane order for both --issues-test and --issues-txt, removing the need for
post-processing of the result.
Reviewed By: cristianoc
Differential Revision: D4167029
fbshipit-source-id: 37e9f1c
Summary:
- rename java.make -> javac.make, config.make -> java.make, and move to infer/tests/ so it's easier to use from infer/tests/build_systems/
- use these from ant's test Makefile, much code reuse!
- factor out common functionality between java and clang
A wrinkle: sorting is now done the same way for --issues-tests and
--issues-txt, which produces bogus (but still as deterministic) sorting for
--issues-txt. This is more of a cosmetic issue, but I hope to fix it in a later
diff that gets rid of calls to `sort` in favour of sorting directly from
`InferPrint`.
Reviewed By: jberdine
Differential Revision: D4166841
fbshipit-source-id: ed6f232
Summary: The thread safety checker is run independently of other analyses, using the command "infer -a threadsafety -- <build-command>".
Reviewed By: sblackshear
Differential Revision: D4148553
fbshipit-source-id: bc7b3f9
Summary:
Our patch to Javalib has been accepted, so we can parse programs with invokedynamic!
invokedynamic still crashes Sawja, but I have worked around this by replacing all invokedynamic's with invokestatic's before passing them to Sawja.
This means we can handle everything about invokedynamic except calling the correct function (I call a dummy function with the correct signature for now).
We can try to actually call the right method in the future.
Reviewed By: jvillard
Differential Revision: D4160384
fbshipit-source-id: a8ef4e1
Summary: When searching for cast errors, types that were not Java objects, e.g. arrays of primitive types were not taken into account, leading to incorrect class cast excpetion reports.
Reviewed By: sblackshear
Differential Revision: D4166184
fbshipit-source-id: 7157c95
Summary:
This adds generic support for reporting error traces as usual infer issues
traces (instead of putting them in the textual description of the error) to
Trace.ml and SinkTrace.ml.
The siof checker is made to use these new traces, and gets an improved error
message mentioning the name of the problematic global as well, which requires a
slight API change in Pvar.re.
The support in Trace.ml is incomplete: passthroughs are ignored. This missing
feature will be needed by Quandary to migrate its error messages.
Reviewed By: sblackshear
Differential Revision: D4159542
fbshipit-source-id: 8c1101d
Summary:
- set SHELL to bash explicitly in Makefiles (Debian uses dash)
- avoid using system headers when using our own clang's headers in tests
- do not rely on the name of the object file to write the frontend debugging scripts. It turns out that `-o` is *not* always present in the arguments of `-cc1` functions so the `Option.get` could crash. Since we don't actually need to get the object file name, just a nice enough name, don't try to be smarter at guessing what object will be created and pick a different name built from the source name instead.
Reviewed By: akotulski
Differential Revision: D4159516
fbshipit-source-id: c7bc2b9
Summary: If a procedure is both a source and a sink for the same value, and it's a sink first, you will get a false positive when applying the summary for the procedure.
Reviewed By: cristianoc
Differential Revision: D4145246
fbshipit-source-id: 97f0022
Summary: `make test` was always exiting with exit code 0, even in the case of test failures. This is definitely not what we want.
Reviewed By: sblackshear
Differential Revision: D4154912
fbshipit-source-id: 87b4b2b
Summary: Mark native methods as defined so that the analysis generates a summary for those methods. When analyzing Java projects compiled with Buck, the summaries for the dependencies methods of are retrieved from the classpath. In this case, having access to the summary is useful to access the attributes of a callee when the callee is part of a, previously analyzed, Buck target.
Reviewed By: sblackshear
Differential Revision: D4141362
fbshipit-source-id: 75888c8
Summary:
Analyses should handle methods whose code is unknown and methods whose summary is a no-op differently.
Previously, this was done correctly for some kinds of methods (e.g., native methods, which were recognized as unknown), but not for others (interface and abstract methods).
This diff makes sure we correctly treat all three kinds as unknown.
Reviewed By: jeremydubreil
Differential Revision: D4142697
fbshipit-source-id: c88cff3
Summary:
Instead of the custom filtering done by `InferPrint --issues-tests`, use the
filtering done by `infer` and run without filtering for our e2e tests. We still
test the filtering for our build systems integration tests, and this diff
restores that behaviour for the ant test (hence the bugs removed from
ant/issues.exp).
Also add internal exceptions to most tests to get more signal out of them (eg,
knowing when we add assertion failures and the like).
Retire the old `--issues-tests` to limit the number of ways we do filtering.
Reviewed By: jeremydubreil
Differential Revision: D4131308
fbshipit-source-id: 35805cc
Summary:
This makes the tests depend on much fewer phony targets, thus reducing the need
to rerun the tests when nothing has changed.
Reviewed By: jberdine
Differential Revision: D4118457
fbshipit-source-id: 664b6e3
Summary:
Our default strategy for handling unknown code is to propagate taint from the actuals to the return value.
But for commonly-used methods like `StringBuilder.append` (used every time you do `+` with a string in Java), this doesn't work.
The taint should be propagated to both the receiver and the return value in these cases.
I'm considering a solution where we always propagate taint to the receiver of unknown functions in the future, but I am concerned about the performance.
So let's stick with a few special string cases for now.
Reviewed By: cristianoc
Differential Revision: D4124355
fbshipit-source-id: 5b2a232
Summary: A must-have for reporting taint errors and any other interprocedural error where the trace is sufficiently complex.
Reviewed By: jvillard
Differential Revision: D4124072
fbshipit-source-id: 26b3b2b
Summary: A must-have for reporting taint errors and any other interprocedural error where the trace is sufficiently complex.
Reviewed By: jvillard
Differential Revision: D4106352
fbshipit-source-id: b2677e6
Summary:
New version of clang plugin exports `-x` arg information as a part of
TranslationUnitDecl. Get it from there instead of reading it from
clang argv
Reviewed By: jvillard
Differential Revision: D4112652
fbshipit-source-id: 5c3af1f
Summary: We want to skip readwrite locks for now, maybe report on their misuses later.
Reviewed By: sblackshear
Differential Revision: D4110998
fbshipit-source-id: 986f77e
Summary:
Previously, we recorded direct sinks as sinks and transitive sinks as passthroughs. This makes it difficult to create an expanded interprocedural trace when recording an error because we can't distinguish between sinks (which we want to expand) and passthroughs (which we don't). This diff changes recording of sinks so that a sink is now the *last* function in a trace to call a sink. To find out what the original sink was, the summary for the transitive sink in the trace will now need to be (recursively) expanded until we bottom out in the original sink.
Will do the same for sources in a follow-up diff.
Reviewed By: cristianoc
Differential Revision: D4103759
fbshipit-source-id: 6f435f5
Summary:
Needed to support upcoming diff(s) that change the nature of sources/sinks in a trace. Today they are the *original* source/sink, but in the future they will be the *transitive* source/sink (last procedure to return a source/call a sink).
This new convention will make the `returnAllSources`/`callAllSinks` form of these tests not so useful, since `returnAllSources`/`callAllSinks` will now show up as a single source/sink in the trace (at least without expanding the trace). By making these tests intraprocedural, we can make sure that we're still testing everything that we want to.
Reviewed By: cristianoc
Differential Revision: D4103754
fbshipit-source-id: 1733ecf
Summary:
See code comment about `throw exn` being translated as `return exn`.
This problem was revealed by D4081279, which started grabbing access paths from exceptions.
Reviewed By: jvillard
Differential Revision: D4096391
fbshipit-source-id: 9d91513
Summary: Doing `sychronized(A.class)` where `A` is an inner class was not previously recognized by the `GuardedBy` checker.
Reviewed By: peterogithub
Differential Revision: D4095094
fbshipit-source-id: c832f9e
Dulma Churchill
Sam Blackshear
Peter O'Hearn
Jules Villard
Andrzej Kotulski
Jeremy Dubreil
Dino Distefano
Josh Berdine
Cristiano Calcagno
Ryan Rhee