Re: Updated throwable and stacktrace printing patch

[Mark Wielaard <mark at klomp dot org>]

Hi,

On Mon, 2002-08-19 at 16:28, Andrew Haley wrote:
> You didn't rebuild Makefile.in.  Or there is no mention of it in your
> patch.

Sorry, I thought the rebuild of Makefile.in was uninteresting.
Will include it next time.

> I'm trying a null pointer exception thrown with
> gnu.gcj.runtime.NameFinder.remove_unknown=false.
> 
> Two bugs:
> 
> java.lang.NullPointerException
>    at 0x4034a026 (Unknown Source)
> 
> Unknown?  It's catch_segv() in libgcj.so.  I'm pretty sure this used
> to work, but perhaps this is a bug in dladdr().

Hmmm. It shows up when I compile with -static but not with shared
libraries. I will look into this. But I was actually planning on
removing the catch_segv/fpe() calls from the user visual stack trace.
See below for design discussion.

>    at 0xffffffffbfffe504 (Unknown Source)
> 
> There's a sign extension bug here.

Yuck. Will follow your advise below to fix this.

>    at Throw_2.main(java.lang.String[]) (/cuddles/aph/gcc.clean/gcc.java-inliner/libjava/testsuite/libjava.lang/Throw_2.java:47)

I tested the Throw_X classes from the testsuite. How precisely did this
one fail for you?

> +jlong
> +gnu::gcj::runtime::NameFinder::getAddr(RawData* addrs, jint n)
> +{
> +  void **p = (void **) addrs;
> +  return (jlong) p[n];
> +}
> 
> It is not legal C++ to convert a pointer to a signed long by means of
> a cast.  Please convert the RawData* to hex and return the hex string
> instead.  A correct way to do this is the "*p" sprintf option, or by
> using a Pmode int and doing it by hand like name-finder.cc did.  This
> would also fix the sign extension bug.

Thanks. I will rewrite that part.

> We no longer indicate that we have thrown an exception that was caused
> by a signal.  It seems that sanitize removes _Jv_ThrowSignal() so you
> can't tell what happened.  This is unfortunate.  In the future I
> intend to decode signal handler info so that you can see where the
> null pointer access occurred.
> You could just print <signal>.

I wrote the code assuming that we want our stacktraces to look just like
conventional VM stacktraces and I believe this patch does that very
nicely. But maybe I have to think outside the box to see what you are
proposing here. (My box does not really include C or C++, as you have
seen, my main programming experience is with logic/functional languages
and some java.)

When designing this piece of code I assumed that the user/programmer is
interested in the stacktrace produced by the code they wrote. And that
the programmer is not interested in how precisely the runtime
environment generates the exception, only in where the exception
occurred in the program they wrote. The same is true for the way the
runtime starts the actual entry point of the program (void main(String[]
args)). That is why the sanitize routine explicitly removes all that
"uninteresting" stuff from the top and the bottom of the stacktrace.

This of course assumes that the runtime is behaving normally as it
should according to the VM specification. For people that actively hack
on the runtime it makes sense to set the demangling, sanitizing, etc off
when they debug the runtime but this is not something a "normal"
programmer is interested in so they all default to true.
But it can all be turned off if needed.

What do you mean exactly when you say that you want to decode the signal
handler info? Is the fact that the null pointer or arithmetic exception
was internally processed by a signal handler really interesting to the
programmer? It seems like a runtime detail. Maybe you can send a small
piece of example code and tell how you think the stacktrace should be
printed?

> You could just print <signal>.

That is actually what happens now since I forgot about removing the
catch_segv/fpe() methods from the trace. But we can keep those if you
like. But first we have to find out why they don't show up when
compiling against shared libs.

> What's the point of the equal test?  Of course it would shorten the
> ouput for recursive calls but why is this special case necessary?

Are you talking about the equal test in Throwable.printStackTrace()?
That test is there for the case were you chain an exception inside a new
exception and then rethrow it. That is a common case which is also shown
in the example in the javadoc (internally you catch a SQLException but
you rethrow it chained in an IOException for example). In those cases
the exception and the chained exception have the bottom of their stack
trace in common so the code makes sure that part is printed only once.

Cheers,

Mark