[armedbear-devel] [PATCH] incorporating java stack frames into backtrace
When evaluating (ext:make-thread #'(lambda ())) you get an error "The value NIL is not of type STRING." -- this is a bug in ABCL itself, by the way, but that's not the matter of this mail. The concern of this mail is the backtrace that'd you get: Backtrace: 0: (INVOKE-DEBUGGER #<TYPE-ERROR {B1A201}>) 1: (MAKE-THREAD #<FUNCTION (LAMBDA ()) {15343C2}>) 2: (SYSTEM::%EVAL (MAKE-THREAD #'(LAMBDA NIL))) 3: (EVAL (MAKE-THREAD #'(LAMBDA NIL))) ... The only thing the backtrace reveales is that the error was signalled somewhere in MAKE-THREAD. With the patch of this mail, the backtrace will also contain the relevant Java stack frames for errors that were signalled from within ABCL's Java runtime: Backtrace: 0: (INVOKE-DEBUGGER #<TYPE-ERROR {B1A201}>) 1: org.armedbear.lisp.Lisp.error(Lisp.java:375) 2: org.armedbear.lisp.Lisp.type_error(Lisp.java:390) 3: org.armedbear.lisp.LispObject.getStringValue(LispObject.java:1061) 4: org.armedbear.lisp.LispThread.<init>(LispThread.java:111) 5: org.armedbear.lisp.LispThread.<init>(LispThread.java:39) 6: org.armedbear.lisp.LispThread$3.execute(LispThread.java:909) 7: org.armedbear.lisp.Primitive.execute(Primitive.java:108) 8: (MAKE-THREAD #<FUNCTION (LAMBDA ()) {15343C2}>) 9: (SYSTEM::%EVAL (MAKE-THREAD #'(LAMBDA NIL))) 10: (EVAL (MAKE-THREAD #'(LAMBDA NIL))) Another example: (LENGTH 12) Backtrace: 0: (INVOKE-DEBUGGER #<TYPE-ERROR {814AF5}>) 1: org.armedbear.lisp.Lisp.error(Lisp.java:375) 2: org.armedbear.lisp.Lisp.type_error(Lisp.java:390) 3: org.armedbear.lisp.LispObject.length(LispObject.java:403) 4: org.armedbear.lisp.LispObject.LENGTH(LispObject.java:410) 5: org.armedbear.lisp.Primitives$24.execute(Primitives.java:427) 6: (LENGTH 12) 7: (SYSTEM::%EVAL (LENGTH 12)) 8: (EVAL (LENGTH 12)) -T. -- Diese Nachricht wurde auf Viren und andere gefaerliche Inhalte untersucht und ist - aktuelle Virenscanner vorausgesetzt - sauber. Freebits E-Mail Virus Scanner
"Tobias C. Rittweiler" writes: The diff contained the following Index: src/org/armedbear/lisp/Lisp.java =================================================================== --- src/org/armedbear/lisp/Lisp.java (revision 12040) +++ src/org/armedbear/lisp/Lisp.java (working copy) @@ -94,6 +94,11 @@ PACKAGE_CL_USER.usePackage(PACKAGE_CL); PACKAGE_CL_USER.usePackage(PACKAGE_EXT); PACKAGE_CL_USER.usePackage(PACKAGE_JAVA); + PACKAGE_CL_USER.usePackage(PACKAGE_THREADS); + + // FIXME: The following packages all have to use EXT because + // of EXT:AUTOLOAD and autoload.lisp. + PACKAGE_SYS.addNickname("SYS"); PACKAGE_SYS.usePackage(PACKAGE_CL); PACKAGE_SYS.usePackage(PACKAGE_EXT); @@ -105,6 +110,7 @@ PACKAGE_TPL.usePackage(PACKAGE_EXT); PACKAGE_EXT.addNickname("EXT"); PACKAGE_EXT.usePackage(PACKAGE_CL); + // FIXME: To be removed. PACKAGE_EXT.usePackage(PACKAGE_THREADS); PACKAGE_JVM.usePackage(PACKAGE_CL); PACKAGE_JVM.usePackage(PACKAGE_EXT); @@ -118,6 +124,8 @@ PACKAGE_LISP.usePackage(PACKAGE_CL); PACKAGE_LISP.usePackage(PACKAGE_EXT); PACKAGE_LISP.usePackage(PACKAGE_SYS); + PACKAGE_THREADS.usePackage(PACKAGE_CL); + PACKAGE_THREADS.usePackage(PACKAGE_EXT); } catch (Throwable t) { Please remove these three hunks before applying the patch. They were from my previous patch about moving MAKE-THREAD-LOCK etc. from EXT to THREADS. You can easily remove these hunks by opening the patch file with Emacs, and then using M-k within a hunk. I won't resend the patch because it's so huge. -T. -- Diese Nachricht wurde auf Viren und andere gefaerliche Inhalte untersucht und ist - aktuelle Virenscanner vorausgesetzt - sauber. Freebits E-Mail Virus Scanner
"Tobias C. Rittweiler" writes:
With the patch of this mail, the backtrace will also contain the relevant Java stack frames for errors that were signalled from within ABCL's Java runtime:
Backtrace: 0: (INVOKE-DEBUGGER #<TYPE-ERROR {B1A201}>) 1: org.armedbear.lisp.Lisp.error(Lisp.java:375) 2: org.armedbear.lisp.Lisp.type_error(Lisp.java:390) 3: org.armedbear.lisp.LispObject.getStringValue(LispObject.java:1061) 4: org.armedbear.lisp.LispThread.<init>(LispThread.java:111) 5: org.armedbear.lisp.LispThread.<init>(LispThread.java:39) 6: org.armedbear.lisp.LispThread$3.execute(LispThread.java:909) 7: org.armedbear.lisp.Primitive.execute(Primitive.java:108) 8: (MAKE-THREAD #<FUNCTION (LAMBDA ()) {15343C2}>) 9: (SYSTEM::%EVAL (MAKE-THREAD #'(LAMBDA NIL))) 10: (EVAL (MAKE-THREAD #'(LAMBDA NIL)))
An updated patch (fixes off-by-one) can be found at http://common-lisp.net/~trittweiler/incorporate-java-frames-into-backtraces-... -T. -- Diese Nachricht wurde auf Viren und andere gefaerliche Inhalte untersucht und ist - aktuelle Virenscanner vorausgesetzt - sauber. Freebits E-Mail Virus Scanner
Attached is a revised patch incorporating Java stack frames in the backtrace against ABCL SVN HEAD. This revision: * solves the "off by one" error of Tobias's original patch in a way that is more robust to stack dumps from other sources (such as calling into ABCL when the top level interpreter has not been established) * restores the ability to inspect a mixed Java/Lisp backtrace via the ABCL inspector * utilizes the stack optimization introduced by Erik in [svn r12064] in using a link in the StackFrame itself The Java stack traces certainly give an ABCL implementer a lot more useful information to understand what is going on although it could be more complete with information on the local variables on the various Java frames. But I wonder how useful this would really be for the "end-user" who would be confused by the abstraction barrier ("Why am I getting reference to a Java stack in my Lisp call?"). Comments please on whether this functionality would be useful, and potential improvements. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson writes:
Attached is a revised patch incorporating Java stack frames in the backtrace against ABCL SVN HEAD.
This revision:
* solves the "off by one" error of Tobias's original patch in a way that is more robust to stack dumps from other sources (such as calling into ABCL when the top level interpreter has not been established)
* restores the ability to inspect a mixed Java/Lisp backtrace via the ABCL inspector
* utilizes the stack optimization introduced by Erik in [svn r12064] in using a link in the StackFrame itself
The Java stack traces certainly give an ABCL implementer a lot more useful information to understand what is going on although it could be more complete with information on the local variables on the various Java frames. But I wonder how useful this would really be for the "end-user" who would be confused by the abstraction barrier ("Why am I getting reference to a Java stack in my Lisp call?").
Comments please on whether this functionality would be useful, and potential improvements.
a) Why did you change SYS:BACKTRACE-AS-LIST to listify the frames? I deliberately changed it to return a list of StackFrame objects, so higher levels can decide whether they want to print java frames, or not. (Which addresses your concern about "abstraction barrier".) Perhaps because the name BACKTRACE-AS-LIST could be understand to do implicit listification? I could follow that line. In fact, I'd have liked to perform the following renamings, but opted out to change too many things with my patch: Rename BACKTRACE to PRINT-BACKTRACE Define BACKTRACE to return a list of StackFrames. Define BACKTRACE-AS-LIST to return a list of listified stack frames. b) I don't like that (frame-to-list #<JAVA-STACK-FRAME>) => ("class.meth(file.java:NN)") I think it should return ("class.meth" :file "file.java" :line NN) so higher levels can use that information. (For example `v' in SLDB.) c) PRINT-FRAME contains an IGNORE-ERRORS which purpose I don't understand. Mind you, you actually copied that from my patch, so it's all my fault---I just can't remember why I put it in there. Do you know? If so, please add a comment. -T.
On 7/30/09 12:41 PM, Tobias C. Rittweiler wrote: […]
Comments please on whether this functionality would be useful, and potential improvements.
a) Why did you change SYS:BACKTRACE-AS-LIST to listify the frames? I deliberately changed it to return a list of StackFrame objects, so higher levels can decide whether they want to print java frames, or not. (Which addresses your concern about "abstraction barrier".)
From what I recall this was to support the use of the :frame and :inspect methods in the top-level REPL as it worked before. One could select a frame in the backtrace via ":frame n" (where n was the frame you were interested in), and then issue an ":inspect *" to start walking through the frame. This should certainly be kept for the LispStackFrame but maybe not for the JavaStackFrame. I think I ran into one problem that the "~S" representation of these objects was pretty opaque (i.e. just "#<JAVA-STACK-FRAME>" and "#<LISP-STACK-FRAME"), but I am not quite sure of my reasoning right now. I'll revisit this in course of the renamings you suggest below.
Perhaps because the name BACKTRACE-AS-LIST could be understand to do implicit listification? I could follow that line. In fact, I'd have liked to perform the following renamings, but opted out to change too many things with my patch:
Rename BACKTRACE to PRINT-BACKTRACE
Define BACKTRACE to return a list of StackFrames.
Define BACKTRACE-AS-LIST to return a list of listified stack frames.
That sounds quite reasonable. Sure, it is a lot of renaming, but other than SLIME, I think our only customer is 'top-level.lisp', so as long as we preserve the behavior there we should be fine. I completely agree that calling our main backtrace function BACKTRACE-AS-LIST is wrong.
b) I don't like that
(frame-to-list #<JAVA-STACK-FRAME>) => ("class.meth(file.java:NN)")
I think it should return ("class.meth" :file "file.java" :line NN) so higher levels can use that information. (For example `v' in SLDB.)
Agreed. I had a version doing plist like things as well, which makes a lot more sense for tools further down the line. I was going to make the whole result a plist so as not to trip up future reorderings. Would you just have the first element of the list be a string?
c) PRINT-FRAME contains an IGNORE-ERRORS which purpose I don't understand. Mind you, you actually copied that from my patch, so it's all my fault---I just can't remember why I put it in there. Do you know? If so, please add a comment.
Nope, I thought *you* had the deep reason here, so I was just slaving away via cut and paste. I'll remove it in the next iteration. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson writes:
From what I recall this was to support the use of the :frame and :inspect methods in the top-level REPL as it worked before. One could select a frame in the backtrace via ":frame n" (where n was the frame you were interested in), and then issue an ":inspect *" to start walking through the frame. This should certainly be kept for the LispStackFrame but maybe not for the JavaStackFrame. I think I ran into one problem that the "~S" representation of these objects was pretty opaque (i.e. just "#<JAVA-STACK-FRAME>" and "#<LISP-STACK-FRAME"), but I am not quite sure of my reasoning right now. I'll revisit this in course of the renamings you suggest below.
I was thinking that the opaqueness of #<JAVA-STACK-FRAME> was just a temporary thing until introspection into Java objects would be added which was an obvious next thing to do for ABCL. (And which Alessio just did.)
b) I don't like that
(frame-to-list #<JAVA-STACK-FRAME>) => ("class.meth(file.java:NN)")
I think it should return ("class.meth" :file "file.java" :line NN) so higher levels can use that information. (For example `v' in SLDB.)
Agreed. I had a version doing plist like things as well, which makes a lot more sense for tools further down the line. I was going to make the whole result a plist so as not to trip up future reorderings. Would you just have the first element of the list be a string?
I do not understand the question. Could you provide examples, please? -T.
On 7/30/09 7:34 PM, Tobias C. Rittweiler wrote: […]
(frame-to-list #<JAVA-STACK-FRAME>) => ("class.meth(file.java:NN)")
I think it should return ("class.meth" :file "file.java" :line NN) so higher levels can use that information. (For example `v' in SLDB.)
Agreed. I had a version doing plist like things as well, which makes a lot more sense for tools further down the line. I was going to make the whole result a plist so as not to trip up future reorderings. Would you just have the first element of the list be a string?
I do not understand the question. Could you provide examples, please?
I meant that the list returned would be a proper plist, with a key for every value, such as (frame-to-list #<JAVA-STACK-FRAME>) => (:CLASS "classname" :METHOD "methodName" :LINE-NUMBER 343 :FILE "s.java" :NATIVE-METHOD t) -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson writes:
I meant that the list returned would be a proper plist, with a key for every value, such as
(frame-to-list #<JAVA-STACK-FRAME>) => (:CLASS "classname" :METHOD "methodName" :LINE-NUMBER 343 :FILE "s.java" :NATIVE-METHOD t)
I don't mind. I suggested ("class.method" ...) because that still looked somewhat like a call frame. Yours more general, though. -T.
The attached patches provide the latest version of Tobias' idea on incorporating the Java/Lisp stack traces into a common abstraction. This patch: 1. SYS:BACKTRACE is now the primary Lisp entry point to get a backtrace, returning a list of LispStackTrace and JavaStackTrace 2. Backtraces returned from the ':bt' command are now meaningfully inspected 3. The Java function backtrace() is now called printBacktrace(), whereas backtraceAsList() is now more simply known as backtrace() 4. There is a convenience function SYS::BACKTRACE-AS-LIST that returns a list of proper Lisp lists representing the backtrace. Not sure if I want to export this symbol. 5. Fixed previous bugs which sometimes caused the Java backtrace to become corrupted (off-by-ones and something else fugly that I never totally understood, but I went back to Tobias' second version of javaStackTrace() and haven't seen anything bad since). My only problem with my work in the current state is that if one uses SLIME to directly inspect the results of SYS:BACKTRACE (i.e. a list of LispStackTrace and JavaStackTrace objects), one gets messages that SLIME is dumping to CL:DESCRIBE because it doesn't know how to inspect the objects of this type. I thought all I had to implement to make a Java object that inherits from LispObject inspectable was to implement LispObject getParts(), but apparently there is more to it than that. -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
On Mon, Aug 3, 2009 at 3:52 PM, Mark Evenson <evenson@panix.com> wrote:
I thought all I had to implement to make a Java object that inherits from LispObject inspectable was to implement LispObject getParts(), but apparently there is more to it than that.
Could it be a problem of SLIME? Afaik the ABCL inspector uses both getParts() and getDescription(), but those are defined in LispObject, so a fallback is used if you don't override them.
I've just committed my version of Tobias' idea about including Java stack information in stack traces when relevant to the trunk. The associated work for SLIME has been committed to SLIME HEAD. From the commit message of [svn 12105][1]: Split StackFrame abstraction into Java and Lisp stack frames. From the original patch/idea from Tobias Rittweiler this introduces more information of primary interest to ABCL implementers such as when a form like (make-thread #'(lambda ())) is evaluated All users of EXT:BACKTRACE-AS-LIST should now use SYS:BACKTRACE, the results of which is a list of the new builtin classes JAVA_STACK_FRAME or LISP_STACK_FRAME. The methods SYS:FRAME-TO-STRING and SYS:FRAME-TO-LIST are defined to break these new objects into inspectable parts. As a convenience, there is a SYS:BACKTRACE-AS-LIST which calls SYS:FRAME-TO-LIST on each element of the computed backtrace. Refactorings have also occurred on the Java side: the misnamed LispThread.backtrace() is now LispThread.printBacktrace(). LispThread.backtraceAsList() is now LispThread.backtrace() as it is a shorter name, and more to the point. Java stack frames only appear after a call through Lisp.error(), which has only the top level as a restart as an option, so concerns about trashing the stack are not really relevant (an early concern of mine). [1]: http://trac.common-lisp.net/armedbear/changeset/12105 -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
2009/8/19 Mark Evenson <evenson@panix.com>:
I've just committed my version of Tobias' idea about including Java stack information in stack traces when relevant to the trunk. The associated work for SLIME has been committed to SLIME HEAD.
Why does it copy args when creating a LispStackFrame with a vararg constructor? We removed that copying because we didn't find any need for it.
On 8/19/09 5:08 PM, Ville Voutilainen wrote:
2009/8/19 Mark Evenson<evenson@panix.com>:
I've just committed my version of Tobias' idea about including Java stack information in stack traces when relevant to the trunk. The associated work for SLIME has been committed to SLIME HEAD.
Why does it copy args when creating a LispStackFrame with a vararg constructor? We removed that copying because we didn't find any need for it.
You're correct. Somehow I missed this in the many merges I have done since tracking this patch from Tobias. Copying the reference instead of the values restored in [svn 12106][1]. [1]: http://trac.common-lisp.net/armedbear/changeset/12106 -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson writes:
I've just committed my version of Tobias' idea about including Java stack information in stack traces when relevant to the trunk. The associated work for SLIME has been committed to SLIME HEAD.
I don't like obscuring its swank backend by the attempt to make it backwards-compatible. ABCL is so young that I think you can expect people to upgrade their ABCL version if they upgrade Slime right now. _Especially_ because the changes make working in Slime so much more pleasurable that people who want to use Slime should just upgrade to them. Just barf with an error if an ABCL < 0.16 tries to compile swank-abcl.lisp. Have I missed the release? -T.
On 8/19/09 5:32 PM, Tobias C. Rittweiler wrote:
Mark Evenson writes:
I've just committed my version of Tobias' idea about including Java stack information in stack traces when relevant to the trunk. The associated work for SLIME has been committed to SLIME HEAD.
I don't like obscuring its swank backend by the attempt to make it backwards-compatible. ABCL is so young that I think you can expect people to upgrade their ABCL version if they upgrade Slime right now.
_Especially_ because the changes make working in Slime so much more pleasurable that people who want to use Slime should just upgrade to them.
Just barf with an error if an ABCL< 0.16 tries to compile swank-abcl.lisp.
Have I missed the release?
No release yet, the version on svn trunk announces itself as "abcl-0.16.0-dev". Maybe I should have put "upcoming 0.16 release". When 0.16 comes out the door, and FreeBSD (and others?) updates its port, I'll remove the compatibility code. Right now we have to support at least abcl-0.15. Does that satisfy your concerns? I would like to keep SLIME HEAD working with ABCL HEAD, but maybe you wish to do this just with patches back and forth? -- "A screaming comes across the sky. It has happened before, but there is nothing to compare to it now."
Mark Evenson writes:
When 0.16 comes out the door, and FreeBSD (and others?) updates its port, I'll remove the compatibility code. Right now we have to support at least abcl-0.15. Does that satisfy your concerns? I would like to keep SLIME HEAD working with ABCL HEAD, but maybe you wish to do this just with patches back and forth?
Well, if you do the maintaining, I surely don't have a problem with that. For example, supporting up to the previous release sounds reasonably, but I think it could get unwiedly to keep track what changes can be got rid of with the time. In this instance, it's just that people who want to use Slime and ABCL should really upgrade ASAP. That said, please get the release out the door even in absence of Erik. It's a good thing to have more than one people who can roll out releases. :-) -T.
2009/8/19 Tobias C. Rittweiler <tcr@freebits.de>:
That said, please get the release out the door even in absence of Erik. It's a good thing to have more than one people who can roll out releases. :-)
Erik's back already, so I expect him to do the release.
participants (4)
-
Alessio Stalla -
Mark Evenson -
Tobias C. Rittweiler -
Ville Voutilainen