Commit Graph

24 Commits

Author SHA1 Message Date
Camillo Bruni
d16a2a6884 [tools] Add DisableGCMole scope
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}
2021-01-12 12:59:39 +00:00
Leszek Swirski
f47e59e045 [asserts] Make assert scopes LocalHeap friendly
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}
2020-11-20 17:39:14 +00:00
Leszek Swirski
076687ab85 [asserts] Add combination assert scopes
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}
2020-11-17 13:27:46 +00:00
Leszek Swirski
7645037c67 [gcmole] Update to LLVM 9.0.1, bootstrap from git
This resolves issues building gcmole under LLVM 8.0.0 with gcc 10 (see
https://bugs.gentoo.org/708730).

Change-Id: I932def25fb3cc841cf21414a513fbe2a2fa8a8dc
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2539946
Reviewed-by: Tamer Tas <tmrts@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Commit-Queue: Tamer Tas <tmrts@chromium.org>
Cr-Commit-Position: refs/heads/master@{#71204}
2020-11-16 13:05:40 +00:00
Maya Lekova
87bc38e390 [gcmole] Fix false negatives with GC guards
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}
2020-10-26 17:02:00 +00:00
Dominik Inführ
dfcd257910 [gcmole] Add support for DisallowGarbageCollection
Add support for DisallowGarbageCollection scope. This scope will be
introduced once this CL landed.

DisallowGarbageCollection works like DisallowHeapAllocation but also
disallows safepoints.

Bug: v8:10315
Change-Id: Ia7d777d4104b32810dd481ad9dbdf0edd075b6cf
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2395561
Commit-Queue: Dominik Inführ <dinfuehr@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#69881}
2020-09-14 13:52:46 +00:00
Maya Lekova
e7606e6b69 [gcmole] Enable use-after-free detection
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}
2020-06-24 09:29:31 +00:00
Maya Lekova
c74e9596ec [gcmole] Add heap-snapshot-generator.cc to ignored_files
Bug: v8:9993
Change-Id: I06428edd7f01805d08927718e4711298c731d2e3
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2253844
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68450}
2020-06-22 08:40:29 +00:00
Maya Lekova
b45f718692 [gcmole] Enable extra logging for --dead-vars
Bug: v8:10009
Change-Id: Iccc42a9b5f9f7340851542185473ac49683c838c
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2253843
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#68430}
2020-06-19 10:06:35 +00:00
Maya Lekova
273f9abdb1 [gcmole] Whitelist entry for frames.cc
Enhance gcmole --verbose printing with the GC suspect.

Bug: v8:9986
Change-Id: Ia1454d4edec334eabb31a764583e4ee559f0fe07
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2207174
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#67867}
2020-05-18 14:06:20 +00:00
Michael Starzinger
b577c1fe95 Remove and update some outdated TODO(mstarzinger).
R=clemensb@chromium.org

Change-Id: Ibd6790a222590fd4dce9f918219a19f01c2e1e0f
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1960293
Reviewed-by: Clemens Backes <clemensb@chromium.org>
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#65439}
2019-12-13 10:18:04 +00:00
Maya Lekova
76fa37bc2a [gcmole] Make gcmole aware of DisallowHeapAllocation
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}
2019-11-26 18:43:56 +00:00
Maya Lekova
f8be16a0b4 [gcmole] Relax gcmole reasoning about raw pointers
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}
2019-11-26 13:55:01 +00:00
Maya Lekova
37a4937baf [gcmole] Fix traversing virtual methods
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}
2019-08-29 15:14:58 +00:00
Michael Starzinger
9af9a6b7e9 [gcmole] Switch assignment operator special case.
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}
2019-03-13 11:09:45 +00:00
Michael Starzinger
ce8a20382c [gcmole] Fix IsRawPointerType predicate.
R=mslekova@chromium.org
BUG=v8:8813

