Make sure gcmole detects issue in DisallowGarbageCollection scopes.
DisallowGarbageCollection is widely used in the codebase to document
code that doesn't allocate. However, this has the rather unexpected
side-effect that gcmole is not run when such a scope is active.
This CL changes the default behavior of gcmole to run even with
DisallowGarbageCollection scopes present. This will give us the best
results of both worlds, dynamic checks by the fuzzer, and static
analysis by gcmole.
To allow crazy local raw pointer operations there is a new
DisableGCMole scope that explicitly disables gcmole.
Change-Id: I0a78fb3b4ceaad35be9bcf7293d917a41f90c91f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2615419
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#72039}
Because of LocalHeap safepoints, our existing assert scopes don't
necessarily maintain the same guarantees as desired. In particular,
DisallowHeapAllocation no longer guarantees that objects don't move.
This patch transitions DisallowHeapAllocation to
DisallowGarbageCollection, to ensure that code using this scope is
also protected against safepoints.
Change-Id: I0411425884f6849982611205fb17bb072881c722
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2540547
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Peter Marshall <petermarshall@chromium.org>
Reviewed-by: Jakob Gruber <jgruber@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Reviewed-by: Tobias Tebbi <tebbi@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71319}
Add a "combination" assert scope class, which combines multiple existing
assert scopes. This will allow scopes with functional overlap, e.g.
DisallowGarbageCollection and DisallowHeapAllocation, to share an assert
type rather than rather than requiring users to remember to set both. To
demonstrate this, this redefines DisallowGarbageCollection to a
combination of DisallowHeapAllocation and a new DisallowSafepoints, and
some of the DCHECKs checking both are simplified to only check one or
the other, as appropriate.
The combination classes become subclasses of the existing assert scopes,
so that they can be used in their place as e.g. a function parameter,
e.g. DisallowGarbageCollection can be passed to a function expecting
const DisallowHeapAllocation&.
As a drive-by, this also changes the per-thread assert scopes to use a
bitmask, rather than a bool array, to store their per-thread data. The
per-isolate scopes already used a bitmask, so this unifies the
behaviour between the two.
Change-Id: I209e0a56f45e124c0ccadbd9fb77f39e070612fe
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2534814
Commit-Queue: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Igor Sheludko <ishell@chromium.org>
Reviewed-by: Georg Neis <neis@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71231}
GCMole mistakenly thought that GC guards such as DisallowHeapAllocation
covered the whole scope of the function they are declared in. This CL
fixes the false negatives and adds appropriate testing.
Bug: v8:10071
Change-Id: Iffb369977af90ca053a55ca8f451e037a4f460f2
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2497451
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70763}
GCMole now comes with the long forgotten use-after-free detection
enabled by default. The CL also improves error logging when test
expectations mismatch with the actual output and updates the hash
of GCMole to be used with the newly built version with enabled UAF
detection.
The CL also contains an ignore for isolate.cc due to inability to
fix a warning there and fixes a couple of UAF warnings.
Bug: v8:9680
Change-Id: I7a009ffd5f67b1b5437567691ca4235ea873de70
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2257236
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68505}
This should help reduce the number of false positives detected
by dead variable analysis.
Bug: v8:9680, chromium:1000635
Change-Id: Id2893dd5f26cad230dede96930a5caacc0272b64
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1924359
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65186}
This CL ensures we care only about our internal pointer types and not
about raw C++ pointers, because normally special care is taken when
storing raw pointers to the managed heap. Furthermore, checking for raw
pointers produces too many false positives in the dead variable
analysis.
Bug: v8:9680, chromium:1000635
Change-Id: Ica9ea1fe09b7456c011910a6886149b6dfdda1f5
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1924357
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65174}
Since this produces a few false positives, also implemented a whitelist
mechanism to not report them.
Also, add a couple of tests and implemented automated testing against
test-expectations file.
Bug: v8:9321
Change-Id: I2915a29fe1891e8bbc51118bbd95ae072c8de023
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1773243
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63456}
This updates the existing special casing of assignment operators by
gcmole to match for assignments of {HeapObject} instead of {HeapObject*}
variables. The former now uses the implicit C++ assignment operator call
instead of a primitive assignment binary operation.
Also removes the dead {handle_decl_name} field as a drive-by-fix.
R=mslekova@chromium.org
BUG=v8:8813
Change-Id: I0b48254e7ca1544bc064707a8ca1f204366ddbe4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1517879
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60212}
After introducing the new pointer-containing Object class in V8 (see
https://docs.google.com/document/d/1_w49sakC1XM1OptjTurBDqO86NE16FH8LwbeUAtrbCo/edit),
gcmole stopped finding errorneous usage of raw pointers in functions that could
trigger GC. This CL modifies the heuristics of the tool to classify Object and
MaybeObject instances as raw pointers, thus giving back the missing warnings.
Updated the gcmole implementation to support modern llvm (tested with llvm 8.0)
for which additional support for MaterializeTemporaryExpr, ExprWithCleanups and
UnaryExprOrTypeTraitExpr was needed.
Basic tests are added to make it harder to introduce such errors without
noticing in the future.
This version gives a lot of false positives when ran on the whole project, see
https://docs.google.com/document/d/1K7eJ0f6m9QX6FZIjZnt_GFtUsjEOC_LpiAwZbcAA3f8/editR=jkummerow@chromium.org,mstarzinger@chromium.org
Bug: v8:8813
Change-Id: Ic0190a4bc2642eda8880d9f7b30b5145a76a7d89
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1494754
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60099}
