This is an archive of the discontinued LLVM Phabricator instance.

Use GNU-style attributes for several __throw_XXX() functions
AbandonedPublic

Authored by dim on Jan 21 2017, 8:02 AM.

Details

Reviewers
mclow.lists
Summary

In rL279744, __throw_XXX() functions were introduced, partially for compatibility with software compiled against libstdc++. _LIBCPP_NORETURN is used on all of them, and when C++11 attributes are available, this gets defined as [[noreturn]], instead of the GNU-style __attribute__((noreturn)) style.

However, this can cause trouble with some software, for example Mozilla Firefox, which attempts to redefine several of these __throw_XXX() functions, and but uses the __attribute__((noreturn)) style exclusively. This then leads to errors like:

/usr/include/c++/v1/new:130:1: error: function declared '[[noreturn]]' after its first declaration
_LIBCPP_NORETURN _LIBCPP_FUNC_VIS void __throw_bad_alloc();  // not in C++ spec
^
/usr/include/c++/v1/__config:283:30: note: expanded from macro '_LIBCPP_NORETURN'
#  define _LIBCPP_NORETURN [[noreturn]]
                             ^
../../dist/include/mozilla/throw_gcc.h:35:1: note: declaration missing '[[noreturn]]' attribute is here
__throw_bad_alloc(void)
^

See also https://bugs.freebsd.org/216186 and https://bugzilla.mozilla.org/1329520 . In Firefox's case, the following list of functions is affected:

__throw_bad_alloc(void)
__throw_bad_cast(void)
__throw_bad_exception(void)
__throw_bad_function_call(void)
__throw_bad_typeid(void)
__throw_domain_error(const char*)
__throw_invalid_argument(const char*)
__throw_ios_failure(const char*)
__throw_length_error(const char*)
__throw_logic_error(const char*)
__throw_out_of_range(const char*)
__throw_overflow_error(const char*)
__throw_range_error(const char*)
__throw_runtime_error(const char*)
__throw_system_error(int)
__throw_underflow_error(const char*)

To make it easier on third-party software, let's define a LIBCPP_NORETURN_GNU variant, which always uses the __attribute__((noreturn)) style, and apply it specifically to the above list of functions.

This should also be merged to the 4.0 branch, when it has been committed.

Event Timeline

dim created this revision.Jan 21 2017, 8:02 AM
mclow.lists edited edge metadata.Jan 23 2017, 2:15 PM

In rL279744, __throw_XXX() functions were introduced, partially for compatibility with software compiled against libstdc++.

You're working from a false premise. These functions were not added for compatibility with libstdc++, but rather to encapsulate the mechanism for throwing an exception - to localize the changes needed to support -fno-exceptions

dim added a comment.Jan 23 2017, 2:32 PM

In rL279744, __throw_XXX() functions were introduced, partially for compatibility with software compiled against libstdc++.

You're working from a false premise. These functions were not added for compatibility with libstdc++, but rather to encapsulate the mechanism for throwing an exception - to localize the changes needed to support -fno-exceptions

Ok, I was mistaken about the premise, but by accident (or by their self-evidentness) the names are still shared with the GNU names. It would be nice to have some way of making them semi-compatible. Alternatively, we could rename them, but that would be more churn, I guess.

It would be nice to have some way of making them semi-compatible.

I'm afraid I disagree. If we make them something that people can "count on", then we have to support them.
The whole point of names that start with __ (or _[A-Z] are that they are internal, implementation details and are NOT part of the published API.

The __throw_xxx functions are not part of the public libstdc++ API and whatever Firefox is trying to do with them is not supported by libstdc++ and is undefined behaviour. Make it stop.

EricWF resigned from this revision.Jan 23 2017, 9:09 PM

I think we've agreed that this change shouldn't proceed. Resigning as reviewer.

dim abandoned this revision.Jan 24 2017, 6:38 AM

Ok, back to Mozilla we go.