[gcmole] Fix and simplify test-run mode

The test-run mode was broken after output improvements and the
introduction of pathlib.

This fixes the string concatenation with paths and updates the test
output to match the status quo. This also changes the test-run mode
to run exclusively when the --test-run option is passed. Now it's
either a test run or a normal run. Like that we can add the test run
as a separate test step on a bot. If both are needed in sequence
for something, gcmole could be called twice.

Bug: v8:12660
Change-Id: I58179d50950fa76d8f66b974325a8fed84dc91b6
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4075727
Commit-Queue: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Leszek Swirski <leszeks@chromium.org>
Reviewed-by: Camillo Bruni <cbruni@chromium.org>
Cr-Commit-Position: refs/heads/main@{#84655}
This commit is contained in:
Michael Achenbach 2022-12-05 15:40:21 +01:00 committed by V8 LUCI CQ
parent 377888f565
commit 7136ea89d2
3 changed files with 213 additions and 61 deletions

View File

@ -6,12 +6,15 @@
# This is main driver for gcmole tool. See README for more details.
# Usage: CLANG_BIN=clang-bin-dir python tools/gcmole/gcmole.py [arm|arm64|ia32|x64]
from contextlib import contextmanager
from contextlib import redirect_stderr
from multiprocessing import cpu_count
from pathlib import Path
import argparse
import collections
import difflib
import io
import json
import os
import re
@ -221,7 +224,7 @@ def invoke_clang_plugin_for_each_file(filenames, plugin, plugin_args, options):
# -----------------------------------------------------------------------------
def build_file_list(options, for_test):
def build_file_list(options):
"""Calculates the list of source files to be checked with gcmole.
The list comprises all files from marked source sections in the
@ -236,7 +239,7 @@ def build_file_list(options, for_test):
Returns: List of file paths (of type Path).
"""
if for_test:
if options.test_run:
return [options.v8_root_dir / "tools/gcmole/gcmole-test.cc"]
result = []
gn_files = [
@ -431,8 +434,8 @@ def write_gcmole_results(collector, options, dst):
# Analysis
def check_correctness_for_arch(options, for_test):
files = build_file_list(options, for_test)
def check_correctness_for_arch(options):
files = build_file_list(options)
if not options.reuse_gcsuspects:
generate_gc_suspects(files, options)
@ -441,7 +444,6 @@ def check_correctness_for_arch(options, for_test):
processed_files = 0
errors_found = False
output = ""
log("Searching for evaluation order problems " +
(' and dead variables' if options.dead_vars else '') + "for" +
@ -459,26 +461,29 @@ def check_correctness_for_arch(options, for_test):
if not errors_found:
errors_found = re.search("^[^:]+:\d+:\d+: (warning|error)", stderr,
re.MULTILINE) is not None
if for_test:
output = output + stderr
else:
sys.stdout.write(stderr)
sys.stderr.write(stderr)
log("Done processing {} files.", processed_files)
log("Errors found" if errors_found else "No errors found")
return errors_found, output
return errors_found
def test_run(options):
def has_unexpected_errors(options, errors_found, file_io):
"""Returns True if error state isn't as expected, False otherwise.
In test-run mode, we expect certain errors and return False if expectations
are met.
"""
if not options.test_run:
return True
return errors_found
log("Test Run")
errors_found, output = check_correctness_for_arch(options, True)
output = file_io.getvalue()
if not errors_found:
log("Test file should produce errors, but none were found. Output:")
print(output)
return False
return True
new_file = options.out_dir / "test-expectations-gen.txt"
with open(new_file, "w") as f:
@ -494,9 +499,9 @@ def test_run(options):
print("#" * 79)
log("Output mismatch from running tests.")
log("Please run gcmole manually with --test-run --verbose.")
log("Expected: " + expected_file)
log("New: " + new_file)
log("*Diff:* " + diff_file)
log(f"Expected: {expected_file}")
log(f"New: {new_file}")
log(f"*Diff:* {diff_file}")
print("#" * 79)
for line in difflib.unified_diff(
expectations.splitlines(),
@ -509,17 +514,17 @@ def test_run(options):
print("#" * 79)
log("Full output")
log("Expected: " + expected_file)
log("Diff: " + diff_file)
log("*New:* " + new_file)
log(f"Expected: {expected_file}")
log(f"Diff: {diff_file}")
log(f"*New*: {new_file}")
print("#" * 79)
print(output)
print("#" * 79)
return False
return True
log("Tests ran successfully")
return True
return False
# =============================================================================
@ -637,15 +642,17 @@ def main(argv):
options.func(options)
def full_run(options):
any_errors_found = False
if not test_run(options):
any_errors_found = True
else:
errors_found, output = check_correctness_for_arch(options, False)
any_errors_found = any_errors_found or errors_found
@contextmanager
def maybe_redirect_stderr(options):
file_io = io.StringIO() if options.test_run else sys.stderr
with redirect_stderr(file_io) as f:
yield f
sys.exit(1 if any_errors_found else 0)
def full_run(options):
with maybe_redirect_stderr(options) as file_io:
errors_found = check_correctness_for_arch(options)
sys.exit(has_unexpected_errors(options, errors_found, file_io))
def verify_and_convert_dirs(parser, options, default_tools_gcmole_dir,

View File

@ -17,7 +17,8 @@ import gcmole
TESTDATA_PATH = os.path.join(
os.path.dirname(os.path.abspath(__file__)), 'testdata', 'v8')
Options = collections.namedtuple('Options', ['v8_root_dir', 'v8_target_cpu'])
Options = collections.namedtuple(
'Options', ['v8_root_dir', 'v8_target_cpu', 'test_run'])
def abs_test_file(f):
@ -27,13 +28,13 @@ def abs_test_file(f):
class FilesTest(unittest.TestCase):
def testFileList_for_testing(self):
options = Options(Path(TESTDATA_PATH), 'x64')
options = Options(Path(TESTDATA_PATH), 'x64', True)
self.assertEqual(
gcmole.build_file_list(options, True),
gcmole.build_file_list(options),
list(map(abs_test_file, ['tools/gcmole/gcmole-test.cc'])))
def testFileList_x64(self):
options = Options(Path(TESTDATA_PATH), 'x64')
options = Options(Path(TESTDATA_PATH), 'x64', False)
expected = [
'file1.cc',
'file2.cc',
@ -45,11 +46,11 @@ class FilesTest(unittest.TestCase):
'test/cctest/test-x64-file2.cc',
]
self.assertEqual(
gcmole.build_file_list(options, False),
gcmole.build_file_list(options),
list(map(abs_test_file, expected)))
def testFileList_arm(self):
options = Options(Path(TESTDATA_PATH), 'arm')
options = Options(Path(TESTDATA_PATH), 'arm', False)
expected = [
'file1.cc',
'file2.cc',
@ -59,7 +60,7 @@ class FilesTest(unittest.TestCase):
'arm/file2.cc',
]
self.assertEqual(
gcmole.build_file_list(options, False),
gcmole.build_file_list(options),
list(map(abs_test_file, expected)))

View File

@ -1,73 +1,217 @@
tools/gcmole/gcmole-test.cc:30:10: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:30:10: warning: Possibly stale variable due to GCs.
return obj;
^
tools/gcmole/gcmole-test.cc:48:3: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:28:20: note: Call might cause unexpected GC.
isolate->heap()->CollectGarbage(OLD_SPACE, GarbageCollectionReason::kTesting);
^
./src/heap/heap.h:983:21: note: GC call here.
V8_EXPORT_PRIVATE bool CollectGarbage(
^
tools/gcmole/gcmole-test.cc:48:3: warning: Possible problem with evaluation order with interleaved GCs.
TwoArgumentsFunction(*CauseGC(obj1, isolate), *CauseGC(obj2, isolate));
^
tools/gcmole/gcmole-test.cc:60:3: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:48:25: note: Call might cause unexpected GC.
TwoArgumentsFunction(*CauseGC(obj1, isolate), *CauseGC(obj2, isolate));
^
tools/gcmole/gcmole-test.cc:21:1: note: GC call here.
Handle<Object> CauseGC(Handle<Object> obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:60:3: warning: Possible problem with evaluation order with interleaved GCs.
TwoSizeTArgumentsFunction(sizeof(*CauseGC(obj1, isolate)),
^
tools/gcmole/gcmole-test.cc:85:7: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:60:37: note: Call might cause unexpected GC.
TwoSizeTArgumentsFunction(sizeof(*CauseGC(obj1, isolate)),
^
tools/gcmole/gcmole-test.cc:21:1: note: GC call here.
Handle<Object> CauseGC(Handle<Object> obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:85:7: warning: Possible problem with evaluation order with interleaved GCs.
so->Method(*CauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:87:7: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:85:15: note: Call might cause unexpected GC.
so->Method(*CauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:21:1: note: GC call here.
Handle<Object> CauseGC(Handle<Object> obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:87:7: warning: Possible problem with evaluation order with interleaved GCs.
so->Method(CauseGCRaw(*obj1, isolate));
^
tools/gcmole/gcmole-test.cc:131:14: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:85:15: note: Call might cause unexpected GC.
so->Method(*CauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:21:1: note: GC call here.
Handle<Object> CauseGC(Handle<Object> obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:131:14: warning: Possible problem with evaluation order with interleaved GCs.
so_handle->Method(*derived.VirtualCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:133:14: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:131:30: note: Call might cause unexpected GC.
so_handle->Method(*derived.VirtualCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:115:3: note: GC call here.
Handle<Object> VirtualCauseGC(Handle<Object> obj, Isolate* isolate) override {
^
tools/gcmole/gcmole-test.cc:133:14: warning: Possible problem with evaluation order with interleaved GCs.
so_handle->Method(*base->VirtualCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:154:14: warning: Possible problem with evaluation order.
tools/gcmole/gcmole-test.cc:131:30: note: Call might cause unexpected GC.
so_handle->Method(*derived.VirtualCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:115:3: note: GC call here.
Handle<Object> VirtualCauseGC(Handle<Object> obj, Isolate* isolate) override {
^
tools/gcmole/gcmole-test.cc:154:14: warning: Possible problem with evaluation order with interleaved GCs.
so_handle->Method(*SomeClass::StaticCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:164:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:154:22: note: Call might cause unexpected GC.
so_handle->Method(*SomeClass::StaticCauseGC(obj1, isolate));
^
tools/gcmole/gcmole-test.cc:140:3: note: GC call here.
static Handle<Object> StaticCauseGC(Handle<Object> obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:164:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:172:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:161:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:172:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:198:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:169:3: note: Call might cause unexpected GC.
Safepoint();
^
tools/gcmole/gcmole-test.cc:19:1: note: GC call here.
void Safepoint() { LocalHeap::Current()->Safepoint(); }
^
tools/gcmole/gcmole-test.cc:198:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:224:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:195:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:224:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:235:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:221:3: note: Call might cause unexpected GC.
Safepoint();
^
tools/gcmole/gcmole-test.cc:19:1: note: GC call here.
void Safepoint() { LocalHeap::Current()->Safepoint(); }
^
tools/gcmole/gcmole-test.cc:235:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:242:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:233:3: note: Call might cause unexpected GC.
Safepoint();
^
tools/gcmole/gcmole-test.cc:19:1: note: GC call here.
void Safepoint() { LocalHeap::Current()->Safepoint(); }
^
tools/gcmole/gcmole-test.cc:242:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:252:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:233:3: note: Call might cause unexpected GC.
Safepoint();
^
tools/gcmole/gcmole-test.cc:19:1: note: GC call here.
void Safepoint() { LocalHeap::Current()->Safepoint(); }
^
tools/gcmole/gcmole-test.cc:252:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:262:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:250:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:262:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:265:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:260:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:265:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:271:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:260:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:271:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:287:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:269:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:287:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:295:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:285:3: note: Call might cause unexpected GC.
TestGuardedDeadVarAnalysisNested(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:268:1: note: GC call here.
void TestGuardedDeadVarAnalysisNested(JSObject raw_obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:295:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:302:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:293:3: note: Call might cause unexpected GC.
TestGuardedDeadVarAnalysisNested(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:268:1: note: GC call here.
void TestGuardedDeadVarAnalysisNested(JSObject raw_obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:302:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:319:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:300:3: note: Call might cause unexpected GC.
TestGuardedDeadVarAnalysisNested(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:268:1: note: GC call here.
void TestGuardedDeadVarAnalysisNested(JSObject raw_obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:319:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:338:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:317:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:338:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:349:3: warning: Possibly dead variable.
tools/gcmole/gcmole-test.cc:334:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
tools/gcmole/gcmole-test.cc:349:3: warning: Possibly stale variable due to GCs.
raw_obj.Print();
^
tools/gcmole/gcmole-test.cc:345:3: note: Call might cause unexpected GC.
CauseGCRaw(raw_obj, isolate);
^
tools/gcmole/gcmole-test.cc:27:1: note: GC call here.
Object CauseGCRaw(Object obj, Isolate* isolate) {
^
24 warnings generated.