Hi Goetz,

Just one follow up for now:

* Add package visible "void setMessage (String msg)" to Throwable.

Yes, just use package accessibility to deal with this, no need to jump through hoops (or the VM :) ).

Thanks,
David

On 8/02/2019 9:51 pm, Lindenmaier, Goetz wrote:
Hi David,

Hi Volker,
... I assume Volker could have contributed this as well, but actually
I must mention Ralf Schmelter as the original author of this :)
You know I'm not going to be a big fan of this :), but as long as we
don't pay for it if we don't want it, then that's okay. (I'm still
trying to gauge that)

I have a little test for this that I ran through your patch:

public class NPE {
    static class B {
      C b() { return null; }
    }
    static class C {
      int c(Object o, String s) {  return 0; }
    }

    public static void main(String[] args) {
      int x  = a().b().c(d(), e().toString());
    }

    static B a() { return null; }
    static Object d() { return null; }
    static Object e() { return null; }

}

and the result was a bit confusing for me:

java.lang.NullPointerException: while trying to invoke the method
NPE$B.b(()LNPE$C;) of a null object returned from NPE.a(()LNPE$B;)

The placement and format of the return type descriptors obfuscates
things to me - especially the Lxxx; format. Can we make that more Java
programmer friendly eg:

"while trying to invoke the method 'NPE$C NPE$B.b()' ..."

though I think trying to produce signatures within the message is going
to be very awkward in the general case. The key part is the "method
NPE.b() ... returned from NPE.a()"
Actually, I have left out code that changes the signatures to the
Java source code wording. I already left that out in my former
exception message contributions.  For example see the messages in
jtreg/runtime/exceptionMsgs/IllegalAccessError/IllegalAccessErrorTest.java,
they have the same bad format:
"class test.Runner4 tried to access private method 
test.IllegalAccessErrorTest.iae4_m()V"

I would like to fix them altogether in a follow-up, is that acceptable to
you?
Also "of a null object" would read better as "on a null reference".
Makes sense, fixed.

But I'm not that sure about changing these:
"while trying to read the field '%s' of a null object"
--> "while trying to read the field '%s' from a null reference"
"while trying to write the field %s of a null object"
--> "while trying to write the field %s  of a null reference"

First you will need to file a CSR request for the new product flags.
I'm not sure whether I need the product flags altogether. I would
prefer removing them.
Second, I don't understand why you need to call into the VM with
JVM_SetDefaultMessage, to set a field in the Java object? Why isn't that
done in Java?
Obviously, the problem is that the field is private.
As Christoph points out, there are several ways to implement this.
Please give advice:
   * reflection
   * shared secret
   * Add package visible "void setMessage (String msg)" to Throwable.

Best regards,
   Goetz



Thanks,
David

On 8/02/2019 2:43 am, Lindenmaier, Goetz wrote:
Hi,

since Java 5, our internal VM reports verbose null pointer exception
messages. I would like to contribute this feature to OpenJDK.

With this change, messages as
     "java.lang.NullPointerException: while trying to load from a null int array
loaded from local variable 'ia1'"
are printed.  For more examples see the JBS bug or the test included.
https://bugs.openjdk.java.net/browse/JDK-8218628

The messages are generated by parsing the bytecodes. For not to have any
overhead when the
NPE is allocated, the message is only generated when it is accessed by
getMessage() or
serialization. For this I added a field to NPE to indicate that the message 
still
needs to be
computed lazily.

Please review:
http://cr.openjdk.java.net/~goetz/wr19/8218628-exMsg-NPE/01/
I'm happy to incorporate your comments.

Best regards,
    Goetz


Reply via email to