[tint][ice] Make the InternalCompilerErrors abort
ICEs are no longer tied to the diagnostic system, and it was assumed that once you ICE the application will terminate. This assumption was not guaranteed and the callers of an ICE would attempt to continue execution despite the compiler being in a broken state.
Attempting to continue execution after an ICE is UB and should be treated as a security issue.
Instead, print the error message and abort().
This will crash the GPU process in Chrome, but that is likely to happen anyway.
Add [[noreturn]] to the InternalCompilerError destructor, and remove all unreachable statements following a TINT_ICE(), TINT_UNREACHABLE(), TINT_UNIMPLEMENTED().
This reduces the stripped tint executable on Linux by ~130KB (1.5%)
Remove TINT_ASSERT_OR_RETURN() and TINT_ASSERT_OR_RETURN_VALUE().
Replace both with TINT_ASSERT() as the return is now unreachable.
Replace the use of EXPECT_FATAL_FAILURE() with EXPECT_DEATH().
Change-Id: Iacbd16c12484610d72aee9850e93742c6bae1bf3
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/186623
Reviewed-by: James Price <jrprice@google.com>
diff --git a/src/tint/utils/rtti/BUILD.bazel b/src/tint/utils/rtti/BUILD.bazel
index e40a7b2..56b972c 100644
--- a/src/tint/utils/rtti/BUILD.bazel
+++ b/src/tint/utils/rtti/BUILD.bazel
@@ -40,6 +40,7 @@
name = "rtti",
srcs = [
"castable.cc",
+ "switch.cc",
],
hdrs = [
"castable.h",
@@ -64,7 +65,6 @@
"switch_test.cc",
],
deps = [
- "//src/tint/utils/ice",
"//src/tint/utils/macros",
"//src/tint/utils/math",
"//src/tint/utils/memory",
@@ -82,7 +82,6 @@
"switch_bench.cc",
],
deps = [
- "//src/tint/utils/ice",
"//src/tint/utils/macros",
"//src/tint/utils/math",
"//src/tint/utils/memory",
diff --git a/src/tint/utils/rtti/BUILD.cmake b/src/tint/utils/rtti/BUILD.cmake
index 6c58e07..72e5490 100644
--- a/src/tint/utils/rtti/BUILD.cmake
+++ b/src/tint/utils/rtti/BUILD.cmake
@@ -42,6 +42,7 @@
utils/rtti/castable.cc
utils/rtti/castable.h
utils/rtti/ignore.h
+ utils/rtti/switch.cc
utils/rtti/switch.h
)
@@ -63,7 +64,6 @@
)
tint_target_add_dependencies(tint_utils_rtti_test test
- tint_utils_ice
tint_utils_macros
tint_utils_math
tint_utils_memory
@@ -84,7 +84,6 @@
)
tint_target_add_dependencies(tint_utils_rtti_bench bench
- tint_utils_ice
tint_utils_macros
tint_utils_math
tint_utils_memory
diff --git a/src/tint/utils/rtti/BUILD.gn b/src/tint/utils/rtti/BUILD.gn
index 8df0474..f4bdbb1 100644
--- a/src/tint/utils/rtti/BUILD.gn
+++ b/src/tint/utils/rtti/BUILD.gn
@@ -47,6 +47,7 @@
"castable.cc",
"castable.h",
"ignore.h",
+ "switch.cc",
"switch.h",
]
deps = [
@@ -65,7 +66,6 @@
]
deps = [
"${tint_src_dir}:gmock_and_gtest",
- "${tint_src_dir}/utils/ice",
"${tint_src_dir}/utils/macros",
"${tint_src_dir}/utils/math",
"${tint_src_dir}/utils/memory",
@@ -79,7 +79,6 @@
sources = [ "switch_bench.cc" ]
deps = [
"${tint_src_dir}:google_benchmark",
- "${tint_src_dir}/utils/ice",
"${tint_src_dir}/utils/macros",
"${tint_src_dir}/utils/math",
"${tint_src_dir}/utils/memory",
diff --git a/src/tint/utils/rtti/switch.cc b/src/tint/utils/rtti/switch.cc
new file mode 100644
index 0000000..249c599
--- /dev/null
+++ b/src/tint/utils/rtti/switch.cc
@@ -0,0 +1,41 @@
+// Copyright 2024 The Dawn & Tint Authors
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are met:
+//
+// 1. Redistributions of source code must retain the above copyright notice, this
+// list of conditions and the following disclaimer.
+//
+// 2. Redistributions in binary form must reproduce the above copyright notice,
+// this list of conditions and the following disclaimer in the documentation
+// and/or other materials provided with the distribution.
+//
+// 3. Neither the name of the copyright holder nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+// AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+// IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+// FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+// DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+// CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+// OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+#include "src/tint/utils/rtti/switch.h"
+#include "src/tint/utils/ice/ice.h"
+
+namespace tint::detail {
+
+void ICENoSwitchPassedNullptr(const char* file, unsigned int line) {
+ InternalCompilerError(file, line) << "Switch() passed nullptr";
+}
+
+void ICENoSwitchCasesMatched(const char* file, unsigned int line, const char* type_name) {
+ InternalCompilerError(file, line) << "Switch() matched no cases. Type: " << type_name;
+}
+
+} // namespace tint::detail
diff --git a/src/tint/utils/rtti/switch.h b/src/tint/utils/rtti/switch.h
index 2436f5f..eb10dee 100644
--- a/src/tint/utils/rtti/switch.h
+++ b/src/tint/utils/rtti/switch.h
@@ -31,7 +31,6 @@
#include <tuple>
#include <utility>
-#include "src/tint/utils/ice/ice.h"
#include "src/tint/utils/macros/defer.h"
#include "src/tint/utils/memory/aligned_storage.h"
#include "src/tint/utils/rtti/castable.h"
@@ -70,9 +69,9 @@
/// [&](TypeB*) { /* ... */ },
/// TINT_ICE_ON_NO_MATCH);
/// ```
-#define TINT_ICE_ON_NO_MATCH \
- tint::SwitchMustMatchCase { \
- __FILE__, __LINE__ \
+#define TINT_ICE_ON_NO_MATCH \
+ ::tint::SwitchMustMatchCase { \
+ __FILE__, __LINE__ \
}
} // namespace tint
@@ -120,7 +119,7 @@
}
/// Resolves to T if T is not nullptr_t, otherwise resolves to Ignore.
template <typename T>
-using NullptrToIgnore = std::conditional_t<std::is_same_v<T, std::nullptr_t>, tint::Ignore, T>;
+using NullptrToIgnore = std::conditional_t<std::is_same_v<T, std::nullptr_t>, ::tint::Ignore, T>;
/// Resolves to `const TYPE` if any of `CASE_RETURN_TYPES` are const or pointer-to-const, otherwise
/// resolves to TYPE.
@@ -145,7 +144,7 @@
/// SwitchReturnTypeImpl specialization for non-castable case types and an inferred return type.
template <typename... CASE_RETURN_TYPES>
-struct SwitchReturnTypeImpl</*IS_CASTABLE*/ false, tint::detail::Infer, CASE_RETURN_TYPES...> {
+struct SwitchReturnTypeImpl</*IS_CASTABLE*/ false, ::tint::detail::Infer, CASE_RETURN_TYPES...> {
/// Resolves to the common type for all the cases return types.
using type = std::common_type_t<CASE_RETURN_TYPES...>;
};
@@ -161,7 +160,7 @@
/// SwitchReturnTypeImpl specialization for castable case types and an inferred return type.
template <typename... CASE_RETURN_TYPES>
-struct SwitchReturnTypeImpl</*IS_CASTABLE*/ true, tint::detail::Infer, CASE_RETURN_TYPES...> {
+struct SwitchReturnTypeImpl</*IS_CASTABLE*/ true, ::tint::detail::Infer, CASE_RETURN_TYPES...> {
private:
using InferredType =
CastableCommonBase<NullptrToIgnore<std::remove_pointer_t<CASE_RETURN_TYPES>>...>;
@@ -176,7 +175,7 @@
/// from the case return types.
template <typename REQUESTED_TYPE, typename... CASE_RETURN_TYPES>
using SwitchReturnType = typename SwitchReturnTypeImpl<
- tint::IsCastable<NullptrToIgnore<std::remove_pointer_t<CASE_RETURN_TYPES>>...>,
+ ::tint::IsCastable<NullptrToIgnore<std::remove_pointer_t<CASE_RETURN_TYPES>>...>,
REQUESTED_TYPE,
CASE_RETURN_TYPES...>::type;
@@ -188,23 +187,34 @@
template <typename CASE>
struct SwitchCaseReturnTypeImpl<CASE, /* is_flag */ false> {
/// The case function's return type.
- using type = tint::traits::ReturnType<CASE>;
+ using type = ::tint::traits::ReturnType<CASE>;
};
/// SwitchCaseReturnTypeImpl specialization for flags.
template <typename CASE>
struct SwitchCaseReturnTypeImpl<CASE, /* is_flag */ true> {
/// These are not functions, they have no return type.
- using type = tint::Ignore;
+ using type = ::tint::Ignore;
};
/// Resolves to the return type for a Switch() case.
-/// If CASE is a flag like SwitchMustMatchCase, then resolves to tint::Ignore
+/// If CASE is a flag like SwitchMustMatchCase, then resolves to ::tint::Ignore
template <typename CASE>
using SwitchCaseReturnType = typename SwitchCaseReturnTypeImpl<
CASE,
std::is_same_v<std::decay_t<CASE>, SwitchMustMatchCase>>::type;
+/// Raises an ICE error that a Switch() was passed a nullptr object and there was no default case
+[[noreturn]] void ICENoSwitchPassedNullptr(const char* file, unsigned int line);
+
+/// Raises an ICE error that a Switch() with a TINT_ICE_ON_NO_MATCH matched no cases.
+/// @param file the file holding the Switch()
+/// @param line the line of the TINT_ICE_ON_NO_MATCH
+/// @type_name the type name of the object passed to Switch()
+[[noreturn]] void ICENoSwitchCasesMatched(const char* file,
+ unsigned int line,
+ const char* type_name);
+
} // namespace tint::detail
namespace tint {
@@ -248,18 +258,19 @@
/// @param args the switch cases followed by an optional TINT_ICE_ON_NO_MATCH
/// @return the value returned by the called case. If no cases matched, then the zero value for the
/// consistent case type.
-template <typename RETURN_TYPE = tint::detail::Infer, typename T = CastableBase, typename... ARGS>
+template <typename RETURN_TYPE = ::tint::detail::Infer, typename T = CastableBase, typename... ARGS>
inline auto Switch(T* object, ARGS&&... args) {
TINT_BEGIN_DISABLE_WARNING(UNUSED_VALUE);
using ArgsTuple = std::tuple<ARGS...>;
static constexpr int kMustMatchCaseIndex =
- tint::detail::IndexOfSwitchMustMatchCase<ArgsTuple>();
+ ::tint::detail::IndexOfSwitchMustMatchCase<ArgsTuple>();
static constexpr bool kHasMustMatchCase = kMustMatchCaseIndex >= 0;
- static constexpr int kDefaultIndex = tint::detail::IndexOfDefaultCase<ArgsTuple>();
+ static constexpr int kDefaultIndex = ::tint::detail::IndexOfDefaultCase<ArgsTuple>();
static constexpr bool kHasDefaultCase = kDefaultIndex >= 0;
using ReturnType =
- tint::detail::SwitchReturnType<RETURN_TYPE, tint::detail::SwitchCaseReturnType<ARGS>...>;
+ ::tint::detail::SwitchReturnType<RETURN_TYPE,
+ ::tint::detail::SwitchCaseReturnType<ARGS>...>;
static constexpr bool kHasReturnType = !std::is_same_v<ReturnType, void>;
// Static assertions
@@ -280,7 +291,7 @@
if (!object) { // Object is nullptr, so no cases can match
if constexpr (kHasMustMatchCase) {
const SwitchMustMatchCase& info = (args, ...);
- tint::InternalCompilerError(info.file, info.line) << "Switch() passed nullptr";
+ ::tint::detail::ICENoSwitchPassedNullptr(info.file, info.line);
if constexpr (kHasReturnType) {
return ReturnType{};
} else {
@@ -303,7 +314,7 @@
AlignedStorage<std::conditional_t<kHasReturnType, ReturnType, uint8_t>> return_storage;
auto* result = &return_storage.Get();
- const tint::TypeInfo& type_info = object->TypeInfo();
+ const ::tint::TypeInfo& type_info = object->TypeInfo();
// Examines the parameter type of the case function.
// If the parameter is a pointer type that `object` is of, or derives from, then that case
@@ -317,10 +328,9 @@
using CaseFunc = std::decay_t<decltype(case_fn)>;
bool success = false;
if constexpr (std::is_same_v<CaseFunc, SwitchMustMatchCase>) {
- tint::InternalCompilerError(case_fn.file, case_fn.line)
- << "Switch() matched no cases. Type: " << type_info.name;
+ ::tint::detail::ICENoSwitchCasesMatched(case_fn.file, case_fn.line, type_info.name);
} else {
- using CaseType = tint::detail::SwitchCaseType<CaseFunc>;
+ using CaseType = ::tint::detail::SwitchCaseType<CaseFunc>;
if constexpr (std::is_same_v<CaseType, Default>) {
if constexpr (kHasReturnType) {
new (result) ReturnType(static_cast<ReturnType>(case_fn(Default{})));
diff --git a/src/tint/utils/rtti/switch_test.cc b/src/tint/utils/rtti/switch_test.cc
index f915c9e..4ae0f95 100644
--- a/src/tint/utils/rtti/switch_test.cc
+++ b/src/tint/utils/rtti/switch_test.cc
@@ -30,7 +30,7 @@
#include <memory>
#include <string>
-#include "gtest/gtest-spi.h"
+#include "gmock/gmock.h"
#include "gtest/gtest.h"
namespace tint {
@@ -230,7 +230,7 @@
}
TEST(Castable, SwitchMustMatch_NoMatchWithoutReturnValue) {
- EXPECT_FATAL_FAILURE(
+ EXPECT_DEATH(
{
std::unique_ptr<Animal> frog = std::make_unique<Frog>();
Switch(
@@ -239,11 +239,11 @@
[&](Mammal*) {}, //
TINT_ICE_ON_NO_MATCH);
},
- "internal compiler error: Switch() matched no cases. Type: Frog");
+ testing::HasSubstr("internal compiler error: Switch() matched no cases. Type: Frog"));
}
TEST(Castable, SwitchMustMatch_NoMatchWithReturnValue) {
- EXPECT_FATAL_FAILURE(
+ EXPECT_DEATH(
{
std::unique_ptr<Animal> frog = std::make_unique<Frog>();
int res = Switch(
@@ -253,11 +253,11 @@
TINT_ICE_ON_NO_MATCH);
ASSERT_EQ(res, 0);
},
- "internal compiler error: Switch() matched no cases. Type: Frog");
+ testing::HasSubstr("internal compiler error: Switch() matched no cases. Type: Frog"));
}
TEST(Castable, SwitchMustMatch_NullptrWithoutReturnValue) {
- EXPECT_FATAL_FAILURE(
+ EXPECT_DEATH(
{
Switch(
static_cast<CastableBase*>(nullptr), //
@@ -265,11 +265,11 @@
[&](Mammal*) {}, //
TINT_ICE_ON_NO_MATCH);
},
- "internal compiler error: Switch() passed nullptr");
+ testing::HasSubstr("internal compiler error: Switch() passed nullptr"));
}
TEST(Castable, SwitchMustMatch_NullptrWithReturnValue) {
- EXPECT_FATAL_FAILURE(
+ EXPECT_DEATH(
{
int res = Switch(
static_cast<CastableBase*>(nullptr), //
@@ -278,7 +278,7 @@
TINT_ICE_ON_NO_MATCH);
ASSERT_EQ(res, 0);
},
- "internal compiler error: Switch() passed nullptr");
+ testing::HasSubstr("internal compiler error: Switch() passed nullptr"));
}
TEST(Castable, SwitchMatchFirst) {
