Commit 0d61117d authored by Jakob Kummerow's avatar Jakob Kummerow Committed by Commit Bot

[build] Disable -ftrivial-auto-var-init in Release mode

Considering that the security benefit is unclear at this point, the
performance and binary size costs are not justified.
This CL includes reverts of earlier partial disablings:
173a2bd8
af7bf14f
85f72be3

Bug: chromium:977230, chromium:1055312, chromium:1055317
Change-Id: I173b61656a542687c4619fa374a0b2ee22c85ef7
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2091474Reviewed-by: 's avatarClemens Backes <clemensb@chromium.org>
Reviewed-by: 's avatarMichael Achenbach <machenbach@chromium.org>
Reviewed-by: 's avatarMichael Hablich <hablich@chromium.org>
Commit-Queue: Clemens Backes <clemensb@chromium.org>
Commit-Queue: Jakob Kummerow <jkummerow@chromium.org>
Cr-Commit-Position: refs/heads/master@{#66623}
parent b097a8e5
......@@ -2984,12 +2984,15 @@ v8_source_set("v8_base_without_compiler") {
"src/utils/version.cc",
"src/utils/version.h",
"src/wasm/baseline/liftoff-assembler-defs.h",
"src/wasm/baseline/liftoff-assembler.cc",
"src/wasm/baseline/liftoff-assembler.h",
"src/wasm/baseline/liftoff-compiler.cc",
"src/wasm/baseline/liftoff-compiler.h",
"src/wasm/baseline/liftoff-register.h",
"src/wasm/compilation-environment.h",
"src/wasm/decoder.h",
"src/wasm/function-body-decoder-impl.h",
"src/wasm/function-body-decoder.cc",
"src/wasm/function-body-decoder.h",
"src/wasm/function-compiler.cc",
"src/wasm/function-compiler.h",
......@@ -3405,7 +3408,6 @@ v8_source_set("v8_base_without_compiler") {
":v8_libbase",
":v8_libsampler",
":v8_shared_internal_headers",
":v8_uninitialized",
":v8_version",
"src/inspector:inspector",
]
......@@ -3499,27 +3501,6 @@ v8_source_set("v8_base_without_compiler") {
}
}
# Disable -ftrivial-auto-var-init=pattern for this sources. Initialization
# can increase v8/Wasm validation time. See crbug.com/1055312.
v8_source_set("v8_uninitialized") {
visibility = [ ":*" ]
sources = [
"src/wasm/baseline/liftoff-assembler.cc",
"src/wasm/baseline/liftoff-compiler.cc",
"src/wasm/function-body-decoder.cc",
]
public_deps = [
":generate_bytecode_builtins_list",
":run_torque",
":v8_maybe_icu",
]
remove_configs = [ "//build/config/compiler:default_init_stack_vars" ]
configs = [ ":internal_config" ]
}
group("v8_base") {
public_deps = [
":v8_base_without_compiler",
......
......@@ -115,6 +115,13 @@ if (is_debug && !v8_optimized_debug) {
}
}
if (!is_debug) {
v8_remove_configs += [
# Too much performance impact, unclear security benefit.
"//build/config/compiler:default_init_stack_vars",
]
}
if (v8_code_coverage && !is_clang) {
v8_add_configs += [
v8_path_prefix + ":v8_gcov_coverage_cflags",
......
......@@ -105,45 +105,4 @@
#define V8_NOEXCEPT
#endif
#if defined(__clang__)
#if __has_attribute(uninitialized)
// Attribute "uninitialized" disables -ftrivial-auto-var-init=pattern for
// the specified variable.
// Library-wide alternative is
// 'configs -= [ "//build/config/compiler:default_init_stack_vars" ]' in .gn
// file.
//
// See "init_stack_vars" in build/config/compiler/BUILD.gn and
// http://crbug.com/977230
// "init_stack_vars" is enabled for non-official builds and we hope to enable it
// in official build in 2020 as well. The flag writes fixed pattern into
// uninitialized parts of all local variables. In rare cases such initialization
// is undesirable and attribute can be used:
// 1. Degraded performance
// In most cases compiler is able to remove additional stores. E.g. if memory is
// never accessed or properly initialized later. Preserved stores mostly will
// not affect program performance. However if compiler failed on some
// performance critical code we can get a visible regression in a benchmark.
// 2. memset, memcpy calls
// Compiler may replace some memory writes with memset or memcpy calls. This is
// not -ftrivial-auto-var-init specific, but it can happen more likely with the
// flag. It can be a problem if code is not linked with C run-time library.
//
// Note: The flag is security risk mitigation feature. So in future the
// attribute uses should be avoided when possible. However to enable this
// mitigation on the most of the code we need to be less strict now and minimize
// number of exceptions later. So if in doubt feel free to use attribute, but
// please document the problem for someone who is going to cleanup it later.
// E.g. platform, bot, benchmark or test name in patch description or next to
// the attribute.
#define V8_STACK_UNINITIALIZED __attribute__((uninitialized))
#else // No attribute uninitialized
#define V8_STACK_UNINITIALIZED
#endif // attribute uninitialized
#else // Not clang
#define V8_STACK_UNINITIALIZED
#endif // clang
#endif // V8_BASE_COMPILER_SPECIFIC_H_
......@@ -2795,10 +2795,8 @@ RUNTIME_FUNCTION(Runtime_LoadElementWithInterceptor) {
Handle<InterceptorInfo> interceptor(receiver->GetIndexedInterceptor(),
isolate);
// Initialization significantly slows down indexed-getter.html of
// blink_perf.bindings on linux-perf. https://crbug.com/977230
V8_STACK_UNINITIALIZED PropertyCallbackArguments arguments(
isolate, interceptor->data(), *receiver, *receiver, Just(kDontThrow));
PropertyCallbackArguments arguments(isolate, interceptor->data(), *receiver,
*receiver, Just(kDontThrow));
Handle<Object> result = arguments.CallIndexedGetter(interceptor, index);
RETURN_FAILURE_IF_SCHEDULED_EXCEPTION(isolate);
......
......@@ -583,8 +583,7 @@ double InternalStringToDouble(Iterator current, EndMark end, int flags,
// The longest form of simplified number is: "-<significant digits>'.1eXXX\0".
const int kBufferSize = kMaxSignificantDigits + 10;
// Avoid slowdown in js-perf-test/Numbers. See crbug.com/1055317.
V8_STACK_UNINITIALIZED char buffer[kBufferSize];
char buffer[kBufferSize];
int buffer_pos = 0;
// Exponent will be adjusted if insignificant digits of the integer part
......
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