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

Index: test/support/noexcept.h
===================================================================
--- /dev/null
+++ test/support/noexcept.h
@@ -0,0 +1,40 @@
+// -*- C++ -*-
+//===----------------------------- noexcept.h -----------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+#ifdef _LIBCPP_NO_EXCEPTIONS
+
+#include<setjmp.h>
+#include<cstdlib>
+
+jmp_buf try_buf;
+
+// Re-write try/catch with if/else to mimic a similar control flow when testing
+// the no-exceptions library variant. The idea is to save as much of the usual
+// with-exceptions assertions as possible. This of course does not work when
+// there are multiple catch statements, in those cases we have to use the
+// _LIBCPP_NO_EXCEPTIONS macro as appropriate; such cases are rare.
+#define try if(!setjmp(try_buf))
+#define catch(ex) else
+
+// The default assert macro calls abort(), we don't want that.
+#undef assert
+
+static void noexcept_assert_failed(const char *exp, const char *file, int line) {
+  fprintf(stderr, "Assertion failed: (%s), at %s line %d.\n", exp, file, line);
+  exit(-1);
+}
+
+#define assert(e) (e) ? (void) 0 : noexcept_assert_failed(#e, __FILE__, __LINE__);
+
+// Jump back to the catch (now else) clause.
+extern "C" void abort(void) {
+  longjmp(try_buf, 1);
+}
+
+#endif // _LIBCPP_NO_EXCEPTIONS
Index: test/std/containers/sequences/array/at.pass.cpp
===================================================================
--- test/std/containers/sequences/array/at.pass.cpp
+++ test/std/containers/sequences/array/at.pass.cpp
@@ -7,7 +7,6 @@
 //
 //===----------------------------------------------------------------------===//
 
-// XFAIL: libcpp-no-exceptions
 // <array>
 
 // reference operator[] (size_type)
@@ -19,6 +18,7 @@
 #include <cassert>
 
 #include "test_macros.h"
+#include "noexcept.h"
 
 // std::array is explicitly allowed to be initialized with A a = { init-list };.
 // Disable the missing braces warning for this reason.
Index: include/array
===================================================================
--- include/array
+++ include/array
@@ -110,6 +110,7 @@
 #if defined(_LIBCPP_NO_EXCEPTIONS)
     #include <cassert>
 #endif
+#include <__noexcept>
 
 #if !defined(_LIBCPP_HAS_NO_PRAGMA_SYSTEM_HEADER)
 #pragma GCC system_header
@@ -201,11 +202,7 @@
 array<_Tp, _Size>::at(size_type __n)
 {
     if (__n >= _Size)
-#ifndef _LIBCPP_NO_EXCEPTIONS
-        throw out_of_range("array::at");
-#else
-        assert(!"array::at out_of_range");
-#endif
+        throw_helper1<out_of_range>("array::at out_of_range");
     return __elems_[__n];
 }
 
@@ -215,11 +212,7 @@
 array<_Tp, _Size>::at(size_type __n) const
 {
     if (__n >= _Size)
-#ifndef _LIBCPP_NO_EXCEPTIONS
-        throw out_of_range("array::at");
-#else
-        assert(!"array::at out_of_range");
-#endif
+        throw_helper1<out_of_range>("array::at out_of_range");
     return __elems_[__n];
 }
 
Index: include/__noexcept
===================================================================
--- /dev/null
+++ include/__noexcept
@@ -0,0 +1,59 @@
+// -*- C++ -*-
+//===--------------------------- __noexcept ----------------------------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is dual licensed under the MIT and the University of Illinois Open
+// Source Licenses. See LICENSE.TXT for details.
+//
+//===-------------------------------------------------------------------------===//
+
+#ifndef _LIBCPP_NOEXCEPT_H
+#define _LIBCPP_NOEXCEPT_H
+
+#include <__config>
+
+#ifdef _LIBCPP_NO_EXCEPTIONS
+#include<cstdlib>
+#include<cstdio>
+#endif
+
+/* Variants of the throw_helper() function for different exception throwing
+   needs. */
+
+template<typename T>
+inline void throw_helper1(const char *msg)
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    throw T();
+#else
+    if (msg)
+      fprintf(stderr, "%s\n", msg);
+    abort();
+#endif
+}
+
+template<typename T>
+inline void throw_helper2(const char *msg)
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    throw T(msg);
+#else
+    fprintf(stderr, "%s\n", msg);
+    abort();
+#endif
+}
+
+template<typename T, typename K>
+inline void throw_helper3(K k, const char *msg)
+{
+#ifndef _LIBCPP_NO_EXCEPTIONS
+    throw T(k);
+#else
+    if (msg)
+      fprintf(stderr, "%s\n", msg);
+    abort();
+#endif
+}
+
+#endif // _LIBCPP_NOEXCEPT_H
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to