From 3f3773757eb2cf5d62caf5e01adf8611c8684dde Mon Sep 17 00:00:00 2001 From: Gabe Black Date: Fri, 7 Sep 2018 01:37:57 -0700 Subject: [PATCH] systemc: Fortify how exceptions are caught and passed around. This change tightens up exception catching and makes gem5's systemc code react to exceptions more in line with the Accellera implementation. This prevents exceptions from being caught by the pybind11 integration which makes it very difficult to see where an exception came from, and makes the output differ by including a (mostly useless) backtrace. Change-Id: I7130d53a98fadd137073d1718f780f32f57c658c Reviewed-on: https://gem5-review.googlesource.com/c/12601 Reviewed-by: Gabe Black Maintainer: Gabe Black --- src/systemc/core/process.cc | 2 + src/systemc/core/process_types.hh | 8 ++- src/systemc/core/sc_main.cc | 4 ++ src/systemc/core/scheduler.cc | 73 ++++++++++++++++++++-- src/systemc/core/scheduler.hh | 6 ++ src/systemc/ext/utils/sc_report_handler.hh | 4 ++ src/systemc/tests/config.py | 7 +-- src/systemc/utils/sc_report_handler.cc | 6 ++ 8 files changed, 101 insertions(+), 9 deletions(-) diff --git a/src/systemc/core/process.cc b/src/systemc/core/process.cc index 317e8c3a28..cc2eff5128 100644 --- a/src/systemc/core/process.cc +++ b/src/systemc/core/process.cc @@ -343,6 +343,8 @@ Process::run() } catch(const ::sc_core::sc_unwind_exception &exc) { reset = exc.is_reset(); _isUnwinding = false; + } catch (...) { + throw; } } while (reset); needsStart(true); diff --git a/src/systemc/core/process_types.hh b/src/systemc/core/process_types.hh index 6f603e73f6..748c249147 100644 --- a/src/systemc/core/process_types.hh +++ b/src/systemc/core/process_types.hh @@ -92,7 +92,13 @@ class Thread : public Process main() override { thread->_needsStart = false; - thread->run(); + try { + thread->run(); + } catch (...) { + thread->terminate(); + scheduler.throwToScMain(); + return; + } thread->terminate(); scheduler.yield(); } diff --git a/src/systemc/core/sc_main.cc b/src/systemc/core/sc_main.cc index 2637cef0e6..5e1cd4fe64 100644 --- a/src/systemc/core/sc_main.cc +++ b/src/systemc/core/sc_main.cc @@ -79,6 +79,10 @@ class ScMainFiber : public Fiber } catch (const sc_report &r) { // There was an exception nobody caught. resultStr = r.what(); + } catch (...) { + // There was some other type of exception we need to wrap. + const sc_report *r = ::sc_gem5::reportifyException(); + resultStr = r->what(); } ::sc_gem5::Kernel::scMainFinished(true); ::sc_gem5::scheduler.clear(); diff --git a/src/systemc/core/scheduler.cc b/src/systemc/core/scheduler.cc index 4c98b68aa7..5348e6665e 100644 --- a/src/systemc/core/scheduler.cc +++ b/src/systemc/core/scheduler.cc @@ -34,6 +34,8 @@ #include "sim/eventq.hh" #include "systemc/core/kernel.hh" #include "systemc/ext/core/sc_main.hh" +#include "systemc/ext/utils/sc_report.hh" +#include "systemc/ext/utils/sc_report_handler.hh" namespace sc_gem5 { @@ -42,7 +44,7 @@ Scheduler::Scheduler() : eq(nullptr), readyEvent(this, false, ReadyPriority), pauseEvent(this, false, PausePriority), stopEvent(this, false, StopPriority), - scMain(nullptr), + scMain(nullptr), _throwToScMain(nullptr), starvationEvent(this, false, StarvationPriority), _started(false), _paused(false), _stopped(false), _stopNow(false), maxTickEvent(this, false, MaxTickPriority), @@ -184,7 +186,11 @@ Scheduler::yield() // If the current process needs to be manually started, start it. if (_current && _current->needsStart()) { _current->needsStart(false); - _current->run(); + try { + _current->run(); + } catch (...) { + throwToScMain(); + } } } if (_current && _current->excWrapper) { @@ -336,7 +342,8 @@ Scheduler::pause() _paused = true; kernel->status(::sc_core::SC_PAUSED); runOnce = false; - scMain->run(); + if (scMain && !scMain->finished()) + scMain->run(); } void @@ -348,7 +355,8 @@ Scheduler::stop() clear(); runOnce = false; - scMain->run(); + if (scMain && !scMain->finished()) + scMain->run(); } void @@ -385,6 +393,12 @@ Scheduler::start(Tick max_tick, bool run_to_time) deschedule(&maxTickEvent); if (starvationEvent.scheduled()) deschedule(&starvationEvent); + + if (_throwToScMain) { + const ::sc_core::sc_report *to_throw = _throwToScMain; + _throwToScMain = nullptr; + throw *to_throw; + } } void @@ -404,6 +418,15 @@ Scheduler::schedulePause() schedule(&pauseEvent); } +void +Scheduler::throwToScMain(const ::sc_core::sc_report *r) +{ + if (!r) + r = reportifyException(); + _throwToScMain = r; + scMain->run(); +} + void Scheduler::scheduleStop(bool finish_delta) { @@ -421,4 +444,46 @@ Scheduler::scheduleStop(bool finish_delta) Scheduler scheduler; +namespace { + +void +throwingReportHandler(const ::sc_core::sc_report &r, + const ::sc_core::sc_actions &) +{ + throw r; +} + +} // anonymous namespace + +const ::sc_core::sc_report * +reportifyException() +{ + ::sc_core::sc_report_handler_proc old_handler = + ::sc_core::sc_report_handler::get_handler(); + ::sc_core::sc_report_handler::set_handler(&throwingReportHandler); + + try { + try { + // Rethrow the current exception so we can catch it and throw an + // sc_report instead if it's not a type we recognize/can handle. + throw; + } catch (const ::sc_core::sc_report &) { + // It's already a sc_report, so nothing to do. + throw; + } catch (const ::sc_core::sc_unwind_exception &) { + panic("Kill/reset exception escaped a Process::run()"); + } catch (const std::exception &e) { + SC_REPORT_ERROR("uncaught exception", e.what()); + } catch (const char *msg) { + SC_REPORT_ERROR("uncaught exception", msg); + } catch (...) { + SC_REPORT_ERROR("uncaught exception", "UNKNOWN EXCEPTION"); + } + } catch (const ::sc_core::sc_report &r) { + ::sc_core::sc_report_handler::set_handler(old_handler); + return &r; + } + panic("No exception thrown in reportifyException."); +} + } // namespace sc_gem5 diff --git a/src/systemc/core/scheduler.hh b/src/systemc/core/scheduler.hh index 29501bc24e..dda483f0cf 100644 --- a/src/systemc/core/scheduler.hh +++ b/src/systemc/core/scheduler.hh @@ -333,6 +333,8 @@ class Scheduler uint64_t changeStamp() { return _changeStamp; } + void throwToScMain(const ::sc_core::sc_report *r=nullptr); + private: typedef const EventBase::Priority Priority; static Priority DefaultPriority = EventBase::Default_Pri; @@ -377,7 +379,9 @@ class Scheduler void stop(); EventWrapper pauseEvent; EventWrapper stopEvent; + Fiber *scMain; + const ::sc_core::sc_report *_throwToScMain; bool starved() @@ -437,6 +441,8 @@ Scheduler::TimeSlot::process() scheduler.completeTimeSlot(this); } +const ::sc_core::sc_report *reportifyException(); + } // namespace sc_gem5 #endif // __SYSTEMC_CORE_SCHEDULER_H__ diff --git a/src/systemc/ext/utils/sc_report_handler.hh b/src/systemc/ext/utils/sc_report_handler.hh index f65b32ccae..6ce20afb45 100644 --- a/src/systemc/ext/utils/sc_report_handler.hh +++ b/src/systemc/ext/utils/sc_report_handler.hh @@ -105,6 +105,10 @@ class sc_report_handler static void set_handler(sc_report_handler_proc); static void default_handler(const sc_report &, const sc_actions &); static sc_actions get_new_action_id(); + // Nonstandard + // Returns the current handler so it can be restored if it needs to be + // changed temporarily. + static sc_report_handler_proc get_handler(); static sc_report *get_cached_report(); static void clear_cached_report(); diff --git a/src/systemc/tests/config.py b/src/systemc/tests/config.py index 439a6828ce..d9ae749942 100755 --- a/src/systemc/tests/config.py +++ b/src/systemc/tests/config.py @@ -31,6 +31,7 @@ import argparse import m5 import os import re +import sys from m5.objects import SystemC_Kernel, Root @@ -58,7 +59,5 @@ if result.code != 0: # generate errors, and as long as their output matches that's still # considered correct. A "real" systemc config should expect sc_main # (if present) not to fail. - scrubbed = re.sub(r'In file: .*$', - 'In file: ', - result.message) - print('\n' + scrubbed) + print('\n' + result.message) + sys.exit(int(result.code)) diff --git a/src/systemc/utils/sc_report_handler.cc b/src/systemc/utils/sc_report_handler.cc index e08ad66248..1ca9d975d7 100644 --- a/src/systemc/utils/sc_report_handler.cc +++ b/src/systemc/utils/sc_report_handler.cc @@ -341,6 +341,12 @@ sc_report_handler::get_new_action_id() return maxAction; } +sc_report_handler_proc +sc_report_handler::get_handler() +{ + return reportHandlerProc; +} + sc_report * sc_report_handler::get_cached_report() {