Don't build test_main.cc in chromium builds

Chromium has its own test main() entrypoint.

To ensure that Chromium doesn't panic about memory leaks with the tests that exercise the ICE cases, we have to explicitly call the FreeInternalCompilerErrors() functions in these tests (at least until I can add this to end of Chromium's test main() function)

Change-Id: I2ea5109fcdb5f68f56a19709a1ec35ed72c0f760
Reviewed-on: https://dawn-review.googlesource.com/c/tint/+/42025
Reviewed-by: Austin Eng <enga@chromium.org>
Reviewed-by: dan sinclair <dsinclair@chromium.org>
Commit-Queue: Ben Clayton <bclayton@google.com>
This commit is contained in:
Ben Clayton 2021-02-18 20:23:09 +00:00 committed by Commit Bot service account
parent 94cfbbeb5f
commit 89036852cc
5 changed files with 45 additions and 26 deletions

View File

@ -762,11 +762,13 @@ if (!build_with_chromium) {
# These targets are separated because they are Chromium sources files that # These targets are separated because they are Chromium sources files that
# can't use the tint_internal config, otherwise Tint's warning flags get # can't use the tint_internal config, otherwise Tint's warning flags get
# applied while compiling a bunch of Chromium's //base (via header inclusion) # applied while compiling a bunch of Chromium's //base (via header inclusion)
if (build_with_chromium) {
source_set("tint_unittests_main") { source_set("tint_unittests_main") {
testonly = true testonly = true
deps = [ ":gmock_and_gtest" ] deps = [ ":gmock_and_gtest" ]
if (build_with_chromium) {
sources = [ "//gpu/tint_unittests_main.cc" ] sources = [ "//gpu/tint_unittests_main.cc" ]
} else {
sources = [ "src/test_main.cc" ]
} }
} }
@ -846,7 +848,6 @@ source_set("tint_unittests_core_src") {
"src/scope_stack_test.cc", "src/scope_stack_test.cc",
"src/symbol_table_test.cc", "src/symbol_table_test.cc",
"src/symbol_test.cc", "src/symbol_test.cc",
"src/test_main.cc",
"src/traits_test.cc", "src/traits_test.cc",
"src/transform/bound_array_accessors_test.cc", "src/transform/bound_array_accessors_test.cc",
"src/transform/emit_vertex_point_size_test.cc", "src/transform/emit_vertex_point_size_test.cc",
@ -1339,9 +1340,7 @@ test("tint_unittests") {
"${tint_spirv_tools_dir}/:spvtools_val", "${tint_spirv_tools_dir}/:spvtools_val",
] ]
if (build_with_chromium) {
deps += [ ":tint_unittests_main" ] deps += [ ":tint_unittests_main" ]
}
configs += [ configs += [
":tint_common_config", ":tint_common_config",

View File

@ -280,6 +280,13 @@ TEST(CloneContext, CloneWithReplace_WithNotANode) {
ctx.Clone(original_root); ctx.Clone(original_root);
}, },
"internal compiler error"); "internal compiler error");
// Ensure that this test does not leak memory.
// This will be automatically called by main() in src/test_main.cc, but
// chromium uses it's own test entry point.
// TODO(ben-clayton): Add this call to the end of Chromium's main(), and we
// can remove this call.
FreeInternalCompilerErrors();
} }
} // namespace } // namespace

View File

@ -27,37 +27,43 @@ namespace {
InternalCompilerErrorReporter* ice_reporter = nullptr; InternalCompilerErrorReporter* ice_reporter = nullptr;
class SourceFileToDelete { /// Note - this class is _not_ thread safe. If we have multiple internal
public:
static SourceFileToDelete& Get() {
static SourceFileToDelete* instance = new SourceFileToDelete();
return *instance;
}
/// Adds file to the list that will be deleted on call to Free()
/// Note - this function is _not_ thread safe. If we have multiple internal
/// compiler errors occurring at the same time on different threads, then /// compiler errors occurring at the same time on different threads, then
/// we're in serious trouble. /// we're in serious trouble.
void Add(Source::File* file) { files.emplace_back(file); } class SourceFileToDelete {
static SourceFileToDelete* instance;
public:
/// Adds file to the list that will be deleted on call to Free()
static void Add(Source::File* file) {
if (!instance) {
instance = new SourceFileToDelete();
}
instance->files.emplace_back(file);
}
/// Free deletes all the source files added by calls to Add() and then this /// Free deletes all the source files added by calls to Add() and then this
/// SourceFileToDelete object. The SourceFileToDelete must not be used after /// SourceFileToDelete object.
/// calling. static void Free() {
void Free() { if (instance) {
for (auto* file : files) { for (auto* file : instance->files) {
delete file; delete file;
} }
delete this; delete instance;
instance = nullptr;
}
} }
private: private:
std::vector<Source::File*> files; std::vector<Source::File*> files;
}; };
SourceFileToDelete* SourceFileToDelete::instance = nullptr;
} // namespace } // namespace
void FreeInternalCompilerErrors() { void FreeInternalCompilerErrors() {
SourceFileToDelete::Get().Free(); SourceFileToDelete::Free();
} }
void SetInternalCompilerErrorReporter(InternalCompilerErrorReporter* reporter) { void SetInternalCompilerErrorReporter(InternalCompilerErrorReporter* reporter) {
@ -72,7 +78,7 @@ InternalCompilerError::InternalCompilerError(const char* file,
InternalCompilerError::~InternalCompilerError() { InternalCompilerError::~InternalCompilerError() {
auto* file = new Source::File(file_, ""); auto* file = new Source::File(file_, "");
SourceFileToDelete::Get().Add(file); SourceFileToDelete::Add(file);
Source source{Source::Range{Source::Location{line_}}, file}; Source source{Source::Range{Source::Location{line_}}, file};
diagnostics_.add_ice(msg_.str(), source); diagnostics_.add_ice(msg_.str(), source);

View File

@ -28,7 +28,7 @@ namespace tint {
using InternalCompilerErrorReporter = void(const diag::List&); using InternalCompilerErrorReporter = void(const diag::List&);
/// Frees any memory allocated for reporting internal compiler errors. /// Frees any memory allocated for reporting internal compiler errors.
/// Must only be called once on application termination. /// Must only be called on application termination.
/// If an internal compiler error is raised and this function is not called, /// If an internal compiler error is raised and this function is not called,
/// then memory will leak. /// then memory will leak.
void FreeInternalCompilerErrors(); void FreeInternalCompilerErrors();

View File

@ -26,6 +26,13 @@ TEST(DebugTest, Unreachable) {
TINT_UNREACHABLE(diagnostics); TINT_UNREACHABLE(diagnostics);
}, },
"internal compiler error"); "internal compiler error");
// Ensure that this test does not leak memory.
// This will be automatically called by main() in src/test_main.cc, but
// chromium uses it's own test entry point.
// TODO(ben-clayton): Add this call to the end of Chromium's main(), and we
// can remove this call.
FreeInternalCompilerErrors();
} }
} // namespace } // namespace