From 9b177e8501ee56fe79a1cb9d34557bc7cc089935 Mon Sep 17 00:00:00 2001 From: Romain Goyet Date: Wed, 12 Sep 2018 11:37:19 +0200 Subject: [PATCH] [poincare] use Poincare::ExceptionCheckpoint to handle exceptions --- apps/apps_container.cpp | 16 +++--- apps/apps_container.h | 2 + poincare/Makefile | 1 + .../include/poincare/exception_checkpoint.h | 57 +++++++++++++++++++ poincare/include/poincare/tree_pool.h | 13 +---- poincare/src/exception_checkpoint.cpp | 28 +++++++++ poincare/src/tree_pool.cpp | 23 +------- poincare/test/parser.cpp | 29 ++++++---- poincare/test/tree/tree_by_reference.cpp | 8 +-- quiz/src/runner.cpp | 42 +++++++------- 10 files changed, 143 insertions(+), 76 deletions(-) create mode 100644 poincare/include/poincare/exception_checkpoint.h create mode 100644 poincare/src/exception_checkpoint.cpp diff --git a/apps/apps_container.cpp b/apps/apps_container.cpp index f8fea9c26..d44192722 100644 --- a/apps/apps_container.cpp +++ b/apps/apps_container.cpp @@ -1,7 +1,7 @@ #include "apps_container.h" #include "global_preferences.h" #include -#include +#include extern "C" { #include @@ -83,17 +83,17 @@ void AppsContainer::suspend(bool checkIfPowerKeyReleased) { } bool AppsContainer::dispatchEvent(Ion::Events::Event event) { - jmp_buf jumpEnvironment; - Poincare::TreePool::sharedPool()->setJumpEnvironment(&jumpEnvironment); - int res = setjmp(jumpEnvironment); - if (res != 0) { - // There has been an exception, return an uninitialized node - Poincare::TreePool::sharedPool()->resetJumpEnvironment();// TODO Needed? + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { + return dispatchEventInner(event); + } else { switchTo(appSnapshotAtIndex(0)); //displayMemoryExhaustionPopUp(); TODO return true; } +} +bool AppsContainer::dispatchEventInner(Ion::Events::Event event) { bool alphaLockWantsRedraw = updateAlphaLock(); bool didProcessEvent = false; @@ -125,10 +125,8 @@ bool AppsContainer::dispatchEvent(Ion::Events::Event event) { } if (!didProcessEvent && alphaLockWantsRedraw) { window()->redraw(); - Poincare::TreePool::sharedPool()->resetJumpEnvironment(); // TODO Needed? return true; } - Poincare::TreePool::sharedPool()->resetJumpEnvironment(); // TODO Needed? return didProcessEvent || alphaLockWantsRedraw; } diff --git a/apps/apps_container.h b/apps/apps_container.h index eb1b75516..a8dee34b0 100644 --- a/apps/apps_container.h +++ b/apps/apps_container.h @@ -59,6 +59,8 @@ private: bool processEvent(Ion::Events::Event event); void resetShiftAlphaStatus(); bool updateAlphaLock(); + bool dispatchEventInner(Ion::Events::Event event); + AppsWindow m_window; EmptyBatteryWindow m_emptyBatteryWindow; #if USE_PIC_VIEW_APP diff --git a/poincare/Makefile b/poincare/Makefile index f79c43db3..6e4e68902 100644 --- a/poincare/Makefile +++ b/poincare/Makefile @@ -34,6 +34,7 @@ objs += $(addprefix poincare/src/,\ objs += $(addprefix poincare/src/,\ init.o\ + exception_checkpoint.o\ ) objs += $(addprefix poincare/src/,\ diff --git a/poincare/include/poincare/exception_checkpoint.h b/poincare/include/poincare/exception_checkpoint.h new file mode 100644 index 000000000..a047f10ba --- /dev/null +++ b/poincare/include/poincare/exception_checkpoint.h @@ -0,0 +1,57 @@ +#ifndef POINCARE_EXCEPTION_CHECKPOINT_H +#define POINCARE_EXCEPTION_CHECKPOINT_H + +#include "tree_pool.h" +#include "tree_node.h" +#include + + +/* Usage: + * + * CAUTION : A scope MUST be created directly around the ExceptionCheckpoint + +void errorCatcher() { + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { + CodeUsingPoincare(); + } else { + ErrorHandler(); + } +} + +To raise an error : ExceptionCheckpoint::Raise(); + +*/ + +#define ExceptionRun(ecp) (setjmp(*(ecp.jumpBuffer())) == 0) + +namespace Poincare { + +class ExceptionCheckpoint { +public: + static void Raise() { + assert(s_topmostExceptionCheckpoint != nullptr); + s_topmostExceptionCheckpoint->rollback(); + } + + ExceptionCheckpoint(); + + ~ExceptionCheckpoint() { + s_topmostExceptionCheckpoint = m_parent; + } + + jmp_buf * jumpBuffer() { return &m_jumpBuffer; } + +private: + void rollback(); + + static ExceptionCheckpoint * s_topmostExceptionCheckpoint; + + jmp_buf m_jumpBuffer; + TreeNode * m_endOfPoolBeforeCheckpoint; + ExceptionCheckpoint * m_parent; +}; + +} + +#endif diff --git a/poincare/include/poincare/tree_pool.h b/poincare/include/poincare/tree_pool.h index ae0ec345c..1c2f2d135 100644 --- a/poincare/include/poincare/tree_pool.h +++ b/poincare/include/poincare/tree_pool.h @@ -6,7 +6,6 @@ #include #include #include -#include #if POINCARE_TREE_LOG #include #include @@ -21,18 +20,12 @@ class TreeByReference; class TreePool { friend class TreeNode; friend class TreeByReference; + friend class ExceptionCheckpoint; public: static TreePool * sharedPool() { assert(SharedStaticPool != nullptr); return SharedStaticPool; } static void RegisterPool(TreePool * pool) { assert(SharedStaticPool == nullptr); SharedStaticPool = pool; } - TreePool() : - m_cursor(m_buffer), - m_currentJumpEnvironment(nullptr), - m_endOfPoolBeforeJump(nullptr) - {} - void setJumpEnvironment(jmp_buf * env); - jmp_buf * jumpEnvironment() { return m_currentJumpEnvironment; } - void resetJumpEnvironment(); + TreePool() : m_cursor(m_buffer) {} // Node TreeNode * node(int identifier) const { @@ -173,8 +166,6 @@ private: char m_buffer[BufferSize]; IdentifierStack m_identifiers; TreeNode * m_nodeForIdentifier[MaxNumberOfNodes]; - jmp_buf * m_currentJumpEnvironment; //TODO make static? - TreeNode * m_endOfPoolBeforeJump; #if POINCARE_ALLOW_STATIC_NODES TreeNode * m_staticNodes[MaxNumberOfStaticNodes]; #endif diff --git a/poincare/src/exception_checkpoint.cpp b/poincare/src/exception_checkpoint.cpp new file mode 100644 index 000000000..8ef9b4ed9 --- /dev/null +++ b/poincare/src/exception_checkpoint.cpp @@ -0,0 +1,28 @@ +#include + +namespace Poincare { + +ExceptionCheckpoint * ExceptionCheckpoint::s_topmostExceptionCheckpoint; + +ExceptionCheckpoint::ExceptionCheckpoint() : + m_endOfPoolBeforeCheckpoint(TreePool::sharedPool()->last()), + m_parent(s_topmostExceptionCheckpoint) +{ + s_topmostExceptionCheckpoint = this; +} + +/* +int ExceptionCheckpoint::run() { + m_endOfPoolBeforeCheckpoint = TreePool::sharedPool()->last(); + m_parent = s_topmostExceptionCheckpoint; + s_topmostExceptionCheckpoint = this; + return setjmp(m_jumpBuffer) == 0; +} +*/ + +void ExceptionCheckpoint::rollback() { + Poincare::TreePool::sharedPool()->freePoolFromNode(m_endOfPoolBeforeCheckpoint); + longjmp(m_jumpBuffer, 1); +} + +} diff --git a/poincare/src/tree_pool.cpp b/poincare/src/tree_pool.cpp index efd874bca..bf62f18ed 100644 --- a/poincare/src/tree_pool.cpp +++ b/poincare/src/tree_pool.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -25,18 +26,6 @@ static void memmove32(uint32_t * dst, uint32_t * src, size_t len) { } } -void TreePool::setJumpEnvironment(jmp_buf * env) { - m_currentJumpEnvironment = env; - m_endOfPoolBeforeJump = last(); -} - -void TreePool::resetJumpEnvironment() { - assert(m_currentJumpEnvironment != nullptr); - assert(m_endOfPoolBeforeJump != nullptr); - m_currentJumpEnvironment = nullptr; - m_endOfPoolBeforeJump = nullptr; -} - void TreePool::freeIdentifier(int identifier) { if (identifier >= 0 && identifier < MaxNumberOfNodes) { m_nodeForIdentifier[identifier] = nullptr; @@ -165,16 +154,8 @@ int TreePool::numberOfNodes() const { #endif void * TreePool::alloc(size_t size) { - /* We are going to try to allocate memory in the pool. If it fails, we must be - * able to escape. We thus assert that m_currentJumpEnvironment is set, so we - * can make a long jump. */ - if (m_cursor >= m_buffer + BufferSize || m_cursor + size > m_buffer + BufferSize) { - assert(m_currentJumpEnvironment != nullptr); - assert(m_endOfPoolBeforeJump != nullptr); - // TODO put the asserts outside the if - freePoolFromNode(m_endOfPoolBeforeJump); - longjmp(*m_currentJumpEnvironment, 1); + ExceptionCheckpoint::Raise(); } void * result = m_cursor; m_cursor += size; diff --git a/poincare/test/parser.cpp b/poincare/test/parser.cpp index 3f1ea1937..08bf31e97 100644 --- a/poincare/test/parser.cpp +++ b/poincare/test/parser.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include #include @@ -31,23 +32,27 @@ QUIZ_CASE(poincare_parser) { #endif } + + QUIZ_CASE(poincare_parser_memory_exhaustion) { - int memoryFailureHasBeenHandled = false; - jmp_buf jumpBuffer; int initialPoolSize = pool_size(); - TreePool::sharedPool()->setJumpEnvironment(&jumpBuffer); - if (setjmp(jumpBuffer) == 0) { - Addition a = Addition(); - while (true) { - Expression e = Expression::parse("1+2+3+4+5+6+7+8+9+10"); - a.addChildAtIndexInPlace(e, 0, a.numberOfChildren()); + int memoryFailureHasBeenHandled = false; + { + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { + Addition a = Addition(); + while (true) { + Expression e = Expression::parse("1+2+3+4+5+6+7+8+9+10"); + a.addChildAtIndexInPlace(e, 0, a.numberOfChildren()); + } + } else { + memoryFailureHasBeenHandled = true; } - } else { - memoryFailureHasBeenHandled = true; } + assert(memoryFailureHasBeenHandled); assert_pool_size(initialPoolSize); Expression e = Expression::parse("1+1"); - // Stupid check to make sure the global variable generated by Bison is not - // ruining everything + /* Stupid check to make sure the global variable generated by Bison is not + * ruining everything */ } diff --git a/poincare/test/tree/tree_by_reference.cpp b/poincare/test/tree/tree_by_reference.cpp index 27f2d648a..6fead78ec 100644 --- a/poincare/test/tree/tree_by_reference.cpp +++ b/poincare/test/tree/tree_by_reference.cpp @@ -1,5 +1,6 @@ #include #include +#include #include #include "helpers.h" @@ -60,11 +61,10 @@ QUIZ_CASE(tree_by_reference_can_be_returned) { } QUIZ_CASE(tree_by_reference_memory_failure) { - int memoryFailureHasBeenHandled = false; - jmp_buf jumpBuffer; int initialPoolSize = pool_size(); - TreePool::sharedPool()->setJumpEnvironment(&jumpBuffer); - if (setjmp(jumpBuffer) == 0) { + int memoryFailureHasBeenHandled = false; + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { TreeByReference tree = BlobByReference(1); while (true) { tree = PairByReference(tree, BlobByReference(1)); diff --git a/quiz/src/runner.cpp b/quiz/src/runner.cpp index 67d1b2461..bc1f9c2c4 100644 --- a/quiz/src/runner.cpp +++ b/quiz/src/runner.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include void quiz_print(const char * message) { #if QUIZ_USE_CONSOLE @@ -23,24 +23,7 @@ void quiz_print(const char * message) { #endif } -void ion_main(int argc, char * argv[]) { - // Initialize Poincare::TreePool::sharedPool - Poincare::init(); - - jmp_buf jumpEnvironment; - Poincare::TreePool::sharedPool()->setJumpEnvironment(&jumpEnvironment); - int res = setjmp(jumpEnvironment); - if (res != 0) { - // There has been a memeory allocation problem - assert(false); -#if !QUIZ_USE_CONSOLE - while (1) { - Ion::msleep(1000); - } -#else - return; -#endif - } +static inline void ion_main_inner() { int i = 0; while (quiz_cases[i] != NULL) { QuizCase c = quiz_cases[i]; @@ -55,3 +38,24 @@ void ion_main(int argc, char * argv[]) { } #endif } + +void ion_main(int argc, char * argv[]) { + // Initialize Poincare::TreePool::sharedPool + Poincare::init(); + + Poincare::ExceptionCheckpoint ecp; + if (ExceptionRun(ecp)) { + ion_main_inner(); + } else { + // There has been a memeory allocation problem +#if POINCARE_TREE_LOG + Poincare::TreePool::sharedPool()->log(); +#endif + assert(false); +#if !QUIZ_USE_CONSOLE + while (1) { + Ion::msleep(1000); + } +#endif + } +}