From 6e1b9141ecb74bcd83eac7a1a5eb6cc0ea7bae40 Mon Sep 17 00:00:00 2001 From: Jakob Linke Date: Wed, 4 Jan 2023 14:25:03 +0100 Subject: [PATCH] Reland "[flags,testrunner] Consider readonly flags for conflict detection" This is a reland of commit ebd933037eb61bb3626675bdf2de800ba9f2518d Original change's description: > [flags,testrunner] Consider readonly flags for conflict detection > > Flag conflict detection 1) bails out on incompatible flag values (e.g. > --jitless and --turbofan) and 2) handles such bailouts transparently in > the test runner by marking affected tests as OUTCOMES_FAIL. > > This CL adds full support for readonly flags to this system, together > with required additional annotations in variants.py. > > Drive-by: assert proper use of v8_enable_slow_dchecks, and add > support when dcheck_always_on is set. > Drive-by: introduce has_maglev build variable detection based on > v8_enable_maglev and use that for .status file annotations. > Drive-by: protect against unintended overwrites of build variables > in statusfile.py. > > Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel > Bug: v8:13629,v8:10577 > Change-Id: I04de399139a0490806df8bfee7e75e2ec767b4b5 > Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4135879 > Reviewed-by: Tobias Tebbi > Reviewed-by: Victor Gomes > Commit-Queue: Jakob Linke > Cr-Commit-Position: refs/heads/main@{#85130} Bug: v8:13629,v8:10577 Change-Id: I49ce322c3fda00a1e1e280d99d2d818772533927 Cq-Include-Trybots: luci.v8.try:v8_linux64_fyi_rel Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/4151087 Reviewed-by: Victor Gomes Commit-Queue: Victor Gomes Auto-Submit: Jakob Linke Reviewed-by: Leszek Swirski Cr-Commit-Position: refs/heads/main@{#85172} --- BUILD.gn | 42 +++++---- bazel/defs.bzl | 8 ++ src/flags/flag-definitions.h | 30 ++++--- src/flags/flags.cc | 86 ++++++++++++++++--- src/flags/flags.h | 1 + src/runtime/runtime-test.cc | 3 +- test/mjsunit/mjsunit.status | 12 ++- tools/testrunner/base_runner.py | 21 +++++ tools/testrunner/build_config.py | 55 +++++++----- tools/testrunner/local/statusfile.py | 13 ++- tools/testrunner/local/variants.py | 83 ++++++++++++++---- tools/testrunner/standard_runner_test.py | 22 +++-- .../testroot1/out/build/v8_build_config.json | 10 ++- .../testroot2/out/build/v8_build_config.json | 10 ++- .../testroot3/out/build/v8_build_config.json | 10 ++- .../out.gn/build/v8_build_config.json | 10 ++- .../testroot6/out/build/v8_build_config.json | 10 ++- 17 files changed, 324 insertions(+), 102 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index 47bcf501ca..7a17a413f9 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -516,6 +516,10 @@ assert(!v8_enable_trace_ignition || v8_enable_trace_unoptimized, "Ignition tracing requires unoptimized tracing to be enabled.") assert(!v8_enable_trace_baseline_exec || v8_enable_trace_unoptimized, "Baseline tracing requires unoptimized tracing to be enabled.") +assert( + v8_enable_debugging_features == true || dcheck_always_on || + !v8_enable_slow_dchecks, + "v8_enable_slow_dchecks requires v8_enable_debugging_features or dcheck_always_on.") if (v8_enable_short_builtin_calls && (!v8_enable_pointer_compression && v8_current_cpu != "x64")) { @@ -1299,13 +1303,12 @@ config("toolchain") { if ((is_linux || is_chromeos) && v8_enable_backtrace) { ldflags += [ "-rdynamic" ] } - + } + if (v8_enable_debugging_features || dcheck_always_on) { defines += [ "DEBUG" ] if (v8_enable_slow_dchecks) { defines += [ "ENABLE_SLOW_DCHECKS" ] } - } else if (dcheck_always_on) { - defines += [ "DEBUG" ] } if (v8_enable_verify_csa) { @@ -2314,6 +2317,7 @@ action("v8_dump_build_config") { script = "tools/testrunner/utils/dump_build_config.py" outputs = [ "$root_out_dir/v8_build_config.json" ] is_gcov_coverage = v8_code_coverage && !is_clang + is_DEBUG_defined = v8_enable_debugging_features || dcheck_always_on is_full_debug = v8_enable_debugging_features && !v8_optimized_debug args = [ rebase_path("$root_out_dir/v8_build_config.json", root_build_dir), @@ -2325,38 +2329,44 @@ action("v8_dump_build_config") { "is_clang=$is_clang", "is_component_build=$is_component_build", "is_debug=$v8_enable_debugging_features", + "is_DEBUG_defined=$is_DEBUG_defined", "is_full_debug=$is_full_debug", "is_gcov_coverage=$is_gcov_coverage", "is_msan=$is_msan", "is_tsan=$is_tsan", "is_ubsan_vptr=$is_ubsan_vptr", "target_cpu=\"$target_cpu\"", + "v8_code_comments=$v8_code_comments", + "v8_control_flow_integrity=$v8_control_flow_integrity", "v8_current_cpu=\"$v8_current_cpu\"", + "v8_dict_property_const_tracking=$v8_dict_property_const_tracking", + "v8_disable_write_barriers=$v8_disable_write_barriers", "v8_enable_atomic_object_field_writes=" + "$v8_enable_atomic_object_field_writes", + "v8_enable_cet_shadow_stack=$v8_enable_cet_shadow_stack", + "v8_enable_concurrent_marking=$v8_enable_concurrent_marking", "v8_enable_conservative_stack_scanning=" + "$v8_enable_conservative_stack_scanning", - "v8_enable_concurrent_marking=$v8_enable_concurrent_marking", - "v8_enable_single_generation=$v8_enable_single_generation", + "v8_enable_debug_code=$v8_enable_debug_code", + "v8_enable_disassembler=$v8_enable_disassembler", + "v8_enable_gdbjit=$v8_enable_gdbjit", "v8_enable_i18n_support=$v8_enable_i18n_support", - "v8_enable_verify_predictable=$v8_enable_verify_predictable", - "v8_enable_verify_csa=$v8_enable_verify_csa", "v8_enable_lite_mode=$v8_enable_lite_mode", - "v8_enable_runtime_call_stats=$v8_enable_runtime_call_stats", + "v8_enable_maglev=$v8_enable_maglev", "v8_enable_pointer_compression=$v8_enable_pointer_compression", "v8_enable_pointer_compression_shared_cage=" + "$v8_enable_pointer_compression_shared_cage", + "v8_enable_runtime_call_stats=$v8_enable_runtime_call_stats", "v8_enable_sandbox=$v8_enable_sandbox", "v8_enable_shared_ro_heap=$v8_enable_shared_ro_heap", - "v8_disable_write_barriers=$v8_disable_write_barriers", - "v8_enable_third_party_heap=$v8_enable_third_party_heap", - "v8_enable_webassembly=$v8_enable_webassembly", - "v8_dict_property_const_tracking=$v8_dict_property_const_tracking", - "v8_control_flow_integrity=$v8_control_flow_integrity", - "v8_target_cpu=\"$v8_target_cpu\"", - "v8_enable_cet_shadow_stack=$v8_enable_cet_shadow_stack", - "v8_enable_verify_heap=$v8_enable_verify_heap", + "v8_enable_single_generation=$v8_enable_single_generation", "v8_enable_slow_dchecks=$v8_enable_slow_dchecks", + "v8_enable_third_party_heap=$v8_enable_third_party_heap", + "v8_enable_verify_csa=$v8_enable_verify_csa", + "v8_enable_verify_heap=$v8_enable_verify_heap", + "v8_enable_verify_predictable=$v8_enable_verify_predictable", + "v8_enable_webassembly=$v8_enable_webassembly", + "v8_target_cpu=\"$v8_target_cpu\"", ] if (v8_current_cpu == "mips64" || v8_current_cpu == "mips64el") { diff --git a/bazel/defs.bzl b/bazel/defs.bzl index 870e7268df..facf35803e 100644 --- a/bazel/defs.bzl +++ b/bazel/defs.bzl @@ -535,6 +535,14 @@ def build_config_content(cpu, icu): ("v8_enable_shared_ro_heap", "false"), ("v8_disable_write_barriers", "false"), ("v8_target_cpu", cpu), + ("v8_code_comments", "false"), + ("v8_enable_debug_code", "false"), + ("v8_enable_verify_heap", "false"), + ("v8_enable_slow_dchecks", "false"), + ("v8_enable_maglev", "false"), + ("v8_enable_disassembler", "false"), + ("is_DEBUG_defined", "false"), + ("v8_enable_gdbjit", "false"), ]) # TODO(victorgomes): Create a rule (instead of a macro), that can diff --git a/src/flags/flag-definitions.h b/src/flags/flag-definitions.h index b795acb382..4659c2df73 100644 --- a/src/flags/flag-definitions.h +++ b/src/flags/flag-definitions.h @@ -42,12 +42,18 @@ #elif defined(FLAG_MODE_DEFINE_DEFAULTS) #define FLAG_FULL(ftype, ctype, nam, def, cmt) \ static constexpr ctype FLAGDEFAULT_##nam{def}; +#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \ + static constexpr ctype FLAGDEFAULT_##nam{def}; // We want to write entries into our meta data table, for internal parsing and -// printing / etc in the flag parser code. We only do this for writable flags. +// printing / etc in the flag parser code. #elif defined(FLAG_MODE_META) #define FLAG_FULL(ftype, ctype, nam, def, cmt) \ {Flag::TYPE_##ftype, #nam, &v8_flags.nam, &FLAGDEFAULT_##nam, cmt, false}, +// Readonly flags don't pass the value pointer since the struct expects a +// mutable value. That's okay since the value always equals the default. +#define FLAG_READONLY(ftype, ctype, nam, def, cmt) \ + {Flag::TYPE_##ftype, #nam, nullptr, &FLAGDEFAULT_##nam, cmt, false}, #define FLAG_ALIAS(ftype, ctype, alias, nam) \ {Flag::TYPE_##ftype, #alias, &v8_flags.nam, &FLAGDEFAULT_##nam, \ "alias for --" #nam, false}, // NOLINT(whitespace/indent) @@ -56,20 +62,20 @@ #elif defined(FLAG_MODE_DEFINE_IMPLICATIONS) #define DEFINE_VALUE_IMPLICATION(whenflag, thenflag, value) \ changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \ - &v8_flags.thenflag, value, false); + &v8_flags.thenflag, #thenflag, value, false); // A weak implication will be overwritten by a normal implication or by an // explicit flag. #define DEFINE_WEAK_VALUE_IMPLICATION(whenflag, thenflag, value) \ changed |= TriggerImplication(v8_flags.whenflag, #whenflag, \ - &v8_flags.thenflag, value, true); + &v8_flags.thenflag, #thenflag, value, true); #define DEFINE_GENERIC_IMPLICATION(whenflag, statement) \ if (v8_flags.whenflag) statement; #define DEFINE_NEG_VALUE_IMPLICATION(whenflag, thenflag, value) \ changed |= TriggerImplication(!v8_flags.whenflag, "!" #whenflag, \ - &v8_flags.thenflag, value, false); + &v8_flags.thenflag, #thenflag, value, false); // We apply a generic macro to the flags. #elif defined(FLAG_MODE_APPLY) @@ -772,6 +778,7 @@ DEFINE_BOOL( stress_concurrent_inlining, false, "create additional concurrent optimization jobs but throw away result") DEFINE_IMPLICATION(stress_concurrent_inlining, concurrent_recompilation) +DEFINE_IMPLICATION(stress_concurrent_inlining, turbofan) DEFINE_NEG_IMPLICATION(stress_concurrent_inlining, lazy_feedback_allocation) DEFINE_WEAK_VALUE_IMPLICATION(stress_concurrent_inlining, interrupt_budget, 15 * KB) @@ -2246,18 +2253,17 @@ DEFINE_NEG_IMPLICATION(perf_prof, compact_code_space) DEFINE_NEG_IMPLICATION(perf_prof, write_protect_code_memory) // --perf-prof-unwinding-info is available only on selected architectures. -#if !V8_TARGET_ARCH_ARM && !V8_TARGET_ARCH_ARM64 && !V8_TARGET_ARCH_X64 && \ - !V8_TARGET_ARCH_S390X && !V8_TARGET_ARCH_PPC64 -#undef DEFINE_PERF_PROF_BOOL -#define DEFINE_PERF_PROF_BOOL(nam, cmt) DEFINE_BOOL_READONLY(nam, false, cmt) -#undef DEFINE_PERF_PROF_IMPLICATION -#define DEFINE_PERF_PROF_IMPLICATION(...) -#endif - +#if V8_TARGET_ARCH_ARM || V8_TARGET_ARCH_ARM64 || V8_TARGET_ARCH_X64 || \ + V8_TARGET_ARCH_S390X || V8_TARGET_ARCH_PPC64 DEFINE_PERF_PROF_BOOL( perf_prof_unwinding_info, "Enable unwinding info for perf linux profiler (experimental).") DEFINE_PERF_PROF_IMPLICATION(perf_prof, perf_prof_unwinding_info) +#else +DEFINE_BOOL_READONLY( + perf_prof_unwinding_info, false, + "Enable unwinding info for perf linux profiler (experimental).") +#endif #undef DEFINE_PERF_PROF_BOOL #undef DEFINE_PERF_PROF_IMPLICATION diff --git a/src/flags/flags.cc b/src/flags/flags.cc index ab66eca43b..2b9ae1dffe 100644 --- a/src/flags/flags.cc +++ b/src/flags/flags.cc @@ -44,6 +44,7 @@ static_assert(sizeof(FlagValues) % kMinimumOSPageSize == 0); // Define all of our flags default values. #define FLAG_MODE_DEFINE_DEFAULTS #include "src/flags/flag-definitions.h" // NOLINT(build/include) +#undef FLAG_MODE_DEFINE_DEFAULTS namespace { @@ -91,6 +92,10 @@ struct Flag { enum class SetBy { kDefault, kWeakImplication, kImplication, kCommandLine }; + constexpr bool IsAnyImplication(Flag::SetBy set_by) { + return set_by == SetBy::kWeakImplication || set_by == SetBy::kImplication; + } + FlagType type_; // What type of flag, bool, int, or string. const char* name_; // Name of the flag, ex "my_flag". void* valptr_; // Pointer to the global flag variable. @@ -178,39 +183,44 @@ struct Flag { } } + template + T GetDefaultValue() const { + return *reinterpret_cast(defptr_); + } + bool bool_default() const { DCHECK_EQ(TYPE_BOOL, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } int int_default() const { DCHECK_EQ(TYPE_INT, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } unsigned int uint_default() const { DCHECK_EQ(TYPE_UINT, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } uint64_t uint64_default() const { DCHECK_EQ(TYPE_UINT64, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } double float_default() const { DCHECK_EQ(TYPE_FLOAT, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } size_t size_t_default() const { DCHECK_EQ(TYPE_SIZE_T, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } const char* string_default() const { DCHECK_EQ(TYPE_STRING, type_); - return *reinterpret_cast(defptr_); + return GetDefaultValue(); } static bool ShouldCheckFlagContradictions() { @@ -244,6 +254,19 @@ struct Flag { MSVC_SUPPRESS_WARNING(4722) ~FatalError() { FATAL("%s.\n%s", str().c_str(), kHint); } }; + // Readonly flags cannot change value. + if (change_flag && IsReadOnly()) { + // Exit instead of abort for certain testing situations. + if (v8_flags.exit_on_contradictory_flags) base::OS::ExitProcess(0); + if (implied_by == nullptr) { + FatalError{} << "Contradictory value for readonly flag " + << FlagName{name()}; + } else { + DCHECK(IsAnyImplication(new_set_by)); + FatalError{} << "Contradictory value for readonly flag " + << FlagName{name()} << " implied by " << implied_by; + } + } // For bool flags, we only check for a conflict if the value actually // changes. So specifying the same flag with the same value multiple times // is allowed. @@ -302,28 +325,39 @@ struct Flag { break; } } + if (change_flag && IsReadOnly()) { + // Readonly flags must never change value. + return false; + } set_by_ = new_set_by; - if (new_set_by == SetBy::kImplication || - new_set_by == SetBy::kWeakImplication) { + if (IsAnyImplication(new_set_by)) { DCHECK_NOT_NULL(implied_by); implied_by_ = implied_by; } return change_flag; } + bool IsReadOnly() const { + // See the FLAG_READONLY definition for FLAG_MODE_META. + return valptr_ == nullptr; + } + template T GetValue() const { DCHECK_EQ(flag_type, type_); + if (IsReadOnly()) return GetDefaultValue(); return *reinterpret_cast*>(valptr_); } template void SetValue(T new_value, SetBy set_by) { DCHECK_EQ(flag_type, type_); - auto* flag_value = reinterpret_cast*>(valptr_); - bool change_flag = flag_value->value() != new_value; + bool change_flag = GetValue() != new_value; change_flag = CheckFlagChange(set_by, change_flag); - if (change_flag) *flag_value = new_value; + if (change_flag) { + DCHECK(!IsReadOnly()); + *reinterpret_cast*>(valptr_) = new_value; + } } // Compare this flag's current value against the default. @@ -395,6 +429,7 @@ struct Flag { Flag flags[] = { #define FLAG_MODE_META #include "src/flags/flag-definitions.h" // NOLINT(build/include) +#undef FLAG_MODE_META }; constexpr size_t kNumFlags = arraysize(flags); @@ -851,10 +886,11 @@ class ImplicationProcessor { // Called from {DEFINE_*_IMPLICATION} in flag-definitions.h. template bool TriggerImplication(bool premise, const char* premise_name, - FlagValue* conclusion_value, T value, + FlagValue* conclusion_value, + const char* conclusion_name, T value, bool weak_implication) { if (!premise) return false; - Flag* conclusion_flag = FindFlagByPointer(conclusion_value); + Flag* conclusion_flag = FindFlagByName(conclusion_name); if (!conclusion_flag->CheckFlagChange( weak_implication ? Flag::SetBy::kWeakImplication : Flag::SetBy::kImplication, @@ -873,6 +909,28 @@ class ImplicationProcessor { return true; } + // Called from {DEFINE_*_IMPLICATION} in flag-definitions.h. + template + bool TriggerImplication(bool premise, const char* premise_name, + const FlagValue* conclusion_value, + const char* conclusion_name, T value, + bool weak_implication) { + if (!premise) return false; + Flag* conclusion_flag = FindFlagByName(conclusion_name); + // Because this is the `const FlagValue*` overload: + DCHECK(conclusion_flag->IsReadOnly()); + if (!conclusion_flag->CheckFlagChange( + weak_implication ? Flag::SetBy::kWeakImplication + : Flag::SetBy::kImplication, + conclusion_value->value() != value, premise_name)) { + return false; + } + // Must equal the default value, otherwise CheckFlagChange should've + // returned false. + DCHECK_EQ(value, conclusion_flag->GetDefaultValue()); + return true; + } + void CheckForCycle() { // Make sure flag implications reach a fixed point within // {kMaxNumIterations} iterations. diff --git a/src/flags/flags.h b/src/flags/flags.h index 690492f078..18446c78bf 100644 --- a/src/flags/flags.h +++ b/src/flags/flags.h @@ -66,6 +66,7 @@ struct alignas(kMinimumOSPageSize) FlagValues { #define FLAG_MODE_DECLARE #include "src/flags/flag-definitions.h" // NOLINT(build/include) +#undef FLAG_MODE_DECLARE }; V8_EXPORT_PRIVATE extern FlagValues v8_flags; diff --git a/src/runtime/runtime-test.cc b/src/runtime/runtime-test.cc index 7e2fa20a00..a7a8f64cae 100644 --- a/src/runtime/runtime-test.cc +++ b/src/runtime/runtime-test.cc @@ -265,7 +265,8 @@ bool CanOptimizeFunction(CodeKind target_kind, Handle function, return CrashUnlessFuzzingReturnFalse(isolate); } - if (!v8_flags.turbofan) return false; + if (target_kind == CodeKind::TURBOFAN && !v8_flags.turbofan) return false; + if (target_kind == CodeKind::MAGLEV && !v8_flags.maglev) return false; if (function->shared().optimization_disabled() && function->shared().disabled_optimization_reason() == diff --git a/test/mjsunit/mjsunit.status b/test/mjsunit/mjsunit.status index 21b9724c18..a800b8fb7c 100644 --- a/test/mjsunit/mjsunit.status +++ b/test/mjsunit/mjsunit.status @@ -213,6 +213,12 @@ # Needs deterministic test helpers for concurrent maglev tiering. # TODO(jgruber,v8:7700): Implement ASAP. 'maglev/18': [SKIP], + + # --perf-prof is only available on Linux, and --perf-prof-unwinding-info only + # on selected architectures. + 'regress/wasm/regress-1032753': [PASS, ['system != linux', SKIP]], + 'regress/regress-913844': [PASS, + ['system != linux or arch not in (arm, arm64, x64, s390x, ppc64)', SKIP]], }], # ALWAYS ############################################################################## @@ -1324,11 +1330,9 @@ }], # no_harness ############################################################################## -['arch != x64 or not pointer_compression or variant in (nooptimization, jitless)', { - # Maglev is x64-only for now. - # TODO(v8:7700): Update as we extend support. +['not has_maglev', { 'maglev/*': [SKIP], -}], # arch != x64 or not pointer_compression or variant in (nooptimization, jitless) +}], # not has_maglev ############################################################################## ['arch != x64 or deopt_fuzzer', { diff --git a/tools/testrunner/base_runner.py b/tools/testrunner/base_runner.py index a88d6fec05..89bb8ae90e 100644 --- a/tools/testrunner/base_runner.py +++ b/tools/testrunner/base_runner.py @@ -552,6 +552,8 @@ class BaseTestRunner(object): sys.byteorder, "cfi_vptr": self.build_config.cfi_vptr, + "code_comments": + self.build_config.code_comments, "component_build": self.build_config.component_build, "conservative_stack_scanning": @@ -564,8 +566,12 @@ class BaseTestRunner(object): self.build_config.single_generation, "dcheck_always_on": self.build_config.dcheck_always_on, + "debug_code": + self.build_config.debug_code, "deopt_fuzzer": False, + "disassembler": + self.build_config.disassembler, "endurance_fuzzer": False, "gc_fuzzer": @@ -574,12 +580,23 @@ class BaseTestRunner(object): False, "gcov_coverage": self.build_config.gcov_coverage, + "gdbjit": + self.build_config.gdbjit, + # TODO(jgruber): Note this rename from maglev to has_maglev is required + # to avoid a name clash with the "maglev" variant. See also the TODO in + # statusfile.py (this really shouldn't be needed). + "has_maglev": + self.build_config.maglev, "has_webassembly": self.build_config.webassembly, "isolates": self.options.isolates, "is_clang": self.build_config.is_clang, + "is_debug": + self.build_config.is_debug, + "is_DEBUG_defined": + self.build_config.is_DEBUG_defined, "is_full_debug": self.build_config.is_full_debug, "interrupt_fuzzer": @@ -607,6 +624,8 @@ class BaseTestRunner(object): "simulator_run": self.build_config.simulator_run and not self.options.dont_skip_simulator_slow_tests, + "slow_dchecks": + self.build_config.slow_dchecks, "system": self.target_os, "third_party_heap": @@ -617,6 +636,8 @@ class BaseTestRunner(object): self.build_config.ubsan_vptr, "verify_csa": self.build_config.verify_csa, + "verify_heap": + self.build_config.verify_heap, "lite_mode": self.build_config.lite_mode, "pointer_compression": diff --git a/tools/testrunner/build_config.py b/tools/testrunner/build_config.py index cdc11681f8..10b5749c5f 100644 --- a/tools/testrunner/build_config.py +++ b/tools/testrunner/build_config.py @@ -23,40 +23,46 @@ class BuildConfig(object): self.asan = build_config['is_asan'] self.cfi_vptr = build_config['is_cfi'] + self.code_comments = build_config['v8_code_comments'] self.component_build = build_config['is_component_build'] + self.concurrent_marking = build_config['v8_enable_concurrent_marking'] self.conservative_stack_scanning = build_config[ 'v8_enable_conservative_stack_scanning'] self.control_flow_integrity = build_config['v8_control_flow_integrity'] - self.concurrent_marking = build_config['v8_enable_concurrent_marking'] - self.single_generation = build_config['v8_enable_single_generation'] self.dcheck_always_on = build_config['dcheck_always_on'] + self.debug_code = build_config['v8_enable_debug_code'] + self.dict_property_const_tracking = build_config[ + 'v8_dict_property_const_tracking'] + self.disassembler = build_config['v8_enable_disassembler'] self.gcov_coverage = build_config['is_gcov_coverage'] + self.gdbjit = build_config['v8_enable_gdbjit'] self.is_android = build_config['is_android'] self.is_clang = build_config['is_clang'] self.is_debug = build_config['is_debug'] + self.is_DEBUG_defined = build_config['is_DEBUG_defined'] self.is_full_debug = build_config['is_full_debug'] + self.lite_mode = build_config['v8_enable_lite_mode'] + self.maglev = build_config['v8_enable_maglev'] self.msan = build_config['is_msan'] self.no_i18n = not build_config['v8_enable_i18n_support'] + self.pointer_compression = build_config['v8_enable_pointer_compression'] + self.pointer_compression_shared_cage = build_config[ + 'v8_enable_pointer_compression_shared_cage'] self.predictable = build_config['v8_enable_verify_predictable'] + self.sandbox = build_config['v8_enable_sandbox'] + self.shared_ro_heap = build_config['v8_enable_shared_ro_heap'] self.simulator_run = ( build_config['target_cpu'] != build_config['v8_target_cpu']) + self.single_generation = build_config['v8_enable_single_generation'] + self.slow_dchecks = build_config['v8_enable_slow_dchecks'] + self.third_party_heap = build_config['v8_enable_third_party_heap'] self.tsan = build_config['is_tsan'] # TODO(machenbach): We only have ubsan not ubsan_vptr. self.ubsan_vptr = build_config['is_ubsan_vptr'] self.verify_csa = build_config['v8_enable_verify_csa'] self.verify_heap = build_config['v8_enable_verify_heap'] - self.slow_dchecks = build_config['v8_enable_slow_dchecks'] - self.lite_mode = build_config['v8_enable_lite_mode'] - self.pointer_compression = build_config['v8_enable_pointer_compression'] - self.pointer_compression_shared_cage = build_config[ - 'v8_enable_pointer_compression_shared_cage'] - self.shared_ro_heap = build_config['v8_enable_shared_ro_heap'] - self.write_barriers = not build_config['v8_disable_write_barriers'] - self.sandbox = build_config['v8_enable_sandbox'] - self.third_party_heap = build_config['v8_enable_third_party_heap'] self.webassembly = build_config['v8_enable_webassembly'] - self.dict_property_const_tracking = build_config[ - 'v8_dict_property_const_tracking'] + self.write_barriers = not build_config['v8_disable_write_barriers'] # Export only for MIPS target if self.arch in ['mips64', 'mips64el']: self._mips_arch_variant = build_config['mips_arch_variant'] @@ -138,24 +144,31 @@ class BuildConfig(object): attrs = [ 'asan', 'cfi_vptr', + 'code_comments', 'control_flow_integrity', 'dcheck_always_on', + 'debug_code', + 'dict_property_const_tracking', + 'disassembler', 'gcov_coverage', + 'gdbjit', + 'is_debug', + 'is_DEBUG_defined', + 'lite_mode', + 'maglev', 'msan', 'no_i18n', + 'pointer_compression', + 'pointer_compression_shared_cage', 'predictable', + 'sandbox', + 'slow_dchecks', + 'third_party_heap', 'tsan', 'ubsan_vptr', 'verify_csa', - 'lite_mode', - 'pointer_compression', - 'pointer_compression_shared_cage', - 'sandbox', - 'third_party_heap', - 'webassembly', - 'dict_property_const_tracking', 'verify_heap', - 'slow_dchecks', + 'webassembly', ] detected_options = [attr for attr in attrs if getattr(self, attr, False)] return '\n'.join(detected_options) diff --git a/tools/testrunner/local/statusfile.py b/tools/testrunner/local/statusfile.py index 5f9766e85c..04485936b6 100644 --- a/tools/testrunner/local/statusfile.py +++ b/tools/testrunner/local/statusfile.py @@ -63,10 +63,12 @@ for var in [ "windows", "linux", "aix", "r1", "r2", "r3", "r5", "r6", "riscv32", "riscv64", "loong64" ]: + assert var not in VARIABLES VARIABLES[var] = var # Allow using variants as keywords. for var in ALL_VARIANTS: + assert var not in VARIABLES VARIABLES[var] = var class StatusFile(object): @@ -244,7 +246,16 @@ def ReadStatusFile(content, variables): prefix_rules = {variant: {} for variant in ALL_VARIANTS} prefix_rules[""] = {} - variables.update(VARIABLES) + # This method can be called with the same `variables` object multiple times. + # Ensure we only update `variables` (and check it for consistency) once. + if ALWAYS not in variables: + # Ensure we don't silently overwrite any build variables with our set of + # default keywords in VARIABLES. + for var in VARIABLES: + assert var not in variables, ( + "build_config variable '%s' conflicts with VARIABLES" % var) + variables.update(VARIABLES) + for conditional_section in ReadContent(content): assert type(conditional_section) == list assert len(conditional_section) == 2 diff --git a/tools/testrunner/local/variants.py b/tools/testrunner/local/variants.py index 31da71cf6d..d66f430c8b 100644 --- a/tools/testrunner/local/variants.py +++ b/tools/testrunner/local/variants.py @@ -58,23 +58,35 @@ ALL_VARIANT_FLAGS = { "google3_noicu": [[]], } +# Note these are specifically for the case when Turbofan is either fully +# disabled (i.e. not part of the binary), or when all codegen is disallowed (in +# jitless mode). +kIncompatibleFlagsForNoTurbofan = [ + "--turbofan", "--always-turbofan", "--liftoff", "--validate-asm", + "--maglev", "--stress-concurrent-inlining" +] + # Flags that lead to a contradiction with the flags provided by the respective # variant. This depends on the flags specified in ALL_VARIANT_FLAGS and on the # implications defined in flag-definitions.h. INCOMPATIBLE_FLAGS_PER_VARIANT = { - "jitless": [ - "--turbofan", "--always-turbofan", "--liftoff", "--track-field-types", - "--validate-asm", "--sparkplug", "--concurrent-sparkplug", "--maglev", - "--always-sparkplug", "--regexp-tier-up", "--no-regexp-interpret-all" + "jitless": + kIncompatibleFlagsForNoTurbofan + [ + "--track-field-types", "--sparkplug", "--concurrent-sparkplug", + "--always-sparkplug", "--regexp-tier-up", + "--no-regexp-interpret-all" + ], + "nooptimization": [ + "--turbofan", "--always-turbofan", "--stress-concurrent-inlining" ], - "nooptimization": ["--always-turbofan"], "slow_path": ["--no-force-slow-path"], "stress_concurrent_allocation": [ "--single-threaded", "--single-threaded-gc", "--predictable" ], "stress_concurrent_inlining": [ "--single-threaded", "--predictable", "--lazy-feedback-allocation", - "--assert-types", "--no-concurrent-recompilation" + "--assert-types", "--no-concurrent-recompilation", "--no-turbofan", + "--jitless" ], # The fast API tests initialize an embedder object that never needs to be # serialized to the snapshot, so we don't have a @@ -111,16 +123,55 @@ INCOMPATIBLE_FLAGS_PER_VARIANT = { # # applies when the code_comments build variable is NOT set. INCOMPATIBLE_FLAGS_PER_BUILD_VARIABLE = { - "lite_mode": ["--no-lazy-feedback-allocation", "--max-semi-space-size=*", - "--stress-concurrent-inlining"] - + INCOMPATIBLE_FLAGS_PER_VARIANT["jitless"], - "predictable": ["--parallel-compile-tasks-for-eager-toplevel", - "--parallel-compile-tasks-for-lazy", - "--concurrent-recompilation", - "--stress-concurrent-allocation", - "--stress-concurrent-inlining"], - "dict_property_const_tracking": [ - "--stress-concurrent-inlining"], + "!code_comments": ["--code-comments"], + "!is_DEBUG_defined": [ + "--check_handle_count", + "--code_stats", + "--dump_wasm_module", + "--enable_testing_opcode_in_wasm", + "--gc_verbose", + "--print_ast", + "--print_break_location", + "--print_global_handles", + "--print_handles", + "--print_scopes", + "--regexp_possessive_quantifier", + "--trace_backing_store", + "--trace_contexts", + "--trace_isolates", + "--trace_lazy", + "--trace_liftoff", + "--trace_module_status", + "--trace_normalization", + "--trace_turbo_escape", + "--trace_wasm_compiler", + "--trace_wasm_decoder", + "--trace_wasm_instances", + "--trace_wasm_interpreter", + "--trace_wasm_lazy_compilation", + "--trace_wasm_native_heap", + "--trace_wasm_serialization", + "--trace_wasm_stack_switching", + "--trace_wasm_streaming", + "--trap_on_abort", + ], + "!verify_heap": ["--verify-heap"], + "!debug_code": ["--debug-code"], + "!disassembler": [ + "--print_all_code", "--print_code", "--print_opt_code", + "--print_code_verbose", "--print_builtin_code", "--print_regexp_code" + ], + "!slow_dchecks": ["--enable-slow-asserts"], + "!gdbjit": ["--gdbjit", "--gdbjit_full", "--gdbjit_dump"], + "!maglev": ["--maglev"], + "lite_mode": ["--no-lazy-feedback-allocation", "--max-semi-space-size=*"] + + INCOMPATIBLE_FLAGS_PER_VARIANT["jitless"], + "predictable": [ + "--parallel-compile-tasks-for-eager-toplevel", + "--parallel-compile-tasks-for-lazy", "--concurrent-recompilation", + "--stress-concurrent-allocation", "--stress-concurrent-inlining" + ], + "dict_property_const_tracking": ["--stress-concurrent-inlining"], } # Flags that lead to a contradiction when a certain extra-flag is present. diff --git a/tools/testrunner/standard_runner_test.py b/tools/testrunner/standard_runner_test.py index 65d98b65ed..770a29bf88 100644 --- a/tools/testrunner/standard_runner_test.py +++ b/tools/testrunner/standard_runner_test.py @@ -231,18 +231,16 @@ class StandardRunnerTest(TestRunnerTest): v8_enable_sandbox=False ) ) - expect_text = ( - '>>> Autodetected:\n' - 'asan\n' - 'cfi_vptr\n' - 'dcheck_always_on\n' - 'msan\n' - 'no_i18n\n' - 'tsan\n' - 'ubsan_vptr\n' - 'webassembly\n' - '>>> Running tests for ia32.release') - result.stdout_includes(expect_text) + result.stdout_includes('>>> Autodetected:') + result.stdout_includes('asan') + result.stdout_includes('cfi_vptr') + result.stdout_includes('dcheck_always_on') + result.stdout_includes('msan') + result.stdout_includes('no_i18n') + result.stdout_includes('tsan') + result.stdout_includes('ubsan_vptr') + result.stdout_includes('webassembly') + result.stdout_includes('>>> Running tests for ia32.release') result.has_returncode(0) # TODO(machenbach): Test some more implications of the auto-detected # options, e.g. that the right env variables are set. diff --git a/tools/testrunner/testdata/testroot1/out/build/v8_build_config.json b/tools/testrunner/testdata/testroot1/out/build/v8_build_config.json index 9f1743780e..c4aa78f7f5 100644 --- a/tools/testrunner/testdata/testroot1/out/build/v8_build_config.json +++ b/tools/testrunner/testdata/testroot1/out/build/v8_build_config.json @@ -32,5 +32,13 @@ "v8_enable_single_generation": false, "v8_enable_third_party_heap": false, "v8_enable_webassembly": true, - "v8_dict_property_const_tracking": false + "v8_dict_property_const_tracking": false, + "v8_code_comments": false, + "v8_enable_debug_code": false, + "v8_enable_verify_heap": false, + "v8_enable_slow_dchecks": false, + "v8_enable_maglev": false, + "v8_enable_disassembler": false, + "is_DEBUG_defined": false, + "v8_enable_gdbjit": false } diff --git a/tools/testrunner/testdata/testroot2/out/build/v8_build_config.json b/tools/testrunner/testdata/testroot2/out/build/v8_build_config.json index a0b2cb87b4..3ad5534b05 100644 --- a/tools/testrunner/testdata/testroot2/out/build/v8_build_config.json +++ b/tools/testrunner/testdata/testroot2/out/build/v8_build_config.json @@ -32,5 +32,13 @@ "v8_enable_single_generation": false, "v8_enable_third_party_heap": false, "v8_enable_webassembly": true, - "v8_dict_property_const_tracking": false + "v8_dict_property_const_tracking": false, + "v8_code_comments": false, + "v8_enable_debug_code": false, + "v8_enable_verify_heap": false, + "v8_enable_slow_dchecks": false, + "v8_enable_maglev": false, + "v8_enable_disassembler": false, + "is_DEBUG_defined": false, + "v8_enable_gdbjit": false } diff --git a/tools/testrunner/testdata/testroot3/out/build/v8_build_config.json b/tools/testrunner/testdata/testroot3/out/build/v8_build_config.json index 9f1743780e..c4aa78f7f5 100644 --- a/tools/testrunner/testdata/testroot3/out/build/v8_build_config.json +++ b/tools/testrunner/testdata/testroot3/out/build/v8_build_config.json @@ -32,5 +32,13 @@ "v8_enable_single_generation": false, "v8_enable_third_party_heap": false, "v8_enable_webassembly": true, - "v8_dict_property_const_tracking": false + "v8_dict_property_const_tracking": false, + "v8_code_comments": false, + "v8_enable_debug_code": false, + "v8_enable_verify_heap": false, + "v8_enable_slow_dchecks": false, + "v8_enable_maglev": false, + "v8_enable_disassembler": false, + "is_DEBUG_defined": false, + "v8_enable_gdbjit": false } diff --git a/tools/testrunner/testdata/testroot5/out.gn/build/v8_build_config.json b/tools/testrunner/testdata/testroot5/out.gn/build/v8_build_config.json index 9f1743780e..c4aa78f7f5 100644 --- a/tools/testrunner/testdata/testroot5/out.gn/build/v8_build_config.json +++ b/tools/testrunner/testdata/testroot5/out.gn/build/v8_build_config.json @@ -32,5 +32,13 @@ "v8_enable_single_generation": false, "v8_enable_third_party_heap": false, "v8_enable_webassembly": true, - "v8_dict_property_const_tracking": false + "v8_dict_property_const_tracking": false, + "v8_code_comments": false, + "v8_enable_debug_code": false, + "v8_enable_verify_heap": false, + "v8_enable_slow_dchecks": false, + "v8_enable_maglev": false, + "v8_enable_disassembler": false, + "is_DEBUG_defined": false, + "v8_enable_gdbjit": false } diff --git a/tools/testrunner/testdata/testroot6/out/build/v8_build_config.json b/tools/testrunner/testdata/testroot6/out/build/v8_build_config.json index 9f1743780e..c4aa78f7f5 100644 --- a/tools/testrunner/testdata/testroot6/out/build/v8_build_config.json +++ b/tools/testrunner/testdata/testroot6/out/build/v8_build_config.json @@ -32,5 +32,13 @@ "v8_enable_single_generation": false, "v8_enable_third_party_heap": false, "v8_enable_webassembly": true, - "v8_dict_property_const_tracking": false + "v8_dict_property_const_tracking": false, + "v8_code_comments": false, + "v8_enable_debug_code": false, + "v8_enable_verify_heap": false, + "v8_enable_slow_dchecks": false, + "v8_enable_maglev": false, + "v8_enable_disassembler": false, + "is_DEBUG_defined": false, + "v8_enable_gdbjit": false }