From 4bfd2631843b562962ec46961186e5bebee693a4 Mon Sep 17 00:00:00 2001
From: Chris Jefferson <caj21@st-andrews.ac.uk>
Date: Sat, 5 May 2018 11:38:52 +0100
Subject: [PATCH] Recover from longjmp Error handling in profiling

Previously, profiling did not detect when Error caused one or more
functions to exit. This lead to incorrect and very deep stack
traces.
---
 src/profile.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 71 insertions(+), 3 deletions(-)

diff --git a/src/profile.c b/src/profile.c
index 7606a86fe86..b7c267ea9ac 100644
--- a/src/profile.c
+++ b/src/profile.c
@@ -14,6 +14,7 @@
 #include <src/bool.h>
 #include <src/calls.h>
 #include <src/code.h>
+#include <src/funcs.h>
 #include <src/error.h>
 #include <src/hookintrprtr.h>
 #include <src/io.h>
@@ -95,7 +96,6 @@
 **  never marked as executed.
 */
 
-
 /****************************************************************************
 **
 ** Store the current state of the profiler
@@ -146,11 +146,56 @@ struct ProfileState
   ** clear */
   UInt profiledPreviously;
 
+  Int LongJmpOccurred;
+
+  // We store the value of RecursionDepth each time we enter a function.
+  // This is the only way to detect if GAP has left a function by performing
+  // a longjmp.
+  // We need to store the actual values, as RecursionDepth can increase
+  // by more than one when a GAP function is called
+  Obj visitedDepths;
 } profileState;
 
 /* We keep this seperate as it is exported for use in other files */
 UInt profileState_Active;
 
+
+static void ProfileRegisterLongJmpOccurred(void)
+{
+    profileState.LongJmpOccurred = 1;
+}
+
+// This function is called when we detect a longjmp occurred, and
+// outputs a 'return' into the profile for any function which was
+// jumped over.
+// It is fine for this function to be called when a longjmp has not
+// occurred, or when no function was longjmped over.
+static void CheckLeaveFunctionsAfterLongjmp(void)
+{
+    if (!profileState.LongJmpOccurred)
+        return;
+
+#ifdef HPCGAP
+    if (profileState.profiledThread != TLS(threadID))
+        return;
+#endif
+
+    profileState.LongJmpOccurred = 0;
+
+    Int pos = LEN_PLIST(profileState.visitedDepths);
+    Int depth = GetRecursionDepth();
+
+    while (pos > 0 && INT_INTOBJ(ELM_PLIST(profileState.visitedDepths, pos)) > depth) {
+        // Give dummy values if we do not know
+        fprintf(profileState.Stream,
+                "{\"Type\":\"O\",\"Fun\":\"nameless\",\"Line\":-1,"
+                "\"EndLine\":-1,\"File\":\"<missing filename>\","
+                "\"FileId\":-1}\n");
+        PopPlist(profileState.visitedDepths);
+        pos--;
+    }
+}
+
 static inline void outputFilenameIdIfRequired(UInt id)
 {
     if (id == 0) {
@@ -213,10 +258,24 @@ void HookedLineOutput(Obj func, char type)
 }
 
 void enterFunction(Obj func)
-{ HookedLineOutput(func, 'I'); }
+{
+    CheckLeaveFunctionsAfterLongjmp();
+    PushPlist(profileState.visitedDepths, INTOBJ_INT(GetRecursionDepth()));
+    HookedLineOutput(func, 'I');
+}
 
 void leaveFunction(Obj func)
-{ HookedLineOutput(func, 'O'); }
+{
+    // Do not crash if we exit the function in which
+    // Profile was originally called. The profiling
+    // package can handle such profiles.
+    if (LEN_PLIST(profileState.visitedDepths) > 0) {
+        PopPlist(profileState.visitedDepths);
+    }
+    CheckLeaveFunctionsAfterLongjmp();
+
+    HookedLineOutput(func, 'O');
+}
 
 /****************************************************************************
 **
@@ -306,6 +365,8 @@ static inline void outputStat(Stat stat, int exec, int visited)
   UInt line;
   int nameid;
 
+  CheckLeaveFunctionsAfterLongjmp();
+
   Int8 ticks = 0, newticks = 0;
 
   // Explicitly skip these two cases, as they are often specially handled
@@ -457,6 +518,7 @@ void enableAtStartup(char * filename, Int repeats, TickMethod tickMethod)
     ActivateHooks(&profileHooks);
 
     profileState_Active = 1;
+    RegisterSyLongjmpObserver(ProfileRegisterLongJmpOccurred);
     profileState.profiledPreviously = 1;
 #ifdef HPCGAP
     profileState.profiledThread = TLS(threadID);
@@ -518,7 +580,10 @@ Obj FuncACTIVATE_PROFILING(Obj self,
         return Fail;
     }
 
+    memset(&profileState, 0, sizeof(profileState));
+
     OutputtedFilenameList = NEW_PLIST(T_PLIST, 0);
+    profileState.visitedDepths = NEW_PLIST(T_PLIST, 0);
 
     if ( ! IsStringConv( filename ) ) {
         ErrorMayQuit("<filename> must be a string",0,0);
@@ -586,6 +651,7 @@ Obj FuncACTIVATE_PROFILING(Obj self,
     }
 
     profileState_Active = 1;
+    RegisterSyLongjmpObserver(ProfileRegisterLongJmpOccurred);
     profileState.profiledPreviously = 1;
 #ifdef HPCGAP
     profileState.profiledThread = TLS(threadID);
@@ -784,6 +850,7 @@ static Int InitLibrary (
     /* init filters and functions                                          */
     InitGVarFuncsFromTable( GVarFuncs );
 
+    profileState.visitedDepths = NEW_PLIST(T_PLIST, 0);
     OutputtedFilenameList = NEW_PLIST(T_PLIST, 0);
     /* return success                                                      */
     return 0;
@@ -798,6 +865,7 @@ static Int InitKernel (
 {
     InitHdlrFuncsFromTable( GVarFuncs );
     InitGlobalBag(&OutputtedFilenameList, "src/profile.c:OutputtedFileList");
+    InitGlobalBag(&profileState.visitedDepths, "src/profile.c:visitedDepths");
     return 0;
 }