From 668246a1b573f4317a91f5eb96291263ba52d22a Mon Sep 17 00:00:00 2001 From: Jochen Eisinger Date: Wed, 3 May 2017 11:58:03 +0200 Subject: [PATCH] Reland "Make unittest link correctly again" This reverts commit 5db25a09065b0e839a8675f351d1c35952e73544. Original change's description: > Make unittest link correctly again > > Remains to port these fixes over to gyp. > > R=machenbach@chromium.org, jkummerow@chromium.org, mstarzinger@chromium.org > BUG=v8:6325 > > Change-Id: I3bebbc6d0ec52fcb60e3d51acd27e616f51d3dbb > Reviewed-on: https://chromium-review.googlesource.com/490108 > Commit-Queue: Jochen Eisinger > Reviewed-by: Clemens Hammacher > Reviewed-by: Michael Starzinger > Reviewed-by: Jakob Kummerow > Reviewed-by: Michael Achenbach > Cr-Commit-Position: refs/heads/master@{#45026} R=jkummerow@chromium.org TBR=mstarzinger@chromium.org,clemensh@chromium.org BUG=v8:6325 Change-Id: Ic3c0ffdf1f13045ea5a3929b720908e0b27a11c3 Reviewed-on: https://chromium-review.googlesource.com/494566 Reviewed-by: Jochen Eisinger Reviewed-by: Jakob Kummerow Commit-Queue: Jochen Eisinger Cr-Commit-Position: refs/heads/master@{#45056} --- BUILD.gn | 46 +++++++- PRESUBMIT.py | 14 ++- src/base/export-template.h | 163 +++++++++++++++++++++++++++++ src/compiler/simplified-operator.h | 3 +- src/objects.cc | 25 +++-- src/objects.h | 2 +- src/objects/dictionary.h | 9 ++ src/objects/hash-table.h | 5 +- src/utils.h | 2 +- src/v8.gyp | 1 + src/wasm/local-decl-encoder.h | 3 +- src/wasm/wasm-result.h | 2 +- test/unittests/BUILD.gn | 13 +-- 13 files changed, 258 insertions(+), 30 deletions(-) create mode 100644 src/base/export-template.h diff --git a/BUILD.gn b/BUILD.gn index e406cca795..e0ee3a0067 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -877,7 +877,6 @@ v8_source_set("v8_builtins_generators") { visibility = [ ":*", "test/cctest:*", - "test/unittests:*", ] deps = [ @@ -2431,6 +2430,7 @@ v8_component("v8_libbase") { "src/base/debug/stack_trace.h", "src/base/division-by-constant.cc", "src/base/division-by-constant.h", + "src/base/export-template.h", "src/base/file-utils.cc", "src/base/file-utils.h", "src/base/flags.h", @@ -2733,6 +2733,31 @@ if (is_component_build) { public_configs = [ ":external_config" ] } + + v8_component("v8_for_testing") { + testonly = true + + sources = [ + "src/v8dll-main.cc", + ] + + deps = [ + ":v8_dump_build_config", + ] + + public_deps = [ + ":v8_base", + ":v8_maybe_snapshot", + ] + + if (v8_use_snapshot) { + public_deps += [ ":v8_builtins_generators" ] + } + + configs = [ ":internal_config" ] + + public_configs = [ ":external_config" ] + } } else { group("v8") { deps = [ @@ -2746,6 +2771,25 @@ if (is_component_build) { public_configs = [ ":external_config" ] } + + group("v8_for_testing") { + testonly = true + + deps = [ + ":v8_dump_build_config", + ] + + public_deps = [ + ":v8_base", + ":v8_maybe_snapshot", + ] + + if (v8_use_snapshot) { + public_deps += [ ":v8_builtins_generators" ] + } + + public_configs = [ ":external_config" ] + } } v8_executable("d8") { diff --git a/PRESUBMIT.py b/PRESUBMIT.py index 6aa94a01b3..7d7faec696 100644 --- a/PRESUBMIT.py +++ b/PRESUBMIT.py @@ -43,6 +43,12 @@ _EXCLUDED_PATHS = ( ) +# Regular expression that matches code which should not be run through cpplint. +_NO_LINT_PATHS = ( + r'src[\\\/]base[\\\/]export-template\.h', +) + + # Regular expression that matches code only used for test binaries # (best effort). _TEST_CODE_EXCLUDED_PATHS = ( @@ -70,9 +76,15 @@ def _V8PresubmitChecks(input_api, output_api): from presubmit import SourceProcessor from presubmit import StatusFilesProcessor + def FilterFile(affected_file): + return input_api.FilterSourceFile( + affected_file, + white_list=None, + black_list=_NO_LINT_PATHS) + results = [] if not CppLintProcessor().RunOnFiles( - input_api.AffectedFiles(include_deletes=False)): + input_api.AffectedFiles(file_filter=FilterFile, include_deletes=False)): results.append(output_api.PresubmitError("C++ lint check failed")) if not SourceProcessor().RunOnFiles( input_api.AffectedFiles(include_deletes=False)): diff --git a/src/base/export-template.h b/src/base/export-template.h new file mode 100644 index 0000000000..861cfe4027 --- /dev/null +++ b/src/base/export-template.h @@ -0,0 +1,163 @@ +// Copyright 2017 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. + +#ifndef V8_BASE_EXPORT_TEMPLATE_H_ +#define V8_BASE_EXPORT_TEMPLATE_H_ + +// Synopsis +// +// This header provides macros for using FOO_EXPORT macros with explicit +// template instantiation declarations and definitions. +// Generally, the FOO_EXPORT macros are used at declarations, +// and GCC requires them to be used at explicit instantiation declarations, +// but MSVC requires __declspec(dllexport) to be used at the explicit +// instantiation definitions instead. + +// Usage +// +// In a header file, write: +// +// extern template class EXPORT_TEMPLATE_DECLARE(FOO_EXPORT) foo; +// +// In a source file, write: +// +// template class EXPORT_TEMPLATE_DEFINE(FOO_EXPORT) foo; + +// Implementation notes +// +// The implementation of this header uses some subtle macro semantics to +// detect what the provided FOO_EXPORT value was defined as and then +// to dispatch to appropriate macro definitions. Unfortunately, +// MSVC's C preprocessor is rather non-compliant and requires special +// care to make it work. +// +// Issue 1. +// +// #define F(x) +// F() +// +// MSVC emits warning C4003 ("not enough actual parameters for macro +// 'F'), even though it's a valid macro invocation. This affects the +// macros below that take just an "export" parameter, because export +// may be empty. +// +// As a workaround, we can add a dummy parameter and arguments: +// +// #define F(x,_) +// F(,) +// +// Issue 2. +// +// #define F(x) G##x +// #define Gj() ok +// F(j()) +// +// The correct replacement for "F(j())" is "ok", but MSVC replaces it +// with "Gj()". As a workaround, we can pass the result to an +// identity macro to force MSVC to look for replacements again. (This +// is why EXPORT_TEMPLATE_STYLE_3 exists.) + +#define EXPORT_TEMPLATE_DECLARE(export) \ + EXPORT_TEMPLATE_INVOKE(DECLARE, EXPORT_TEMPLATE_STYLE(export, ), export) +#define EXPORT_TEMPLATE_DEFINE(export) \ + EXPORT_TEMPLATE_INVOKE(DEFINE, EXPORT_TEMPLATE_STYLE(export, ), export) + +// INVOKE is an internal helper macro to perform parameter replacements +// and token pasting to chain invoke another macro. E.g., +// EXPORT_TEMPLATE_INVOKE(DECLARE, DEFAULT, FOO_EXPORT) +// will export to call +// EXPORT_TEMPLATE_DECLARE_DEFAULT(FOO_EXPORT, ) +// (but with FOO_EXPORT expanded too). +#define EXPORT_TEMPLATE_INVOKE(which, style, export) \ + EXPORT_TEMPLATE_INVOKE_2(which, style, export) +#define EXPORT_TEMPLATE_INVOKE_2(which, style, export) \ + EXPORT_TEMPLATE_##which##_##style(export, ) + +// Default style is to apply the FOO_EXPORT macro at declaration sites. +#define EXPORT_TEMPLATE_DECLARE_DEFAULT(export, _) export +#define EXPORT_TEMPLATE_DEFINE_DEFAULT(export, _) + +// The "MSVC hack" style is used when FOO_EXPORT is defined +// as __declspec(dllexport), which MSVC requires to be used at +// definition sites instead. +#define EXPORT_TEMPLATE_DECLARE_MSVC_HACK(export, _) +#define EXPORT_TEMPLATE_DEFINE_MSVC_HACK(export, _) export + +// EXPORT_TEMPLATE_STYLE is an internal helper macro that identifies which +// export style needs to be used for the provided FOO_EXPORT macro definition. +// "", "__attribute__(...)", and "__declspec(dllimport)" are mapped +// to "DEFAULT"; while "__declspec(dllexport)" is mapped to "MSVC_HACK". +// +// It's implemented with token pasting to transform the __attribute__ and +// __declspec annotations into macro invocations. E.g., if FOO_EXPORT is +// defined as "__declspec(dllimport)", it undergoes the following sequence of +// macro substitutions: +// EXPORT_TEMPLATE_STYLE(FOO_EXPORT, ) +// EXPORT_TEMPLATE_STYLE_2(__declspec(dllimport), ) +// EXPORT_TEMPLATE_STYLE_3(EXPORT_TEMPLATE_STYLE_MATCH__declspec(dllimport)) +// EXPORT_TEMPLATE_STYLE_MATCH__declspec(dllimport) +// EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_dllimport +// DEFAULT +#define EXPORT_TEMPLATE_STYLE(export, _) EXPORT_TEMPLATE_STYLE_2(export, ) +#define EXPORT_TEMPLATE_STYLE_2(export, _) \ + EXPORT_TEMPLATE_STYLE_3( \ + EXPORT_TEMPLATE_STYLE_MATCH_foj3FJo5StF0OvIzl7oMxA##export) +#define EXPORT_TEMPLATE_STYLE_3(style) style + +// Internal helper macros for EXPORT_TEMPLATE_STYLE. +// +// XXX: C++ reserves all identifiers containing "__" for the implementation, +// but "__attribute__" and "__declspec" already contain "__" and the token-paste +// operator can only add characters; not remove them. To minimize the risk of +// conflict with implementations, we include "foj3FJo5StF0OvIzl7oMxA" (a random +// 128-bit string, encoded in Base64) in the macro name. +#define EXPORT_TEMPLATE_STYLE_MATCH_foj3FJo5StF0OvIzl7oMxA DEFAULT +#define EXPORT_TEMPLATE_STYLE_MATCH_foj3FJo5StF0OvIzl7oMxA__attribute__(...) \ + DEFAULT +#define EXPORT_TEMPLATE_STYLE_MATCH_foj3FJo5StF0OvIzl7oMxA__declspec(arg) \ + EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_##arg + +// Internal helper macros for EXPORT_TEMPLATE_STYLE. +#define EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_dllexport MSVC_HACK +#define EXPORT_TEMPLATE_STYLE_MATCH_DECLSPEC_dllimport DEFAULT + +// Sanity checks. +// +// EXPORT_TEMPLATE_TEST uses the same macro invocation pattern as +// EXPORT_TEMPLATE_DECLARE and EXPORT_TEMPLATE_DEFINE do to check that they're +// working correctly. When they're working correctly, the sequence of macro +// replacements should go something like: +// +// EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport)); +// +// static_assert(EXPORT_TEMPLATE_INVOKE(TEST_DEFAULT, +// EXPORT_TEMPLATE_STYLE(__declspec(dllimport), ), +// __declspec(dllimport)), "__declspec(dllimport)"); +// +// static_assert(EXPORT_TEMPLATE_INVOKE(TEST_DEFAULT, +// DEFAULT, __declspec(dllimport)), "__declspec(dllimport)"); +// +// static_assert(EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT( +// __declspec(dllimport)), "__declspec(dllimport)"); +// +// static_assert(true, "__declspec(dllimport)"); +// +// When they're not working correctly, a syntax error should occur instead. +#define EXPORT_TEMPLATE_TEST(want, export) \ + static_assert(EXPORT_TEMPLATE_INVOKE( \ + TEST_##want, EXPORT_TEMPLATE_STYLE(export, ), export), \ + #export) +#define EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT(...) true +#define EXPORT_TEMPLATE_TEST_MSVC_HACK_MSVC_HACK(...) true + +EXPORT_TEMPLATE_TEST(DEFAULT, ); +EXPORT_TEMPLATE_TEST(DEFAULT, __attribute__((visibility("default")))); +EXPORT_TEMPLATE_TEST(MSVC_HACK, __declspec(dllexport)); +EXPORT_TEMPLATE_TEST(DEFAULT, __declspec(dllimport)); + +#undef EXPORT_TEMPLATE_TEST +#undef EXPORT_TEMPLATE_TEST_DEFAULT_DEFAULT +#undef EXPORT_TEMPLATE_TEST_MSVC_HACK_MSVC_HACK + +#endif // V8_BASE_EXPORT_TEMPLATE_H_ diff --git a/src/compiler/simplified-operator.h b/src/compiler/simplified-operator.h index 3750861bf0..ac53bfc72e 100644 --- a/src/compiler/simplified-operator.h +++ b/src/compiler/simplified-operator.h @@ -141,7 +141,8 @@ enum class CheckForMinusZeroMode : uint8_t { size_t hash_value(CheckForMinusZeroMode); -std::ostream& operator<<(std::ostream&, CheckForMinusZeroMode); +V8_EXPORT_PRIVATE std::ostream& operator<<(std::ostream&, + CheckForMinusZeroMode); CheckForMinusZeroMode CheckMinusZeroModeOf(const Operator*) WARN_UNUSED_RESULT; diff --git a/src/objects.cc b/src/objects.cc index d6db304b13..b0ad2219b6 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -16818,9 +16818,11 @@ template class Dictionary >; template class Dictionary >; -template class Dictionary; +template class EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) + HashTable; + +template class EXPORT_TEMPLATE_DEFINE(V8_EXPORT_PRIVATE) + Dictionary; template class Dictionary HashTable >:: Shrink(Handle, Handle); -template Handle -HashTable:: - Shrink(Handle, uint32_t); - template Handle HashTable::Shrink(Handle, uint32_t); @@ -16941,9 +16939,6 @@ template Handle Dictionary >:: EnsureCapacity(Handle, int, Handle); -template int HashTable::FindEntry(uint32_t); - template int NameDictionaryBase::FindEntry( Handle); @@ -16991,6 +16986,16 @@ Dictionary>::CollectKeysTo( dictionary, KeyAccumulator* keys); +template int +Dictionary::AddEntry(Handle dictionary, + uint32_t key, Handle value, + PropertyDetails details, uint32_t hash); + +template int +Dictionary::NumberOfElementsFilterAttributes(PropertyFilter filter); + Handle JSObject::PrepareSlowElementsForSort( Handle object, uint32_t limit) { DCHECK(object->HasDictionaryElements()); diff --git a/src/objects.h b/src/objects.h index 21fcc16720..04ae7680cd 100644 --- a/src/objects.h +++ b/src/objects.h @@ -1189,7 +1189,7 @@ class Object { inline double Number() const; INLINE(bool IsNaN() const); INLINE(bool IsMinusZero() const); - bool ToInt32(int32_t* value); + V8_EXPORT_PRIVATE bool ToInt32(int32_t* value); inline bool ToUint32(uint32_t* value); inline Representation OptimalRepresentation(); diff --git a/src/objects/dictionary.h b/src/objects/dictionary.h index 05fa088ffb..8a480cc473 100644 --- a/src/objects/dictionary.h +++ b/src/objects/dictionary.h @@ -7,6 +7,9 @@ #include "src/objects/hash-table.h" +#include "src/base/export-template.h" +#include "src/globals.h" + // Has to be the last include (doesn't have include guards): #include "src/objects/object-macros.h" @@ -274,6 +277,12 @@ class UnseededNumberDictionaryShape : public NumberDictionaryShape { static inline Map* GetMap(Isolate* isolate); }; +extern template class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) + HashTable; + +extern template class EXPORT_TEMPLATE_DECLARE(V8_EXPORT_PRIVATE) + Dictionary; + class SeededNumberDictionary : public Dictionary { diff --git a/src/objects/hash-table.h b/src/objects/hash-table.h index 1d7f61d800..1dffec07e1 100644 --- a/src/objects/hash-table.h +++ b/src/objects/hash-table.h @@ -7,6 +7,9 @@ #include "src/objects.h" +#include "src/base/compiler-specific.h" +#include "src/globals.h" + // Has to be the last include (doesn't have include guards): #include "src/objects/object-macros.h" @@ -63,7 +66,7 @@ class BaseShape { static inline Map* GetMap(Isolate* isolate); }; -class HashTableBase : public FixedArray { +class V8_EXPORT_PRIVATE HashTableBase : public NON_EXPORTED_BASE(FixedArray) { public: // Returns the number of elements in the hash table. inline int NumberOfElements(); diff --git a/src/utils.h b/src/utils.h index f00674df55..ab25771be3 100644 --- a/src/utils.h +++ b/src/utils.h @@ -979,7 +979,7 @@ void PRINTF_FORMAT(2, 3) PrintIsolate(void* isolate, const char* format, ...); // Safe formatting print. Ensures that str is always null-terminated. // Returns the number of chars written, or -1 if output was truncated. int PRINTF_FORMAT(2, 3) SNPrintF(Vector str, const char* format, ...); -int PRINTF_FORMAT(2, 0) +V8_EXPORT_PRIVATE int PRINTF_FORMAT(2, 0) VSNPrintF(Vector str, const char* format, va_list args); void StrNCpy(Vector dest, const char* src, size_t n); diff --git a/src/v8.gyp b/src/v8.gyp index 5843d223d1..0a425c7e59 100644 --- a/src/v8.gyp +++ b/src/v8.gyp @@ -1881,6 +1881,7 @@ 'base/division-by-constant.h', 'base/debug/stack_trace.cc', 'base/debug/stack_trace.h', + 'base/export-template.h', 'base/file-utils.cc', 'base/file-utils.h', 'base/flags.h', diff --git a/src/wasm/local-decl-encoder.h b/src/wasm/local-decl-encoder.h index 4123e0268c..e0725efe9b 100644 --- a/src/wasm/local-decl-encoder.h +++ b/src/wasm/local-decl-encoder.h @@ -5,6 +5,7 @@ #ifndef V8_WASM_LOCAL_DECL_ENCODER_H_ #define V8_WASM_LOCAL_DECL_ENCODER_H_ +#include "src/globals.h" #include "src/wasm/wasm-opcodes.h" #include "src/zone/zone-containers.h" #include "src/zone/zone.h" @@ -15,7 +16,7 @@ namespace wasm { // A helper for encoding local declarations prepended to the body of a // function. -class LocalDeclEncoder { +class V8_EXPORT_PRIVATE LocalDeclEncoder { public: explicit LocalDeclEncoder(Zone* zone, FunctionSig* s = nullptr) : sig(s), local_decls(zone), total(0) {} diff --git a/src/wasm/wasm-result.h b/src/wasm/wasm-result.h index 43c9a8371b..ca9f646882 100644 --- a/src/wasm/wasm-result.h +++ b/src/wasm/wasm-result.h @@ -22,7 +22,7 @@ class Isolate; namespace wasm { // Base class for Result. -class ResultBase { +class V8_EXPORT_PRIVATE ResultBase { protected: ResultBase(ResultBase&& other) : error_offset_(other.error_offset_), diff --git a/test/unittests/BUILD.gn b/test/unittests/BUILD.gn index 42a7bebff6..18517bf44f 100644 --- a/test/unittests/BUILD.gn +++ b/test/unittests/BUILD.gn @@ -187,7 +187,7 @@ v8_executable("unittests") { #}], deps = [ - "../..:v8_builtins_generators", + "../..:v8_for_testing", "../..:v8_libbase", "../..:v8_libplatform", "//build/config/sanitizers:deps", @@ -200,17 +200,6 @@ v8_executable("unittests") { deps += [ "//third_party/icu" ] } - defines = [] - - if (is_component_build) { - # unittests can't be built against a shared library, so we - # need to depend on the underlying static target in that case. - deps += [ "../..:v8_maybe_snapshot" ] - defines += [ "BUILDING_V8_SHARED" ] - } else { - deps += [ "../..:v8" ] - } - if (is_win) { # This warning is benignly triggered by the U16 and U32 macros in # bytecode-utils.h.