[PATCH] D14653: [libcxx] Introduce the mechanism for fixing -fno-exceptions test failures.

[Asiri Rathnayake via cfe-commits]

rmaprath created this revision.
rmaprath added reviewers: mclow.lists, jroelofs, rengolin.
rmaprath added a subscriber: cfe-commits.

Summary:

r252598 and r252870 added XFAILs to all those tests that are currently
failing on the -fno-exceptions libc++ library variant. This patch introduces
a mechanism to fix those tests.

Details:

Most of the failing tests have checks of the following nature:

// [1] setup some global state
try {
  do_something_bad();
  assert(false);
} catch (some_exception e) {
  // [2] check some (mostly global) state
  // for consistency.
}
// [3] check some more state
// [4] possibly more try/catch clauses

These tests have been written with the -fexceptions library in mind and does
not compile with -fno-exceptions, and hence cannot be used as-is for testing
the -fno-exceptions library variant.

The current patch attempts to improve the testing of the -fno-exceptions library
variant by leveraging as much as possible of these existing tests, without having
to change the tests themselves massively.

We have the following goals in mind:

  1. do_something_bad() should call abort() in the -fno-exceptions library
     variant. The tests should be able to verify this.

       Note: The -fno-exceptions library variant is a custom extension, the
       behaviour of this library when responding to error states does not fall
       into the scope of the C++ standard (which always assumes exceptions).
       Here we have made the decision to call abort() because:

         I. It captures the intention of -fno-exceptions: when the library has
         entered an error state and has no means of notifying it to user code,
         it makes sense to abort the program rather than continue.

         II. This behaviour is in-line with the IA64 CXX ABI:
           https://mentorembedded.github.io/cxx-abi/abi-eh.html#base-framework
         Note the unwinding process, where it says:
           "If the search phase reports failure, e.g. because no handler was
            found, it will call terminate() rather than commence phase 2"

  2. The abort() call in 1) should not terminate the test itself, as that
     would prevent any follow-up tests (in the same source file) from running.

  3. We should leverage as much of the existing tests as possible, without having
     to change them drastically.

These goals lead us to the following approach:

We introduce a special support header file (noexcept.h) which when included in
the test file, re-writes code segments like the one above with something along
the lines of:

iflabel:
// [1] setup some global state
if (!abort_called()) {
  do_something_bad();
  assert(false);
} else {
  // [2] check some (mostly global) state
  // for consistency.
}
// [3] check some more state
// [4] possibly more try/catch clauses

The custom abort() method provided in the support header file makes a jump to
"iflabel" when called, causing the else branch to be taken after the abort()
call. This mechanism allows us to achieve all of the above goals without having
to modify the test cases greatly (in most cases, simply including the noexcept.h
header is all that is needed).

Are there alternatives approaches to improve the testing? without this kind of
re-writing of try/catch clauses?

  Two approaches come to mind:
    1. Conditionalize try/catch/throw statements on _LIBCPP_NO_EXCEPTIONS
       and do the necessary adjustments case by case. This requires more effort
       and modifies the existing tests a lot.
    2. Keep the current XFAILS, add separate tests for the -fno-exceptions
       library variant. Again, this is quite an involved task.

In sum, the approach above gives us the most benefit (able to catch quite a lot
of defects in the -fno-exceptions library) with significantly less effort. Of
course, additional tests can be added that specifically test the -fno-exceptions
library variant. Such tests may use _LIBCPP_NO_EXCEPTIONS explicitly.

Aside from the tests, we have also introduced one configuration header file to
the library itself (__noexcept). This header file introduces utility functions
that can be used to simplify the -fno-exceptions code path. The following
example shows how the library handles the -fno-exceptions as of today:

if (some_bad_state()) {
 #ifndef _LIBCPP_NO_EXCEPTIONS
   throw some_exception("some message");
 #else
   // Do one of:
   //  - assert(false)
   //  - abort()
   //  - ignore
 #endif
}

Note: In the sample test fix (.../array/at.pass.cpp and include/array) attached
to this patch, an assert() is used in the -fno-exceptions code path. The use of
assert() is unsafe here as compiling with NDEBUG will get rid of them, resulting
in code that essentially ignores the erroneous state.

The test changes presented above catches such situations where abort() is not
called. The throw_helper() functions in the __noexcept header does the right
thing depending on the availability of exceptions. So the library developers can
use these helper functions and simplify the code. With the use of helper
functions, above code snippet reduces to:

#include <__noexcept>
...
if (some_bad_state())
  throw_helper<some_exception>("some message");

Note that no significant loss of readability is introduced. This also has the
added benefit of being able to easily track down all the places in the library
that throws exceptions.

http://reviews.llvm.org/D14653

Files:
  include/__noexcept
  include/array
  test/std/containers/sequences/array/at.pass.cpp
  test/support/noexcept.h

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D14653.40145.patch
Type: text/x-patch
Size: 4578 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151113/b86336b5/attachment-0001.bin>