Skip to content

Commit d16a2a6

Browse files
camillobruniCommit Bot
camillobruni
authored and
Commit Bot
committed
[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 <[email protected]> Reviewed-by: Maya Lekova <[email protected]> Commit-Queue: Camillo Bruni <[email protected]> Cr-Commit-Position: refs/heads/master@{#72039}
1 parent 2059ee8 commit d16a2a6

13 files changed

Lines changed: 254 additions & 127 deletions

‎src/builtins/builtins-array.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
#include "src/builtins/builtins-utils-inl.h"
77
#include "src/builtins/builtins.h"
88
#include "src/codegen/code-factory.h"
9+
#include "src/common/assert-scope.h"
910
#include "src/debug/debug.h"
1011
#include "src/execution/isolate.h"
1112
#include "src/execution/protectors-inl.h"
@@ -969,6 +970,7 @@ void CollectElementIndices(Isolate* isolate, Handle<JSObject> object,
969970
case FAST_SLOPPY_ARGUMENTS_ELEMENTS:
970971
case SLOW_SLOPPY_ARGUMENTS_ELEMENTS: {
971972
DisallowGarbageCollection no_gc;
973+
DisableGCMole no_gc_mole;
972974
FixedArrayBase elements = object->elements();
973975
JSObject raw_object = *object;
974976
ElementsAccessor* accessor = object->GetElementsAccessor();

‎src/common/assert-scope.cc‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@ template class PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT, false>;
8484
template class PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT, true>;
8585
template class PerThreadAssertScope<CODE_ALLOCATION_ASSERT, false>;
8686
template class PerThreadAssertScope<CODE_ALLOCATION_ASSERT, true>;
87+
template class PerThreadAssertScope<GC_MOLE, false>;
8788

8889
template class PerIsolateAssertScope<JAVASCRIPT_EXECUTION_ASSERT, false>;
8990
template class PerIsolateAssertScope<JAVASCRIPT_EXECUTION_ASSERT, true>;

‎src/common/assert-scope.h‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,9 @@ enum PerThreadAssertType {
2626
HANDLE_ALLOCATION_ASSERT,
2727
HANDLE_DEREFERENCE_ASSERT,
2828
CODE_DEPENDENCY_CHANGE_ASSERT,
29-
CODE_ALLOCATION_ASSERT
29+
CODE_ALLOCATION_ASSERT,
30+
// Dummy type for disabling GC mole.
31+
GC_MOLE,
3032
};
3133

3234
enum PerIsolateAssertType {
@@ -35,7 +37,7 @@ enum PerIsolateAssertType {
3537
JAVASCRIPT_EXECUTION_DUMP,
3638
DEOPTIMIZATION_ASSERT,
3739
COMPILATION_ASSERT,
38-
NO_EXCEPTION_ASSERT
40+
NO_EXCEPTION_ASSERT,
3941
};
4042

4143
template <PerThreadAssertType kType, bool kAllow>
@@ -191,6 +193,11 @@ using AllowCodeAllocation =
191193
// DisallowHeapAllocation by also forbidding safepoints.
192194
using DisallowGarbageCollection =
193195
CombinationAssertScope<DisallowSafepoints, DisallowHeapAllocation>;
196+
197+
// Scope to skip gc mole verification in places where we do tricky raw
198+
// work.
199+
using DisableGCMole = PerThreadAssertScopeDebugOnly<GC_MOLE, false>;
200+
194201
// The DISALLOW_GARBAGE_COLLECTION macro can be used to define a
195202
// DisallowGarbageCollection field in classes that isn't present in release
196203
// builds.
@@ -215,15 +222,6 @@ using AllowHeapAccess =
215222
CombinationAssertScope<AllowCodeDependencyChange, AllowHandleDereference,
216223
AllowHandleAllocation, AllowHeapAllocation>;
217224

218-
// The DISALLOW_GARBAGE_COLLECTION macro can be used to define a
219-
// DisallowSafepoints field in classes that isn't present in release
220-
// builds.
221-
#ifdef DEBUG
222-
#define DISALLOW_GARBAGE_COLLECTION(name) DisallowGarbageCollection name;
223-
#else
224-
#define DISALLOW_GARBAGE_COLLECTION(name)
225-
#endif
226-
227225
class DisallowHeapAccessIf {
228226
public:
229227
explicit DisallowHeapAccessIf(bool condition) {
@@ -328,6 +326,7 @@ extern template class PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT,
328326
extern template class PerThreadAssertScope<CODE_DEPENDENCY_CHANGE_ASSERT, true>;
329327
extern template class PerThreadAssertScope<CODE_ALLOCATION_ASSERT, false>;
330328
extern template class PerThreadAssertScope<CODE_ALLOCATION_ASSERT, true>;
329+
extern template class PerThreadAssertScope<GC_MOLE, false>;
331330

332331
extern template class PerIsolateAssertScope<JAVASCRIPT_EXECUTION_ASSERT, false>;
333332
extern template class PerIsolateAssertScope<JAVASCRIPT_EXECUTION_ASSERT, true>;

‎src/regexp/experimental/experimental.cc‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
#include "src/regexp/experimental/experimental.h"
66

7+
#include "src/common/assert-scope.h"
78
#include "src/objects/js-regexp-inl.h"
89
#include "src/regexp/experimental/experimental-compiler.h"
910
#include "src/regexp/experimental/experimental-interpreter.h"
@@ -145,6 +146,8 @@ int32_t ExecRawImpl(Isolate* isolate, RegExp::CallOrigin call_origin,
145146
int32_t* output_registers, int32_t output_register_count,
146147
int32_t subject_index) {
147148
DisallowGarbageCollection no_gc;
149+
// TODO(cbruni): remove once gcmole is fixed.
150+
DisableGCMole no_gc_mole;
148151

149152
int register_count_per_match =
150153
JSRegExp::RegistersForCaptureCount(capture_count);

‎src/regexp/regexp-macro-assembler.cc‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -204,22 +204,25 @@ int NativeRegExpMacroAssembler::CheckStackGuardState(
204204
bool is_one_byte = String::IsOneByteRepresentationUnderneath(*subject_handle);
205205
int return_value = 0;
206206

207-
if (js_has_overflowed) {
208-
AllowGarbageCollection yes_gc;
209-
isolate->StackOverflow();
210-
return_value = EXCEPTION;
211-
} else if (check.InterruptRequested()) {
212-
AllowGarbageCollection yes_gc;
213-
Object result = isolate->stack_guard()->HandleInterrupts();
214-
if (result.IsException(isolate)) return_value = EXCEPTION;
215-
}
207+
{
208+
DisableGCMole no_gc_mole;
209+
if (js_has_overflowed) {
210+
AllowGarbageCollection yes_gc;
211+
isolate->StackOverflow();
212+
return_value = EXCEPTION;
213+
} else if (check.InterruptRequested()) {
214+
AllowGarbageCollection yes_gc;
215+
Object result = isolate->stack_guard()->HandleInterrupts();
216+
if (result.IsException(isolate)) return_value = EXCEPTION;
217+
}
216218

217-
if (*code_handle != re_code) { // Return address no longer valid
218-
// Overwrite the return address on the stack.
219-
intptr_t delta = code_handle->address() - re_code.address();
220-
Address new_pc = old_pc + delta;
221-
// TODO(v8:10026): avoid replacing a signed pointer.
222-
PointerAuthentication::ReplacePC(return_address, new_pc, 0);
219+
if (*code_handle != re_code) { // Return address no longer valid
220+
// Overwrite the return address on the stack.
221+
intptr_t delta = code_handle->address() - re_code.address();
222+
Address new_pc = old_pc + delta;
223+
// TODO(v8:10026): avoid replacing a signed pointer.
224+
PointerAuthentication::ReplacePC(return_address, new_pc, 0);
225+
}
223226
}
224227

225228
// If we continue, we need to update the subject string addresses.

‎tools/gcmole/README‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,9 +35,9 @@ simply to store it as a Handle.
3535

3636
PREREQUISITES -----------------------------------------------------------------
3737

38-
(1) Install Lua 5.1
38+
(1) Install Python
3939

40-
$ sudo apt-get install lua5.1
40+
$ sudo apt-get install python
4141

4242
(2) Get LLVM 8.0 and Clang 8.0 sources and build them.
4343

@@ -62,14 +62,14 @@ PREREQUISITES -----------------------------------------------------------------
6262

6363
USING GCMOLE ------------------------------------------------------------------
6464

65-
gcmole consists of driver script written in Lua and Clang plugin that does
65+
gcmole consists of driver script written in Python and Clang plugin that does
6666
C++ AST processing. Plugin (libgcmole.so) is expected to be in the same
67-
folder as driver (gcmole.lua).
67+
folder as driver (gcmole.py).
6868

6969
To start analysis cd into the root of v8 checkout and execute the following
7070
command:
7171

72-
CLANG_BIN=<path-to-clang-bin-folder> lua tools/gcmole/gcmole.lua [<arch>]
72+
CLANG_BIN=<path-to-clang-bin-folder> python tools/gcmole/gcmole.py [<arch>]
7373

7474
where arch should be one of architectures supported by V8 (arm, ia32, x64).
7575

@@ -89,7 +89,7 @@ If any errors were found driver exits with non-zero status.
8989

9090
TESTING -----------------------------------------------------------------------
9191

92-
Tests are automatically run by the main lua runner. Expectations are in
92+
Tests are automatically run by the main python runner. Expectations are in
9393
test-expectations.txt and need to be updated whenever the sources of the tests
9494
in gcmole-test.cc are modified (line numbers also count).
9595

‎tools/gcmole/bootstrap.sh‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -83,34 +83,40 @@ set -x
8383

8484
NUM_JOBS=3
8585
if [[ "${OS}" = "Linux" ]]; then
86-
NUM_JOBS="$(grep -c "^processor" /proc/cpuinfo)"
86+
if [[ -e "/proc/cpuinfo" ]]; then
87+
NUM_JOBS="$(grep -c "^processor" /proc/cpuinfo)"
88+
else
89+
# Hack when running in chroot
90+
NUM_JOBS="32"
91+
fi
8792
elif [ "${OS}" = "Darwin" ]; then
8893
NUM_JOBS="$(sysctl -n hw.ncpu)"
8994
fi
9095

9196
# Build clang.
92-
if [ ! -e "${BUILD_DIR}" ]; then
93-
mkdir "${BUILD_DIR}"
94-
fi
95-
cd "${BUILD_DIR}"
96-
cmake -GNinja -DCMAKE_CXX_FLAGS="-static-libstdc++" -DLLVM_ENABLE_TERMINFO=OFF \
97-
-DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang \
98-
-DLLVM_ENABLE_Z3_SOLVER=OFF "${LLVM_PROJECT_DIR}/llvm"
99-
MACOSX_DEPLOYMENT_TARGET=10.5 ninja -j"${NUM_JOBS}"
100-
101-
# Strip the clang binary.
102-
STRIP_FLAGS=
103-
if [ "${OS}" = "Darwin" ]; then
104-
# See http://crbug.com/256342
105-
STRIP_FLAGS=-x
106-
fi
107-
strip ${STRIP_FLAGS} bin/clang
108-
cd -
97+
# if [ ! -e "${BUILD_DIR}" ]; then
98+
# mkdir "${BUILD_DIR}"
99+
# fi
100+
# cd "${BUILD_DIR}"
101+
# cmake -GNinja -DCMAKE_CXX_FLAGS="-static-libstdc++" -DLLVM_ENABLE_TERMINFO=OFF \
102+
# -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang \
103+
# -DLLVM_ENABLE_Z3_SOLVER=OFF "${LLVM_PROJECT_DIR}/llvm"
104+
# MACOSX_DEPLOYMENT_TARGET=10.5 ninja -j"${NUM_JOBS}"
105+
#
106+
# # Strip the clang binary.
107+
# STRIP_FLAGS=
108+
# if [ "${OS}" = "Darwin" ]; then
109+
# # See http://crbug.com/256342
110+
# STRIP_FLAGS=-x
111+
# fi
112+
# strip ${STRIP_FLAGS} bin/clang
113+
# cd -
109114

110115
# Build libgcmole.so
111116
make -C "${THIS_DIR}" clean
112117
make -C "${THIS_DIR}" LLVM_SRC_ROOT="${LLVM_PROJECT_DIR}/llvm" \
113-
CLANG_SRC_ROOT="${LLVM_PROJECT_DIR}/clang" BUILD_ROOT="${BUILD_DIR}" libgcmole.so
118+
CLANG_SRC_ROOT="${LLVM_PROJECT_DIR}/clang" \
119+
BUILD_ROOT="${BUILD_DIR}" libgcmole.so
114120

115121
set +x
116122

0 commit comments

Comments
 (0)