Commit 45ae9e0a authored by Maya Lekova's avatar Maya Lekova Committed by Commit Bot

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: 's avatarMichael Starzinger <mstarzinger@chromium.org>
Cr-Commit-Position: refs/heads/master@{#60099}
parent 6e98fa92
...@@ -21,7 +21,7 @@ PREREQUISITES ----------------------------------------------------------------- ...@@ -21,7 +21,7 @@ PREREQUISITES -----------------------------------------------------------------
1) Install Lua 5.1 1) Install Lua 5.1
2) Get LLVM 2.9 and Clang 2.9 sources and build them. 2) Get LLVM 8.0 and Clang 8.0 sources and build them.
Follow the instructions on http://clang.llvm.org/get_started.html. Follow the instructions on http://clang.llvm.org/get_started.html.
...@@ -60,3 +60,27 @@ warning. Messages "Failed to resolve v8::internal::Object" are benign and ...@@ -60,3 +60,27 @@ warning. Messages "Failed to resolve v8::internal::Object" are benign and
can be ignored. can be ignored.
If any errors were found driver exits with non-zero status. If any errors were found driver exits with non-zero status.
TROUBLESHOOTING ------------------------
gcmole is tighly coupled with the AST structure that Clang produces. Therefore
when upgrading to a newer Clang version, it might start producing bogus output
or completely stop outputting warnings. In such occasion, one might start the
debugging process by checking weather a new AST node type is introduced which
is currently not supported by gcmole. Insert the following code at the end of
the FunctionAnalyzer::VisitExpr method to see the unsupported AST class(es)
and the source position which generates them:
if (expr) {
clang::Stmt::StmtClass stmtClass = expr->getStmtClass();
d_.Report(clang::FullSourceLoc(expr->getExprLoc(), sm_),
d_.getCustomDiagID(clang::DiagnosticsEngine::Remark, "%0")) << stmtClass;
}
For instance, gcmole currently doesn't support AtomicExprClass statements
introduced for atomic operations.
A convenient way to observe the AST generated by Clang is to pass the following
flags when invoking clang++
-Xclang -ast-dump -fsyntax-only
// Copyright 2019 the V8 project authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "src/handles-inl.h"
#include "src/handles.h"
#include "src/isolate.h"
#include "src/objects/maybe-object.h"
#include "src/objects/object-macros.h"
namespace v8 {
namespace internal {
Handle<Object> CauseGC(Handle<Object> obj, Isolate* isolate) {
isolate->heap()->CollectGarbage(OLD_SPACE, GarbageCollectionReason::kTesting);
return obj;
}
void TwoArgumentsFunction(Object a, Object b) {
a->Print();
b->Print();
}
void TestTwoArguments(Isolate* isolate) {
Handle<JSObject> obj1 = isolate->factory()->NewJSObjectWithNullProto();
Handle<JSObject> obj2 = isolate->factory()->NewJSObjectWithNullProto();
TwoArgumentsFunction(*CauseGC(obj1, isolate), *CauseGC(obj2, isolate));
}
void TwoSizeTArgumentsFunction(size_t a, size_t b) {
USE(a);
USE(b);
}
void TestTwoSizeTArguments(Isolate* isolate) {
Handle<JSObject> obj1 = isolate->factory()->NewJSObjectWithNullProto();
Handle<JSObject> obj2 = isolate->factory()->NewJSObjectWithNullProto();
TwoSizeTArgumentsFunction(sizeof(*CauseGC(obj1, isolate)),
sizeof(*CauseGC(obj2, isolate)));
}
class SomeObject : public Object {
public:
void Method(Object a) { a->Print(); }
DECL_CAST(SomeObject)
OBJECT_CONSTRUCTORS(SomeObject, Object);
};
void TestMethodCall(Isolate* isolate) {
SomeObject obj;
Handle<SomeObject> so = handle(obj, isolate);
Handle<JSObject> obj1 = isolate->factory()->NewJSObjectWithNullProto();
so->Method(*CauseGC(obj1, isolate));
}
} // namespace internal
} // namespace v8
This diff is collapsed.
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment