Skip to content

Commit 63334db

Browse files
jupp0rmeta-codesync[bot]
authored andcommitted
Support virtual inheritance paths to std::exception in exception tracer library
Summary: This diff implements stack trace collection for derived exceptions in virtual inheritance hierarchies. ## Description of current behavior Stack traces for exceptions `A` at the time they are thrown are stored by `SmartExceptionTracer` in a global `map<void*, ExceptionInfo>`. Previously, when performing lookups to obtain stack traces, the address of the object `B` to obtain a stack trace for needed to match the pointer previously stored. This works for simple cases where `A` and `B` are [pointer interconvertible](https://en.cppreference.com/w/cpp/language/static_cast.html#pointer-interconvertible), but failed lookups for cases where there is a non-zero offset between addresses of B and the thrown object A due to their differing layout. ## Fix The fix uses `dynamic_cast<void*>` to obtain the address of the [most derived object](https://eel.is/c++draft/intro.object#6) associated with the object passed into `getTrace` for the case where a reference to a derived type (like `std::exception`) is passed in. This ensures that the address used in the lookup in the global map of thrown exceptions corresponds to the address placed there by the hook (ie the address passed by the runtime to `__cxa_throw()`. ## Side effects of this change One side effect of how the fix is implemented is that we avoid a `dynamic_cast` to `std::exception` in the lookup paths where `std::exception_ptr` or `folly::ExceptionWrapper` are passed. This enables support for collecting stack traces for thrown objects that are not derived from `std::exception` (see the changed `NonStdException` test case). In addition to that, this diffs skips unnecessary RTTI inspection of thrown objects to find out whether they are compatible with `std::exception` as the pointer to the originally thrown exception is already available when `std::exception_ptr` or `folly::exception_wrapper` is available, which increases performance slightly (see benchmarks below). Reviewed By: shr, yfeldblum Differential Revision: D84131474 fbshipit-source-id: 4e5434ffa01ad5739fed4226ec5b8891ee3c2daa
1 parent 5435449 commit 63334db

2 files changed

Lines changed: 65 additions & 8 deletions

File tree

