Merge pull request #815 from kmod/exceptions

Clean up our exception handling

.gitignore

@@ -32,6 +32,7 @@ pystontmp*/
 *.cache
 tests/t.py
 tests/t2.py
+tests/t3.py
 *.bc
 stdlib.ll
 *.o

src/CMakeLists.txt

@@ -90,6 +90,7 @@ add_library(PYSTON_OBJECTS OBJECT ${OPTIONAL_SRCS}
 		runtime/cxx_unwind.cpp
 		runtime/descr.cpp
 		runtime/dict.cpp
+		runtime/exceptions.cpp
 		runtime/file.cpp
 		runtime/float.cpp
 		runtime/frame.cpp
@@ -104,7 +105,6 @@ add_library(PYSTON_OBJECTS OBJECT ${OPTIONAL_SRCS}
 		runtime/long.cpp
 		runtime/objmodel.cpp
 		runtime/set.cpp
-		runtime/stacktrace.cpp
 		runtime/str.cpp
 		runtime/super.cpp
 		runtime/traceback.cpp

src/codegen/ast_interpreter.cpp

@@ -333,7 +333,7 @@ Box* ASTInterpreter::execJITedBlock(CFGBlock* b) {
 
         auto source = getCL()->source.get();
         stmt->cxx_exception_count++;
-        exceptionCaughtInInterpreter(LineInfo(stmt->lineno, stmt->col_offset, source->fn, source->getName()), &e);
+        caughtCxxException(LineInfo(stmt->lineno, stmt->col_offset, source->fn, source->getName()), &e);
 
         next_block = ((AST_Invoke*)stmt)->exc_dest;
         last_exception = e;
@@ -773,7 +773,7 @@ Value ASTInterpreter::visit_invoke(AST_Invoke* node) {
 
         auto source = getCL()->source.get();
         node->cxx_exception_count++;
-        exceptionCaughtInInterpreter(LineInfo(node->lineno, node->col_offset, source->fn, source->getName()), &e);
+        caughtCxxException(LineInfo(node->lineno, node->col_offset, source->fn, source->getName()), &e);
 
         next_block = node->exc_dest;
         last_exception = e;

src/codegen/irgen/hooks.h

@@ -39,10 +39,6 @@ void compileAndRunModule(AST_Module* m, BoxedModule* bm);
 // will we always want to generate unique function names? (ie will this function always be reasonable?)
 CompiledFunction* cfForMachineFunctionName(const std::string&);
 
-extern "C" void capiExcCaughtInJit(AST_stmt* current_stmt, void* source_info);
-// This is just meant for the use of the JIT (normal runtime code should call throwCAPIException)
-extern "C" void reraiseJitCapiExc() __attribute__((noreturn));
-
 extern "C" Box* exec(Box* boxedCode, Box* globals, Box* locals, FutureFlags caller_future_flags);
 extern "C" Box* eval(Box* boxedCode, Box* globals, Box* locals);
 extern "C" Box* compile(Box* source, Box* filename, Box* mode, Box** _args /* flags, dont_inherit */);

src/codegen/irgen/irgenerator.cpp

@@ -131,8 +131,8 @@ static llvm::Value* getClosureElementGep(IREmitter& emitter, llvm::Value* closur
 
 static llvm::Value* getBoxedLocalsGep(llvm::IRBuilder<true>& builder, llvm::Value* v) {
     static_assert(offsetof(FrameInfo, exc) == 0, "");
-    static_assert(sizeof(ExcInfo) == 32, "");
-    static_assert(offsetof(FrameInfo, boxedLocals) == 32, "");
+    static_assert(sizeof(ExcInfo) == 24, "");
+    static_assert(offsetof(FrameInfo, boxedLocals) == 24, "");
     return builder.CreateConstInBoundsGEP2_32(v, 0, 1);
 }
 
@@ -143,9 +143,9 @@ static llvm::Value* getExcinfoGep(llvm::IRBuilder<true>& builder, llvm::Value* v
 
 static llvm::Value* getFrameObjGep(llvm::IRBuilder<true>& builder, llvm::Value* v) {
     static_assert(offsetof(FrameInfo, exc) == 0, "");
-    static_assert(sizeof(ExcInfo) == 32, "");
+    static_assert(sizeof(ExcInfo) == 24, "");
     static_assert(sizeof(Box*) == 8, "");
-    static_assert(offsetof(FrameInfo, frame_obj) == 40, "");
+    static_assert(offsetof(FrameInfo, frame_obj) == 32, "");
     return builder.CreateConstInBoundsGEP2_32(v, 0, 2);
     // TODO: this could be made more resilient by doing something like
     // gep->accumulateConstantOffset(g.tm->getDataLayout(), ap_offset)
@@ -2805,14 +2805,14 @@ public:
         capi_current_statements[final_dest] = current_stmt;
 
         emitter.setCurrentBasicBlock(capi_exc_dest);
-        emitter.getBuilder()->CreateCall2(g.funcs.capiExcCaughtInJit,
+        emitter.getBuilder()->CreateCall2(g.funcs.caughtCapiException,
                                           embedRelocatablePtr(current_stmt, g.llvm_aststmt_type_ptr),
                                           embedRelocatablePtr(irstate->getSourceInfo(), g.i8_ptr));
 
         if (!final_dest) {
             // Propagate the exception out of the function:
             if (irstate->getExceptionStyle() == CXX) {
-                emitter.getBuilder()->CreateCall(g.funcs.reraiseJitCapiExc);
+                emitter.getBuilder()->CreateCall(g.funcs.reraiseCapiExcAsCxx);
                 emitter.getBuilder()->CreateUnreachable();
             } else {
                 emitter.getBuilder()->CreateRet(getNullPtr(g.llvm_value_type_ptr));

src/codegen/runtime_hooks.cpp

@@ -312,8 +312,8 @@ void initGlobalFuncs(GlobalState& g) {
     GET(PyErr_Fetch);
     GET(PyErr_NormalizeException);
     GET(PyErr_Restore);
-    GET(capiExcCaughtInJit);
-    GET(reraiseJitCapiExc);
+    GET(caughtCapiException);
+    GET(reraiseCapiExcAsCxx);
     GET(deopt);
 
     GET(div_float_float);

src/codegen/runtime_hooks.h

@@ -51,7 +51,7 @@ struct GlobalFuncs {
 
     llvm::Value* __cxa_end_catch;
     llvm::Value* raise0, *raise3, *raise3_capi;
-    llvm::Value* PyErr_Fetch, *PyErr_NormalizeException, *PyErr_Restore, *capiExcCaughtInJit, *reraiseJitCapiExc;
+    llvm::Value* PyErr_Fetch, *PyErr_NormalizeException, *PyErr_Restore, *caughtCapiException, *reraiseCapiExcAsCxx;
     llvm::Value* deopt;
 
     llvm::Value* div_float_float, *floordiv_float_float, *mod_float_float, *pow_float_float;

src/codegen/unwinding.h

@@ -44,12 +44,19 @@ Box* getTraceback();
 class PythonUnwindSession;
 PythonUnwindSession* beginPythonUnwindSession();
 PythonUnwindSession* getActivePythonUnwindSession();
-void throwingException(PythonUnwindSession* unwind_session);
 void endPythonUnwindSession(PythonUnwindSession* unwind_session);
 void* getPythonUnwindSessionExceptionStorage(PythonUnwindSession* unwind_session);
 void unwindingThroughFrame(PythonUnwindSession* unwind_session, unw_cursor_t* cursor);
 
-void exceptionCaughtInInterpreter(LineInfo line_info, ExcInfo* exc_info);
+// TODO move these to exceptions.h
+void logException(ExcInfo* exc_info);
+void startReraise();
+bool exceptionAtLineCheck();
+void exceptionAtLine(LineInfo line_info, Box** traceback);
+void caughtCxxException(LineInfo line_info, ExcInfo* exc_info);
+extern "C" void caughtCapiException(AST_stmt* current_stmt, void* source_info);
+extern "C" void reraiseCapiExcAsCxx() __attribute__((noreturn));
+
 
 CLFunction* getTopPythonFunction();
 

src/core/types.h

@@ -777,13 +777,8 @@ public:
 
 struct ExcInfo {
     Box* type, *value, *traceback;
-    bool reraise;
 
-#ifndef NDEBUG
-    ExcInfo(Box* type, Box* value, Box* traceback);
-#else
-    ExcInfo(Box* type, Box* value, Box* traceback) : type(type), value(value), traceback(traceback), reraise(false) {}
-#endif
+    constexpr ExcInfo(Box* type, Box* value, Box* traceback) : type(type), value(value), traceback(traceback) {}
     bool matches(BoxedClass* cls) const;
     void printExcAndTraceback() const;
 };

src/runtime/cxx_unwind.cpp

@@ -688,7 +688,7 @@ extern "C" void __cxa_throw(void* exc_obj, std::type_info* tinfo, void (*dtor)(v
     pyston::StatTimer::overrideCounter(unwinding_stattimer);
 #endif
     // let unwinding.cpp know we've started unwinding
-    pyston::throwingException(pyston::getActivePythonUnwindSession());
+    pyston::logException(exc_data);
     pyston::unwind(exc_data);
 }
 

src/runtime/stacktrace.cpp → src/runtime/exceptions.cpp

@@ -25,28 +25,8 @@
 #include "runtime/types.h"
 #include "runtime/util.h"
 
-#define UNW_LOCAL_ONLY
-#include <libunwind.h>
-#undef UNW_LOCAL_ONLY
-
 namespace pyston {
 
-// from http://www.nongnu.org/libunwind/man/libunwind(3).html
-void showBacktrace() {
-    unw_cursor_t cursor;
-    unw_context_t uc;
-    unw_word_t ip, sp;
-
-    unw_getcontext(&uc);
-    unw_init_local(&cursor, &uc);
-
-    while (unw_step(&cursor) > 0) {
-        unw_get_reg(&cursor, UNW_REG_IP, &ip);
-        unw_get_reg(&cursor, UNW_REG_SP, &sp);
-        printf("ip = %lx, sp = %lx\n", (long)ip, (long)sp);
-    }
-}
-
 void raiseExc(Box* exc_obj) {
     assert(!PyErr_Occurred());
     throw ExcInfo(exc_obj->cls, exc_obj, None);
@@ -88,96 +68,6 @@ void raiseSyntaxErrorHelper(llvm::StringRef file, llvm::StringRef func, AST* nod
     raiseSyntaxError(buf, node_at->lineno, node_at->col_offset, file, "");
 }
 
-void _printStacktrace() {
-    static bool recursive = false;
-
-    if (recursive) {
-        fprintf(stderr, "_printStacktrace ran into an issue; refusing to try it again!\n");
-        return;
-    }
-
-    recursive = true;
-    printTraceback(getTraceback());
-    recursive = false;
-}
-
-// where should this go...
-extern "C" void abort() {
-    static void (*libc_abort)() = (void (*)())dlsym(RTLD_NEXT, "abort");
-
-    // In case displaying the traceback recursively calls abort:
-    static bool recursive = false;
-
-    if (!recursive) {
-        recursive = true;
-        Stats::dump();
-        fprintf(stderr, "Someone called abort!\n");
-
-        // If traceback_cls is NULL, then we somehow died early on, and won't be able to display a traceback.
-        if (traceback_cls) {
-
-            // If we call abort(), things may be seriously wrong.  Set an alarm() to
-            // try to handle cases that we would just hang.
-            // (Ex if we abort() from a static constructor, and _printStackTrace uses
-            // that object, _printStackTrace will hang waiting for the first construction
-            // to finish.)
-            alarm(1);
-            try {
-                _printStacktrace();
-            } catch (ExcInfo) {
-                fprintf(stderr, "error printing stack trace during abort()");
-            }
-
-            // Cancel the alarm.
-            // This is helpful for when running in a debugger, since otherwise the debugger will catch the
-            // abort and let you investigate, but the alarm will still come back to kill the program.
-            alarm(0);
-        }
-    }
-
-    if (PAUSE_AT_ABORT) {
-        fprintf(stderr, "PID %d about to call libc abort; pausing for a debugger...\n", getpid());
-
-        // Sometimes stderr isn't available (or doesn't immediately appear), so write out a file
-        // just in case:
-        FILE* f = fopen("pausing.txt", "w");
-        if (f) {
-            fprintf(f, "PID %d about to call libc abort; pausing for a debugger...\n", getpid());
-            fclose(f);
-        }
-
-        while (true) {
-            sleep(1);
-        }
-    }
-    libc_abort();
-    __builtin_unreachable();
-}
-
-#if 0
-extern "C" void exit(int code) {
-    static void (*libc_exit)(int) = (void (*)(int))dlsym(RTLD_NEXT, "exit");
-
-    if (code == 0) {
-        libc_exit(0);
-        __builtin_unreachable();
-    }
-
-    fprintf(stderr, "Someone called exit with code=%d!\n", code);
-
-    // In case something calls exit down the line:
-    static bool recursive = false;
-    if (!recursive) {
-        recursive = true;
-
-        _printStacktrace();
-    }
-
-    libc_exit(code);
-    __builtin_unreachable();
-}
-#endif
-
 extern "C" void raise0() {
     ExcInfo* exc_info = getFrameExcInfo();
     assert(exc_info->type);
@@ -186,17 +76,11 @@ extern "C" void raise0() {
     if (exc_info->type == None)
         raiseExcHelper(TypeError, "exceptions must be old-style classes or derived from BaseException, not NoneType");
 
-    exc_info->reraise = true;
+    startReraise();
     assert(!PyErr_Occurred());
     throw * exc_info;
 }
 
-#ifndef NDEBUG
-ExcInfo::ExcInfo(Box* type, Box* value, Box* traceback)
-    : type(type), value(value), traceback(traceback), reraise(false) {
-}
-#endif
-
 void ExcInfo::printExcAndTraceback() const {
     PyErr_Display(type, value, traceback);
 }
@@ -268,7 +152,9 @@ extern "C" void raise3(Box* arg0, Box* arg1, Box* arg2) {
     bool reraise = arg2 != NULL && arg2 != None;
     auto exc_info = excInfoForRaise(arg0, arg1, arg2);
 
-    exc_info.reraise = reraise;
+    if (reraise)
+        startReraise();
+
     assert(!PyErr_Occurred());
     throw exc_info;
 }
@@ -279,12 +165,11 @@ extern "C" void raise3_capi(Box* arg0, Box* arg1, Box* arg2) noexcept {
     ExcInfo exc_info(NULL, NULL, NULL);
     try {
         exc_info = excInfoForRaise(arg0, arg1, arg2);
-        exc_info.reraise = reraise;
     } catch (ExcInfo e) {
         exc_info = e;
     }
 
-    assert(!exc_info.reraise); // would get thrown away
+    assert(!reraise); // would get thrown away
     PyErr_Restore(exc_info.type, exc_info.value, exc_info.traceback);
 }
 
@@ -316,4 +201,71 @@ void raiseExcHelper(BoxedClass* cls, const char* msg, ...) {
         raiseExc(exc_obj);
     }
 }
+
+
+void logException(ExcInfo* exc_info) {
+#if STAT_EXCEPTIONS
+    static StatCounter num_exceptions("num_exceptions");
+    num_exceptions.log();
+
+    std::string stat_name;
+    if (PyType_Check(exc_info->type))
+        stat_name = "num_exceptions_" + std::string(static_cast<BoxedClass*>(exc_info->type)->tp_name);
+    else
+        stat_name = "num_exceptions_" + std::string(exc_info->value->cls->tp_name);
+    Stats::log(Stats::getStatCounter(stat_name));
+#if STAT_EXCEPTIONS_LOCATION
+    logByCurrentPythonLine(stat_name);
+#endif
+#endif
+}
+
+extern "C" void caughtCapiException(AST_stmt* stmt, void* _source_info) {
+    SourceInfo* source = static_cast<SourceInfo*>(_source_info);
+    PyThreadState* tstate = PyThreadState_GET();
+
+    exceptionAtLine(LineInfo(stmt->lineno, stmt->col_offset, source->fn, source->getName()), &tstate->curexc_traceback);
+}
+
+extern "C" void reraiseCapiExcAsCxx() {
+    ensureCAPIExceptionSet();
+    // TODO: we are normalizing to many times?
+    ExcInfo e = excInfoForRaise(cur_thread_state.curexc_type, cur_thread_state.curexc_value,
+                                cur_thread_state.curexc_traceback);
+    PyErr_Clear();
+    startReraise();
+    throw e;
+}
+
+void caughtCxxException(LineInfo line_info, ExcInfo* exc_info) {
+    static StatCounter frames_unwound("num_frames_unwound_python");
+    frames_unwound.log();
+
+    exceptionAtLine(line_info, &exc_info->traceback);
+}
+
+
+
+struct ExcState {
+    bool is_reraise;
+    constexpr ExcState() : is_reraise(false) {}
+} static __thread exc_state;
+
+bool exceptionAtLineCheck() {
+    if (exc_state.is_reraise) {
+        exc_state.is_reraise = false;
+        return false;
+    }
+    return true;
+}
+
+void exceptionAtLine(LineInfo line_info, Box** traceback) {
+    if (exceptionAtLineCheck())
+        BoxedTraceback::here(line_info, traceback);
+}
+
+void startReraise() {
+    assert(!exc_state.is_reraise);
+    exc_state.is_reraise = true;
+}
 }

src/runtime/inline/link_forcer.cpp

@@ -130,8 +130,8 @@ void force() {
     FORCE(PyErr_Fetch);
     FORCE(PyErr_NormalizeException);
     FORCE(PyErr_Restore);
-    FORCE(capiExcCaughtInJit);
-    FORCE(reraiseJitCapiExc);
+    FORCE(caughtCapiException);
+    FORCE(reraiseCapiExcAsCxx);
     FORCE(deopt);
 
     FORCE(div_i64_i64);

tools/tester.py

@@ -518,7 +518,7 @@ def main(orig_dir):
     patterns = opts.pattern
 
     if not patterns and not TESTS_TO_SKIP:
-        TESTS_TO_SKIP = ["t", "t2"]
+        TESTS_TO_SKIP = ["t", "t2", "t3"]
 
     assert os.path.isdir(TEST_DIR), "%s doesn't look like a directory with tests in it" % TEST_DIR