One thing you've missed is that there are situations where there is no
spawning activation. For example, from native code, I can attach a thread,
create a message object and send it a START message. In that situation,
there is no activation to create a stack frame from. Also, the stack frame
in question might be a native activation, so you need to make sure that
these function as well.
Rick
On Sun, Aug 18, 2024 at 8:29 AM Rony G. Flatscher <rony.flatsc...@wu.ac.at>
wrote:
> With the introduction of TraceObject it becomes possible to create and
> store trace logs for further analysis. In March, I started to create a
> little tracetool.rex/tracutil.cls to help the Rexx programmers analyze
> their code. Testing and analyzing what can be inferred, it turned out that
> in the case of a started message and a reply statement, no information
> about the caller/invoker is currently available, making it very difficult,
> if not impossible, at times to find out which instruction caused the
> creation of a new activity.
>
> In the discussions on the developer list, it became clear that storing a
> StackFrame object itself may cause crashes if the activity for which it was
> created has gone away if I understood the problem correctly. Therefore,
> TraceObject would not store the StackFrame object but rather create a
> StringTable receiving the relevant StackFrame information such that
> accessing the StringTable later would not cause any problems.
>
> This information gets only added to TraceObject if a
> TRACE_PREFIX_INVOCATION is active (tracing the invocation of a routine or
> method).
> Looking around in the source code for ways to safely make the caller's
> stackframe information available in the case that a new activity gets
> spawned as a result of a start message or a reply keyword statement, the
> following idea would be proposed, hoping that it is safe and does not
> impact the interpreter unduly:
>
> -
>
> define a method Activity::generateCallerStackFrame(bool skipFirst)
> -
>
> this returns the caller's StackFrame
> -
>
> define for Activity a field "StringTable *spawnerStackFrameInfo"
> -
>
> this is used to store a StringTable with the caller's StackFrame
> information
> -
>
> define a method Activity::setCallerStackFrameAsStringTable(Activity
> *oldActivity, Activity *newActivity, bool skipFirst)
> -
>
> o this method gets only invoked in "Method::start()" and in the
> reply section of "RexxActivation::run(...)", which are the two cases
> where
> a new activity gets spawned to execute the message in the case of
> "Method:start()" and the remaining instructions in the case of a reply
> keyword instruction
> -
>
> this method queries the caller's stackframe using
> Activity::generateCallerStackFrame(skipFirst) and using
> "RexxActivation::getStackFrameAsStringTable(StackFrameClass *
> stackFrame)"
> returning a StringTable with the stackframe information and storing it
> with
> "spawnerStackFrameInfo" field
> -
>
> in addition, it will get an entry THREAD with the thread ID added,
> such that a) the correct invocation can be located (the same location
> could
> be used over and over from different threads; it is important to become
> able to identify which activity/thread was used) and b) one can
> distinguish
> with the callerStackFrame in TraceObject whether it got created
>
> An experimental implementation seems to work, and so far, no adverse
> behavior has been observed.
>
> Here the diff:
>
> *Index: interpreter/classes/MessageClass.cpp*
> ===================================================================
> --- interpreter/classes/MessageClass.cpp (revision 12859)
> +++ interpreter/classes/MessageClass.cpp (working copy)
> @@ -537,6 +537,10 @@
> // spawn a new activity off of the old activity
> Activity *oldActivity = ActivityManager::currentActivity;
> Activity *newActivity = oldActivity->spawnReply();+
> + // save current frame info in a StringTable as it has caused the
> creation of a new activity
> + Activity::setCallerStackFrameAsStringTable(oldActivity, newActivity,
> true); // create stackframe from parent/caller
> +
> // mark which activity we're running on, then dispatch the message
> // on the new activity (which is sitting waiting for work to perform)
> setField(startActivity, newActivity);*Index:
> interpreter/concurrency/Activity.cpp*
> ===================================================================
> --- interpreter/concurrency/Activity.cpp (revision 12859)
> +++ interpreter/concurrency/Activity.cpp (working copy)
> @@ -1159,31 +1159,54 @@
> }
>
>
> +
> /**
> - * Generates a StackFrame from the parent and returns it.+ * Generate the
> caller's stack frame.
> *
> - * @return parent's StackFrame or NULLOBJECT, if no parent exists+ * @param
> skipFirst Determines if we should skip the first frame:
> + * false for reply in RexxActivation::run(...),
> + * true for Message::start() and in the general case.
> + *
> + * @return The stackframe of the caller which caused a spawned activity.
> */
> -StackFrameClass* Activity::generateParentStackFrame()+StackFrameClass*
> Activity::generateCallerStackFrame(bool skipFirst)
> {
> - // create lists for both the stack frames and the traceback lines
> - StackFrameClass *parentStackFrame = NULLOBJECT;
> -
> ActivationFrame *frame = activationFrames;+ StackFrameClass
> *callerStackFrame = NULL;
>
> - if (frame != NULL)+ if (frame && skipFirst)
> {
> - frame = frame->next; // get parent
> - if (frame != NULL)
> - {
> - parentStackFrame = frame->createStackFrame();
> - }+ frame = frame->next;
> }
> - return parentStackFrame;+ if (frame)
> + {
> + callerStackFrame = frame->createStackFrame();
> + }
> + return callerStackFrame;
> }
>
>
> /**+ * Generates and saves a StringTable to store stackFrame infos of
> oldActivity in newActivity
> + * (for TraceObject), used by MessageClass::start().
> + *
> + */
> +void Activity::setCallerStackFrameAsStringTable(Activity *oldActivity,
> Activity *newActivity, bool skipFirst)
> +{
> + // create lists for both the stack frames and the traceback lines
> + StringTable *spawnerStackFrameInfo = NULLOBJECT;
> +
> + StackFrameClass *oldActivityStackFrame = oldActivity ->
> generateCallerStackFrame(skipFirst);
> + // returns a StringTable that may be empty if
> oldActivityStackFrame==NULLOBJECT
> + newActivity ->
> spawnerStackFrameInfo=RexxActivation::getStackFrameAsStringTable(oldActivityStackFrame);
> + // save thread ID to indicate from which thread ID the spawnReply()
> came from (helpful, if StringTable is empty)
> + newActivity -> spawnerStackFrameInfo -> put(new_integer(oldActivity ->
> getIdntfr()), GlobalNames::THREAD );
> + return;
> +}
> +
> +
> +
> +/**
> * Build a message and perform the indicated substitutions.
> *
> * @param messageCode*Index: interpreter/concurrency/Activity.hpp*
> ===================================================================
> --- interpreter/concurrency/Activity.hpp (revision 12859)
> +++ interpreter/concurrency/Activity.hpp (working copy)
> @@ -159,8 +159,12 @@
> void unwindToFrame(RexxActivation *frame);
> void cleanupStackFrame(ActivationBase *poppedStackFrame);
> ArrayClass *generateStackFrames(bool skipFirst);
> - StackFrameClass *generateParentStackFrame();
> + // allow TraceObject's callerStackFrame entry to indicate the caller
> that caused the spawned activity
> + StackFrameClass* Activity::generateCallerStackFrame(bool );
> + static void Activity::setCallerStackFrameAsStringTable(Activity
> *oldActivity, Activity *newActivity, bool);
> + StringTable *spawnerStackFrameInfo;
> +
> Activity *spawnReply();
>
> void exitKernel();*Index:
> interpreter/execution/RexxActivation.cpp*
> ===================================================================
> --- interpreter/execution/RexxActivation.cpp (revision 12859)
> +++ interpreter/execution/RexxActivation.cpp (working copy)
> @@ -702,6 +702,9 @@
>
> activity = oldActivity->spawnReply();
> + // save current frame info in a StringTable as it has
> caused the creation of a new activity
> + Activity::setCallerStackFrameAsStringTable(oldActivity,
> activity, false);
> +
> // save the pointer to the start of our stack frame. We're
> // going to need to release this after we migrate everything
> // over.
> @@ -5157,8 +5160,23 @@
> }
> else if (tracePrefix == TRACE_PREFIX_INVOCATION) // tracing an
> invocation entry
> {
> - StackFrameClass *stackFrame = activity -> generateParentStackFrame();
> - traceObject -> put(stackFrame ?
> getStackFrameAsStringTable(stackFrame) : TheNilObject,
> GlobalNames::CALLERSTACKFRAME);+ StackFrameClass *stackFrame =
> activity -> generateCallerStackFrame(true);
> + if (stackFrame)
> + {
> + traceObject -> put(getStackFrameAsStringTable(stackFrame),
> GlobalNames::CALLERSTACKFRAME);
> + }
> + else
> + {
> + if (activity -> spawnerStackFrameInfo) // spawner's frame infos
> saved as a StringTable?
> + {
> + // allows for analyzing trace logs to identify the caller of
> a spawned activity
> + traceObject -> put(activity -> spawnerStackFrameInfo,
> GlobalNames::CALLERSTACKFRAME);
> + }
> + else
> + {
> + traceObject -> put(TheNilObject,
> GlobalNames::CALLERSTACKFRAME);
> + }
> + }
> }
>
> // METHODCALL, fill in method related information
> @@ -5232,26 +5250,28 @@
> * the identityHash of the StackFrame entries EXECUTABLE and TARGET if they
> are not
> * .nil.
> *
> -* @param stackFrame the StackFrame object to use
> -* @return a StringTable with all the entries of stackFrame+* @param
> stackFrame the StackFrame object to use, may be NULLOBJECT
> +* @return a StringTable with all the entries of stackFrame, if available
> */
> StringTable * RexxActivation::getStackFrameAsStringTable(StackFrameClass *
> stackFrame)
> {
> - ProtectedObject result;
> StringTable *tmpStringTable = new_string_table();
> - // array
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::ARGUMENTS ,
> result), GlobalNames::ARGUMENTS );+ if (stackFrame != NULLOBJECT)
> + {
> + ProtectedObject result;
> + // array
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::ARGUMENTS
> , result), GlobalNames::ARGUMENTS );
> - // strings
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::LINE ,
> result), GlobalNames::LINE );
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::NAME ,
> result), GlobalNames::NAME );
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TRACELINE ,
> result), GlobalNames::TRACELINE );
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TYPE ,
> result), GlobalNames::TYPE );+ // strings
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::LINE
> , result), GlobalNames::LINE );
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::NAME
> , result), GlobalNames::NAME );
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TRACELINE
> , result), GlobalNames::TRACELINE );
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TYPE
> , result), GlobalNames::TYPE );
>
> - // objects
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::EXECUTABLE,
> result), GlobalNames::EXECUTABLE);
> - tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TARGET ,
> result), GlobalNames::TARGET );
> -+ // objects
> + tmpStringTable ->
> put(stackFrame->sendMessage(GlobalNames::EXECUTABLE, result),
> GlobalNames::EXECUTABLE);
> + tmpStringTable -> put(stackFrame->sendMessage(GlobalNames::TARGET
> , result), GlobalNames::TARGET );
> + }
> return tmpStringTable;
> }
>
>
> To get at the patch directly, I created a RFE with the above patch at
> <https://sourceforge.net/p/oorexx/feature-requests/842/>
> <https://sourceforge.net/p/oorexx/feature-requests/842/>.
>
> Any comments, suggestions?
>
> ---rony
>
>
> _______________________________________________
> Oorexx-devel mailing list
> Oorexx-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/oorexx-devel
>
_______________________________________________
Oorexx-devel mailing list
Oorexx-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/oorexx-devel
