[libcxx] Fix infinite loop regression when building libc++ with ubsan

[thakis]

Summary

https://reviews.llvm.org/rG360ead76480adf7bd7dcef22944e2acc2cc72720
made libcxxabi call typeinfo::operator==() in the implementation of
__dynamic_cast(). When ubsan instrumentation is active, it emits
a call to dynamic_cast<>() for the implementation of
typeinfo::operator== to verify the dynamic type of this. This leads
to infinite recursion.

As a workaround, disable ubsan instrumentation for
typeinfo::operator==().

See https://crbug.com/1181057 for more notes.

[ldionne]

The fix looks OK to me, but why doesn't this show up when we run the tests under ubsan?

[thakis]

In D97323#2582807, @ldionne wrote:

The fix looks OK to me, but why doesn't this show up when we run the tests under ubsan?

I guess the test runner builds against system libc++abi, or doesn't instrument libc++abi with ubsan (?).

[rsmith]

The ubsan runtime expects its dependencies to not have ubsan enabled. How are you configuring your build so that libc++abi ends up ubsan-instrumented? Can we ensure that doesn't happen somehow?

[thakis]

In D97323#2582883, @rsmith wrote:

The ubsan runtime expects its dependencies to not have ubsan enabled. How are you configuring your build so that libc++abi ends up ubsan-instrumented? Can we ensure that doesn't happen somehow?

Sure, that's possible too. We've been running with sanitizer-enabled libc++(abi) for years though, and this is the only issue we've run into.

[ldionne]

In D97323#2582883, @rsmith wrote:

The ubsan runtime expects its dependencies to not have ubsan enabled. How are you configuring your build so that libc++abi ends up ubsan-instrumented? Can we ensure that doesn't happen somehow?

In that case, I think we should fix the issue at its root and not make this workaround in libc++.

Is there a way we could detect this sort of issue in the sanitizers to error early instead of just misbehaving when someone violates that requirement?

[thakis]

In D97323#2583597, @ldionne wrote:

In D97323#2582883, @rsmith wrote:

The ubsan runtime expects its dependencies to not have ubsan enabled. How are you configuring your build so that libc++abi ends up ubsan-instrumented? Can we ensure that doesn't happen somehow?

In that case, I think we should fix the issue at its root and not make this workaround in libc++.

Is there a way we could detect this sort of issue in the sanitizers to error early instead of just misbehaving when someone violates that requirement?

If the review feedback on D52386 gets addressed, we could add an #if __has_feature(ubsan_vptr) #error to private_typeinfo.cpp.

libcxx/include/typeinfo

public:
 
     _LIBCPP_INLINE_VISIBILITY
     size_t hash_code() const _NOEXCEPT
     {
       return __impl::__hash(__type_name);
     }
 
     _LIBCPP_INLINE_VISIBILITY
+#if __has_attribute(no_sanitize)
+     // libcxxabi's __dynamic_cast() implementation calls this operator to
+     // implement dynamic_cast<>(), but ubsan's instrumentation inserts a
+     // call to dynamic_cast<>() into the implementation of this function to
+     // check the dynamic type of |this|, leading to infinite recursion.
+     __attribute__((__no_sanitize__("undefined")))
+#endif
     bool operator==(const type_info& __arg) const _NOEXCEPT
     {
       return __impl::__eq(__type_name, __arg.__type_name);
     }
 
     _LIBCPP_INLINE_VISIBILITY
     bool operator!=(const type_info& __arg) const _NOEXCEPT
     { return !operator==(__arg); }