third-party/folly/src/folly/debugging/exception_tracer/SmartExceptionTracer.cpp

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ std::atomic_bool loggedMessage{false};
4141
// the stack frames to use.
4242
template <typename ExceptionMetaFunc>
4343
ExceptionInfo getTraceWithFunc(
44-
const std::exception& ex, ExceptionMetaFunc func) {
44+
void* ex, const std::type_info* typeInfo, ExceptionMetaFunc func) {
4545
if (!detail::isSmartExceptionTracerHookEnabled() &&
4646
!loggedMessage.load(std::memory_order_relaxed)) {
4747
LOG(WARNING)
@@ -50,9 +50,9 @@ ExceptionInfo getTraceWithFunc(
5050
}
5151

5252
ExceptionInfo info;
53-
info.type = &typeid(ex);
53+
info.type = typeInfo;
5454

55-
if (auto meta = get_default(*detail::getMetaMap().rlock(), &ex)) {
55+
if (auto meta = get_default(*detail::getMetaMap().rlock(), ex)) {
5656
auto [traceBeginIt, traceEndIt] = func(*meta);
5757
info.frames.assign(traceBeginIt, traceEndIt);
5858
}
@@ -63,12 +63,22 @@ ExceptionInfo getTraceWithFunc(
6363
template <typename ExceptionMetaFunc>
6464
ExceptionInfo getTraceWithFunc(
6565
const std::exception_ptr& ptr, ExceptionMetaFunc func) {
66-
if (auto* ex = folly::exception_ptr_get_object<std::exception>(ptr)) {
67-
return getTraceWithFunc(*ex, std::move(func));
66+
if (auto ex = folly::exception_ptr_get_object(ptr, nullptr)) {
67+
return getTraceWithFunc(
68+
ex, folly::exception_ptr_get_type(ptr), std::move(func));
6869
}
6970
return ExceptionInfo();
7071
}
7172

73+
template <typename ExceptionMetaFunc>
74+
ExceptionInfo getTraceWithFunc(
75+
const std::exception& e, ExceptionMetaFunc func) {
76+
return getTraceWithFunc(
77+
const_cast<void*>(dynamic_cast<const void*>(&e)),
78+
&typeid(e),
79+
std::move(func));
80+
}
81+
7282
template <typename ExceptionMetaFunc>
7383
ExceptionInfo getTraceWithFunc(
7484
const exception_wrapper& ew, ExceptionMetaFunc func) {

third-party/folly/src/folly/debugging/exception_tracer/test/SmartExceptionTracerTest.cpp

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <folly/coro/BlockingWait.h>
1818
#include <folly/coro/Task.h>
1919
#include <folly/debugging/exception_tracer/SmartExceptionTracer.h>
20+
#include <folly/portability/GMock.h>
2021
#include <folly/portability/GTest.h>
2122

2223
#if FOLLY_HAVE_ELF && FOLLY_HAVE_DWARF
@@ -67,16 +68,15 @@ TEST(SmartExceptionTracer, EmptyExceptionWrapper) {
6768
ss.str().find("Exception type: (unknown type)") != std::string::npos);
6869
}
6970

70-
TEST(SmartExceptionTracer, InvalidException) {
71+
TEST(SmartExceptionTracer, NonStdException) {
7172
try {
7273
throw 10;
7374
} catch (...) {
7475
auto info = getTrace(std::current_exception());
7576

7677
std::ostringstream ss;
7778
ss << info;
78-
ASSERT_TRUE(
79-
ss.str().find("Exception type: (unknown type)") != std::string::npos);
79+
ASSERT_THAT(ss.str(), testing::HasSubstr("Exception type: int"));
8080
}
8181
}
8282

@@ -143,4 +143,51 @@ TEST(SmartExceptionTracer, AsyncStackTrace) {
143143
}
144144
}
145145

146+
class ExceptionE : virtual public std::runtime_error {
147+
public:
148+
ExceptionE() : std::runtime_error("ExceptionA") {}
149+
};
150+
151+
class ExceptionF : virtual public std::runtime_error {
152+
public:
153+
ExceptionF() : std::runtime_error("ExceptionB") {}
154+
};
155+
156+
class ExceptionG : virtual public ExceptionE, virtual public ExceptionF {
157+
public:
158+
ExceptionG() : std::runtime_error("ExceptionC"), ExceptionE(), ExceptionF() {}
159+
};
160+
161+
[[noreturn]] FOLLY_NOINLINE void throwDiamond() {
162+
throw ExceptionG();
163+
}
164+
165+
TEST(SmartExceptionTracer, diamondExceptionPointer) {
166+
try {
167+
throwDiamond();
168+
} catch (...) {
169+
ASSERT_NE(
170+
folly::exception_ptr_get_object<std::exception>(
171+
std::current_exception()),
172+
nullptr);
173+
auto info = getTrace(std::current_exception());
174+
auto infoStr = std::stringstream{} << info;
175+
176+
EXPECT_THAT(info.frames, testing::SizeIs(testing::Gt(0)));
177+
EXPECT_THAT(infoStr.str(), testing::HasSubstr("throwDiamond"));
178+
}
179+
}
180+
181+
TEST(SmartExceptionTracer, diamondStdException) {
182+
try {
183+
throwDiamond();
184+
} catch (const std::exception& e) {
185+
auto info = getTrace(e);
186+
auto infoStr = std::stringstream{} << info;
187+
188+
EXPECT_THAT(info.frames, testing::SizeIs(testing::Gt(0)));
189+
EXPECT_THAT(infoStr.str(), testing::HasSubstr("throwDiamond"));
190+
}
191+
}
192+
146193
#endif // FOLLY_HAVE_ELF && FOLLY_HAVE_DWARF

0 commit comments

Comments
 (0)