Change-Id: I4d35fdff0f398c4eac3a97932a5e6ce43a49d9c8
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1514683
Commit-Queue: Michael Starzinger <mstarzinger@chromium.org>
Reviewed-by: Maya Lekova <mslekova@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60171}
2019-03-11 16:37:57 +00:00
Maya Lekova
45ae9e0ae9 Update gcmole to work with llvm 8 and the new Object design
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/edit

R=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}
2019-03-07 15:22:22 +00:00
clemensh
4b0edcf7e0 [gcmole] Fixes for unreachable code
Enforce the invariant that unreachable implicates an empty live set and
fix the implementation of |= and &= operator.
This is a fix-up for http://crrev.com/2694103005.

R=vegorov@chromium.org
CC=mstarzinger@chromium.org, machenbach@chromium.org
BUG=v8:5970

Review-Url: https://codereview.chromium.org/2691103008
Cr-Commit-Position: refs/heads/master@{#43251}
2017-02-16 14:13:11 +00:00
clemensh
b8787e348d [gcmole] Avoid hardcoded maximum of 256 locals
This CL changes the datastructure to store live variables from a
std::bitset<256> to a std::vector<bool> to support an arbitrary number
of locals. Unfortunately, std::vector<bool> does not define |= and &=
operators, so I added them on the Environment class.

R=vegorov@chromium.org, mstarzinger@chromium.org, machenbach@chromium.org
BUG=v8:5970

Review-Url: https://codereview.chromium.org/2694103005
Cr-Commit-Position: refs/heads/master@{#43216}
2017-02-15 14:43:22 +00:00
svenpanne@chromium.org
6e75bfc35b Update gcmole to a more recent clang/llvm.
* Changes for 2.9:
     * Use CXX in Makefile instead of hardwired g++, we need a more
       modern GCC than 4.6 later, anyway.

* Changes for 3.0:
     * Use llvm namespace.
     * Diagnostic => DiagnosticsEngine.

* Changes for 3.1:
     * The BlockDeclRefExpr AST node is gone.
     * The structure of the CXXNewExpr AST node has changed.
     * Path changed from Release to Release+Asserts.
     * Use clang++ instead of -cc1, otherwise we lose the system include
       paths.

* Changes for 3.2:
     none needed

* Changes for 3.3:
     * Use lookup_iterator::begin/end instead of first/second.

* Changes for 3.4:
     * createItaniumMangleContext => ItaniumMangleContext::create.

* Changes for 3.5:
     * clang uses <type_traits> now, so -std=c++0x is needed.
     * Type-trait-related AST changes.
     * getCustomDiagID signature changed.
     * We must link the C++ library statically now.

R=mstarzinger@chromium.org

Review URL: https://codereview.chromium.org/445983002

git-svn-id: https://v8.googlecode.com/svn/branches/bleeding_edge@22972 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
2014-08-07 12:56:53 +00:00
mstarzinger@chromium.org
c46cbccefd Make it clear that GCMole depends on Clang 2.9 currently.
R=jkummerow@chromium.org

Review URL: https://codereview.chromium.org/13950017

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@14393 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
2013-04-23 12:48:59 +00:00
vegorov@chromium.org
9e87728572 In gcmole assume that methods exiting VM can cause GC.
Detect those methods by reference to StateTag::EXTERNAL in their bodies.

R=fschneider@google.com

Review URL: https://chromiumcodereview.appspot.com/9420046

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@10738 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
2012-02-17 11:34:50 +00:00
vegorov@chromium.org
850b106400 Extend GCMole with poor man's data flow analysis to catch dead raw pointer vars.
Fix various places in the code found by improved GCMole.

Review URL: http://codereview.chromium.org/6973063

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7895 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
2011-05-16 09:06:16 +00:00
vegorov@chromium.org
8578dfc6ce Add GCMole to the repository.
GCMole is a simple static analysis tool that searches for GC-usafe evaluation order dependent callsites.

Review URL: http://codereview.chromium.org/6812002

git-svn-id: http://v8.googlecode.com/svn/branches/bleeding_edge@7540 ce2b1a6d-e550-0410-aec6-3dcde31c8c00
2011-04-07 12:38:16 +00:00