Summary: To avoid false positives, we treat `operator[]` in cpp as container read. Moreover, if a container `c` is owned, we make all accesses `c[i]` to be also owned.
Reviewed By: sblackshear
Differential Revision: D6396574
fbshipit-source-id: 94aabff
Summary:
Deduping issues when generating a single report and then diffing the
reports can lead to introduced issues being considered duplicates of
existing issues.
Reviewed By: sblackshear
Differential Revision: D6414673
fbshipit-source-id: bba81fd
Summary:
In C++ some modeled functions have definitions, which leads to traces
that contain an access from the modeling, but continue on into the
implementation of the modeled function. Such traces appear the same as
those that are truncated due to limitations of the buck integration in
the Java analysis. Since all Java models are for functions without
definitions in the code base, this diff limits the truncated trace
suppression to the Java analysis.
Reviewed By: sblackshear
Differential Revision: D6373793
fbshipit-source-id: 1f01509
Summary:
Naming a variable `_foo` makes the compiler not warn about them if they are
unused, but there are lots of instances of such variables in the code where
they are in fact used, defeating the warning and introducing confusion for
those used to this naming convention.
Basically `sed -i -e "s/ _\([a-zA-Z][a-zA-Z0-9_']*\)/ \1_/g" **/*.ml` followed
by manual fixing of compilation errors (lots of `compare__foo` ->
`compare_foo_`).
Reviewed By: mbouaziz
Differential Revision: D6358837
fbshipit-source-id: 7ffb4ac
Summary: In a thread safety report we used the access path from the final sink. This diffs change the report to include the expanded access path from the initial sink.
Reviewed By: sblackshear
Differential Revision: D6297848
fbshipit-source-id: 2386063
Summary:
Change ocamlformat installation procedure to use opam instead of
pinning.
Reformat all code with v0.2, which has a few improvements.
Reviewed By: jvillard
Differential Revision: D6292057
fbshipit-source-id: 759967f
Summary:
When fuzzy-matching cpp names, allow to match only a prefix of
blacklist entries.
Reviewed By: da319
Differential Revision: D6233055
fbshipit-source-id: a3a4913
Summary: We were conflating reads/writes with container reads/writes that created false positives.
Reviewed By: sblackshear
Differential Revision: D6232768
fbshipit-source-id: 39159cb
Summary:
When C++ functions are translated to SIL procedures, their type is C rather then C++. In RacerD, we want to treat C++ functions the same as C++ methods.
Added a function to check if the procedure is Objc/Objc++/C/C++.
Reviewed By: sblackshear
Differential Revision: D6209523
fbshipit-source-id: 293f938
Summary:
Seems it should have been done there all along.
The analyzer does not currently understand the implementation of
atomicity in folly::AtomicStruct.
The analyzer does not currently understand when std::atomic operations
are are used correctly versus incorrectly.
The analyzer does not currently understand that the representation of
folly::ThreadLocal is, ah, thread-local, leading to false alarms.
The analyzer does not currently understand the control flow /
scheduling constraints imposed by the implementation of Future.
It seems that the implementation of folly::Optional is more C++
template magic than the analyzer can currently understand.
The model of std::vector contains bogus memory accesses, leading to
false alarms.
Reviewed By: sblackshear
Differential Revision: D6226199
fbshipit-source-id: 8cb083b
Summary:
Destructors usually do not race with other methods.
We do not want to analyze or report on destructors.
Reviewed By: sblackshear
Differential Revision: D6222145
fbshipit-source-id: 5266622
Summary: More general version of the fix in D6138749. This diff moves RacerD's lock modeling into a separate module and uses the module in the HIL translation to check when a function has lock/unlock semantics.
Reviewed By: jberdine, da319
Differential Revision: D6191886
fbshipit-source-id: 6e1fdc3
Summary: Functions that do not belong to a class or a struct are translated to c-style functions even in the context of cpp. We need to add ownership to locals for c-style functions too.
Reviewed By: sblackshear
Differential Revision: D6196882
fbshipit-source-id: 715f129
Summary: `std::unique_lock` constructor allows to create a unique lock without locking the mutex. `std::unique_lock::try_lock` returns true if mutex has been acquired successfully, and false otherwise. It could be that an exception is being thrown while trying to acquire mutex, which is not modeled.
Reviewed By: jberdine
Differential Revision: D6185568
fbshipit-source-id: 192bf10
Summary:
The concurrency analyzer often does not understand object lifetimes
well enough to realize that destructors are usually not called in
parallel with any other methods. This leads to false alarms. This diff
suppresses these by simply skipping destructors in the concurrency
analysis.
Reviewed By: sblackshear
Differential Revision: D6182646
fbshipit-source-id: e9d1cac
Summary:
The clang frontend translates static locals incorrectly, in the sense
that the initializer is executed many times instead of once. This
leads to false alarms in the concurrency analysis. This diff
suppresses these by ignoring accesses to static locals.
Reviewed By: sblackshear
Differential Revision: D6182644
fbshipit-source-id: d8ca4c0
Summary:
Code often uses std::unique_lock::owns_lock to test if a deferred lock
using the 2-arg std::unique_lock constructor actually acquired the
lock.
Reviewed By: sblackshear
Differential Revision: D6181631
fbshipit-source-id: 11e9df2
Summary:
Use a distinct issue type for the Java and C++ concurrency analyses,
as the properties they are checking are significantly different.
Reviewed By: sblackshear
Differential Revision: D6151682
fbshipit-source-id: 00e00eb
Summary:
Due to limitations in our Buck integration, the thread-safety analysis cannot create a trace that bottoms out in a Buck target that is not a direct dependency of the current target.
These truncated traces are confusing and tough to act on.
Until we can address these limitations, let's avoid reporting on truncated traces.
Reviewed By: jeremydubreil
Differential Revision: D5969840
fbshipit-source-id: 877b9de
Summary:
Before this change, analyses using HIL needed to pass `IdAcessPathMapDomain.empty` to abstract interpreter, and would get back the map as part of the post.
This is a confusing API and was a pain point for Dino in trying to use HIL.
This diff adds a HIL wrapper around the abstract interpreter that hides these details.
It replaces `LowerHIL.makeDefault` as the new "simplest possible way" to use HIL.
Reviewed By: jberdine
Differential Revision: D6125597
fbshipit-source-id: 560856b
Summary:
Install ocamlformat from github as part of `make devsetup`, and use it
for formatting OCaml (and jbuild) code.
Reviewed By: jvillard
Differential Revision: D6092464
fbshipit-source-id: 4ba0845
Dulma Churchill
Daiva Naudziuniene
Josh Berdine
Sam Blackshear
Jules Villard
Peter O'Hearn
Philip Cohn-Cort