union: __assert2() when a tag mismatches
Rather than abort()ing without any message, __assert2() will leave a bit
of hints.
- bad access: get<tag>() with a wrong tag.
- can't reach here: writeToParcel() with an unknown tag. (in practice,
this won't happen though)
Bug: 170681273
Test: aidl_integration_test
Change-Id: I85a872d602720542657ca1cbb1c931a3e6d15749
diff --git a/aidl_to_cpp_common.cpp b/aidl_to_cpp_common.cpp
index 6ac1d30..31b44ba 100644
--- a/aidl_to_cpp_common.cpp
+++ b/aidl_to_cpp_common.cpp
@@ -422,6 +422,7 @@
}
const vector<string> UnionWriter::headers{
+ "cassert", // __assert for logging
"type_traits", // std::is_same_v
"utility", // std::mode/forward for value
"variant", // std::variant for value
@@ -491,13 +492,13 @@
template <Tag _tag>
const auto& get() const {{
- if (getTag() != _tag) {{ abort(); }}
+ if (getTag() != _tag) {{ __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }}
return std::get<_tag>(_value);
}}
template <Tag _tag>
auto& get() {{
- if (getTag() != _tag) {{ abort(); }}
+ if (getTag() != _tag) {{ __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }}
return std::get<_tag>(_value);
}}
@@ -575,7 +576,7 @@
out << ";\n";
}
out << "}\n";
- out << "abort();\n";
+ out << "__assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, \"can't reach here\");\n";
}
} // namespace cpp
diff --git a/aidl_unittest.cpp b/aidl_unittest.cpp
index a4209c0..d76d449 100644
--- a/aidl_unittest.cpp
+++ b/aidl_unittest.cpp
@@ -2990,6 +2990,7 @@
#include <binder/Parcel.h>
#include <binder/ParcelFileDescriptor.h>
#include <binder/Status.h>
+#include <cassert>
#include <codecvt>
#include <cstdint>
#include <locale>
@@ -2999,6 +3000,10 @@
#include <variant>
#include <vector>
+#ifndef __BIONIC__
+#define __assert2(a,b,c,d) ((void)0)
+#endif
+
namespace a {
class Foo : public ::android::Parcelable {
@@ -3062,13 +3067,13 @@
template <Tag _tag>
const auto& get() const {
- if (getTag() != _tag) { abort(); }
+ if (getTag() != _tag) { __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }
return std::get<_tag>(_value);
}
template <Tag _tag>
auto& get() {
- if (getTag() != _tag) { abort(); }
+ if (getTag() != _tag) { __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }
return std::get<_tag>(_value);
}
@@ -3161,7 +3166,7 @@
case e: return _aidl_parcel->writeByte(static_cast<int8_t>(get<e>()));
case pfd: return _aidl_parcel->writeParcelable(get<pfd>());
}
- abort();
+ __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "can't reach here");
}
} // namespace a
@@ -3174,6 +3179,7 @@
#include <locale>
#include <sstream>
+#include <cassert>
#include <type_traits>
#include <utility>
#include <variant>
@@ -3186,6 +3192,11 @@
#include <android/binder_stability.h>
#endif // BINDER_STABILITY_SUPPORT
#include <aidl/a/ByteEnum.h>
+
+#ifndef __BIONIC__
+#define __assert2(a,b,c,d) ((void)0)
+#endif
+
namespace aidl {
namespace a {
class Foo {
@@ -3233,13 +3244,13 @@
template <Tag _tag>
const auto& get() const {
- if (getTag() != _tag) { abort(); }
+ if (getTag() != _tag) { __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }
return std::get<_tag>(_value);
}
template <Tag _tag>
auto& get() {
- if (getTag() != _tag) { abort(); }
+ if (getTag() != _tag) { __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "bad access: a wrong tag"); }
return std::get<_tag>(_value);
}
@@ -3332,7 +3343,7 @@
case e: return AParcel_writeByte(_parcel, static_cast<int8_t>(get<e>()));
case pfd: return ::ndk::AParcel_writeRequiredParcelFileDescriptor(_parcel, get<pfd>());
}
- abort();
+ __assert2(__FILE__, __LINE__, __PRETTY_FUNCTION__, "can't reach here");
}
} // namespace a
diff --git a/generate_cpp.cpp b/generate_cpp.cpp
index 94863b6..8e2fd53 100644
--- a/generate_cpp.cpp
+++ b/generate_cpp.cpp
@@ -1254,9 +1254,15 @@
const string code = CodeWriter::RunWith(&GenerateToString, parcel);
parcel_class->AddPublic(std::make_unique<LiteralDecl>(code));
+ auto decls = NestInNamespaces(std::move(parcel_class), parcel.GetSplitPackage());
+ // TODO(b/31559095) bionic on host should define this
+ if constexpr (std::is_same_v<T, AidlUnionDecl>) {
+ decls.insert(decls.begin(),
+ std::make_unique<LiteralDecl>(
+ "#ifndef __BIONIC__\n#define __assert2(a,b,c,d) ((void)0)\n#endif\n\n"));
+ }
return unique_ptr<Document>{
- new CppHeader{vector<string>(includes.begin(), includes.end()),
- NestInNamespaces(std::move(parcel_class), parcel.GetSplitPackage())}};
+ new CppHeader{vector<string>(includes.begin(), includes.end()), std::move(decls)}};
}
template <typename T, typename Traits = ParcelableTraits<T>>
diff --git a/generate_ndk.cpp b/generate_ndk.cpp
index e841457..90d53b5 100644
--- a/generate_ndk.cpp
+++ b/generate_ndk.cpp
@@ -1120,6 +1120,13 @@
}
GenerateHeaderIncludes(out, types, defined_type);
+ // TODO(b/31559095) bionic on host should define this
+ out << "\n";
+ out << "#ifndef __BIONIC__\n";
+ out << "#define __assert2(a,b,c,d) ((void)0)\n";
+ out << "#endif\n";
+ out << "\n";
+
EnterNdkNamespace(out, defined_type);
out << cpp::TemplateDecl(defined_type);
out << "class " << clazz << " {\n";
diff --git a/tests/aidl_test_client_parcelables.cpp b/tests/aidl_test_client_parcelables.cpp
index 1b3a51e..04ea939 100644
--- a/tests/aidl_test_client_parcelables.cpp
+++ b/tests/aidl_test_client_parcelables.cpp
@@ -183,7 +183,7 @@
EXPECT_EQ(one_two, std::vector<int>({1, 2}));
// abort with a bad access
- EXPECT_DEATH(one_two.get<Union::n>(), "");
+ EXPECT_DEATH(one_two.get<Union::n>(), "bad access");
// set<tag>(...) overwrites the content with a new tag
one_two_three.set<Union::s>("123");