* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException [not found] <CABc96T_0THd_tKzdHSjD-xcPd7=dqoFkr0aRwrdvx-h4s2Hnxg@mail.gmail.com> @ 2011-08-07 11:00 ` Jie Liu 2011-08-08 14:16 ` Tom Tromey 2011-08-15 14:07 ` Andrew Haley 0 siblings, 2 replies; 11+ messages in thread From: Jie Liu @ 2011-08-07 11:00 UTC (permalink / raw) To: gcc-patches, java-patches Hi, When I use gcj on an RTOS(RTEMS), Double.parseDouble(null) throw NumberFormatException, but it should throw NullPointerException. So I add the patch below: Index: natVMDouble.cc =================================================================== --- natVMDouble.cc (revision 172224) +++ natVMDouble.cc (working copy) @@ -19,6 +19,7 @@ #include <java/lang/VMDouble.h> #include <java/lang/Character.h> #include <java/lang/NumberFormatException.h> +#include <java/lang/NullPointerException.h> #include <jvm.h> #include <stdio.h> @@ -162,6 +163,9 @@ jdouble java::lang::VMDouble::parseDouble(jstring str) { + if(str == NULL) + throw new NullPointerException(); + int length = str->length(); while (length > 0 The testsuite/Throw_2.java has been PASS after this patch. what do you think about this patch? Thanks, Jie ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-07 11:00 ` [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException Jie Liu @ 2011-08-08 14:16 ` Tom Tromey 2011-08-09 17:29 ` Jie Liu 2011-08-15 14:07 ` Andrew Haley 1 sibling, 1 reply; 11+ messages in thread From: Tom Tromey @ 2011-08-08 14:16 UTC (permalink / raw) To: Jie Liu; +Cc: gcc-patches, java-patches >>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: Jie> + if(str == NULL) Jie> + throw new NullPointerException(); Jie> + Jie> int length = str->length(); Why doesn't 'str->length()' throw the NPE? Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-08 14:16 ` Tom Tromey @ 2011-08-09 17:29 ` Jie Liu 2011-08-10 13:34 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Jie Liu @ 2011-08-09 17:29 UTC (permalink / raw) To: Tom Tromey, java-patches, gcc-patches, Joel Sherrill Hi Tom, RTEMS does not have virtual memory management, so there is no error when access the 0 address on rtems. So 'str->length()' donot throw NPE and just return an meaningless value. Ps: There is a test: http://code.google.com/p/rtemsgcj/source/browse/trunk/algorithm/20110810NPE/?r=127, npe.c is the main file and *.scn is the output. Thanks, Jie 2011/8/8 Tom Tromey <tromey@redhat.com>: >>>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: > > Jie> + if(str == NULL) > Jie> + throw new NullPointerException(); > Jie> + > Jie> int length = str->length(); > > Why doesn't 'str->length()' throw the NPE? > > Tom > =========== The first mail, for adding Joel =========== Hi, When I use gcj on an RTOS(RTEMS), Double.parseDouble(null) throw NumberFormatException, but it should throw NullPointerException. So I add the patch below: Index: natVMDouble.cc =================================================================== --- natVMDouble.cc (revision 172224) +++ natVMDouble.cc (working copy) @@ -19,6 +19,7 @@ #include <java/lang/VMDouble.h> #include <java/lang/Character.h> #include <java/lang/NumberFormatException.h> +#include <java/lang/NullPointerException.h> #include <jvm.h> #include <stdio.h> @@ -162,6 +163,9 @@ jdouble java::lang::VMDouble::parseDouble(jstring str) { + if(str == NULL) + throw new NullPointerException(); + int length = str->length(); while (length > 0 The testsuite/Throw_2.java has been PASS after this patch. what do you think about this patch? Thanks, Jie ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-09 17:29 ` Jie Liu @ 2011-08-10 13:34 ` Tom Tromey 2011-08-10 15:59 ` Jie Liu 0 siblings, 1 reply; 11+ messages in thread From: Tom Tromey @ 2011-08-10 13:34 UTC (permalink / raw) To: Jie Liu; +Cc: java-patches, gcc-patches, Joel Sherrill >>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: Jie> RTEMS does not have virtual memory management, so there is no error Jie> when access the 0 address on rtems. Jie> So 'str->length()' donot throw NPE and just return an meaningless value. If you compile the Java parts of the library with -fcheck-references, it should work. This is something you have to set up as part of the port, as it is decided at (libgcj-) configure time. This won't help with the native code. IIRC in the end we just gave up on that; if you wanted real correctness you would have to add a null check at every dereference in the C++ code. I believe Andrew had a g++ patch to do this in the compiler, but it was rejected. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-10 13:34 ` Tom Tromey @ 2011-08-10 15:59 ` Jie Liu 2011-08-10 16:41 ` Tom Tromey 0 siblings, 1 reply; 11+ messages in thread From: Jie Liu @ 2011-08-10 15:59 UTC (permalink / raw) To: Tom Tromey; +Cc: java-patches, gcc-patches, Joel Sherrill 2011/8/10 Tom Tromey <tromey@redhat.com>: >>>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: > > Jie> RTEMS does not have virtual memory management, so there is no error > Jie> when access the 0 address on rtems. > Jie> So 'str->length()' donot throw NPE and just return an meaningless value. > > If you compile the Java parts of the library with -fcheck-references, it > should work. This is something you have to set up as part of the port, > as it is decided at (libgcj-) configure time. > Thank you very much for the information you provide. :) I add the following patch for -fcheck-references: Index: configure.host =================================================================== --- configure.host (revision 172224) +++ configure.host (working copy) @@ -347,8 +347,14 @@ slow_pthread_self= can_unwind_signal=yes ;; + *-*-rtems*) + can_unwind_signal=no + CHECKREFSPEC=-fcheck-references + DIVIDESPEC=-fuse-divide-subroutine + ;; esac + case "${host}" in *-cygwin* | *-mingw*) fallback_backtrace_h=sysdep/i386/backtrace.h And we can see the following have -fcheck-references: /mnt/gcj/mytoolchain/libexec/gcc/i386-rtems/4.7.0/jc1 /tmp/ccYuDgD5.jar -fsource-filename=HelloWorld.java -fhash-synchronization -fuse-divide-subroutine -fcheck-references -fuse-boehm-gc -fkeep-inline-functions -quiet -dumpbase HelloWorld.java -mtune=i386 -march=i386 -auxbase HelloWorld -g -g -Wall -version -fsaw-java-file -fbootclasspath=./:/usr/share/java/ecj.jar:/mnt/gcj/mytoolchain/share/java/libgcj-4.7.0.jar -faux-classpath /tmp/cc6nFx0I.zip -o /tmp/ccCuf7ju.s GNU Java (GCC) version 4.7.0 20110409 (experimental) (i386-rtems) But it does not work as we want, is there something wrong? Thanks, Jie > This won't help with the native code. IIRC in the end we just gave up > on that; if you wanted real correctness you would have to add a null > check at every dereference in the C++ code. I believe Andrew had a g++ > patch to do this in the compiler, but it was rejected. > > Tom > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-10 15:59 ` Jie Liu @ 2011-08-10 16:41 ` Tom Tromey 2011-08-12 7:39 ` Jie Liu 2011-08-12 17:12 ` Jie Liu 0 siblings, 2 replies; 11+ messages in thread From: Tom Tromey @ 2011-08-10 16:41 UTC (permalink / raw) To: Jie Liu; +Cc: java-patches, gcc-patches, Joel Sherrill >>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: Jie> + *-*-rtems*) Jie> + can_unwind_signal=no Jie> + CHECKREFSPEC=-fcheck-references Jie> + DIVIDESPEC=-fuse-divide-subroutine Jie> + ;; This part is OK with a ChangeLog entry. Jie> + Spurious newline addition. Jie> But it does not work as we want, is there something wrong? Did you rebuild all of libgcj? If you did, then I don't know, you'll have to debug it, sorry. I vaguely recollect that -fcheck-references adds a check for 'this' at the start of final methods. If I'm misremembering, then that is probably the problem. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-10 16:41 ` Tom Tromey @ 2011-08-12 7:39 ` Jie Liu 2011-08-12 16:22 ` Bryce McKinlay 2011-08-12 17:12 ` Jie Liu 1 sibling, 1 reply; 11+ messages in thread From: Jie Liu @ 2011-08-12 7:39 UTC (permalink / raw) To: Tom Tromey; +Cc: java-patches, gcc-patches, Joel Sherrill > Jie> But it does not work as we want, is there something wrong? > > Did you rebuild all of libgcj? Yes. :) > > If you did, then I don't know, you'll have to debug it, sorry. > > I vaguely recollect that -fcheck-references adds a check for 'this' at > the start of final methods. If I'm misremembering, then that is > probably the problem. > The method length() is not a final method, as java/lang/String.java line 447: public int length() { return count; } Is this the problem? Thanks, Jie > Tom > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-12 7:39 ` Jie Liu @ 2011-08-12 16:22 ` Bryce McKinlay 0 siblings, 0 replies; 11+ messages in thread From: Bryce McKinlay @ 2011-08-12 16:22 UTC (permalink / raw) To: Jie Liu; +Cc: Tom Tromey, java-patches, gcc-patches, Joel Sherrill On Fri, Aug 12, 2011 at 8:27 AM, Jie Liu <lj8175@gmail.com> wrote: > The method length() is not a final method, as java/lang/String.java line 447: > > public int length() > { > return count; > } > > Is this the problem? As String is a final class, all its methods are implicitly final. Bryce ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-10 16:41 ` Tom Tromey 2011-08-12 7:39 ` Jie Liu @ 2011-08-12 17:12 ` Jie Liu 2011-08-14 13:04 ` Tom Tromey 1 sibling, 1 reply; 11+ messages in thread From: Jie Liu @ 2011-08-12 17:12 UTC (permalink / raw) To: Tom Tromey; +Cc: java-patches, gcc-patches, Joel Sherrill > If you did, then I don't know, you'll have to debug it, sorry. > > I vaguely recollect that -fcheck-references adds a check for 'this' at > the start of final methods. If I'm misremembering, then that is > probably the problem. In my debug, there appears no check for 'this' at start of String.length(): (gdb) s java::lang::VMDouble::parseDouble (str=0x0) at ../../../gcc-trunk/libjava/java/lang/natVMDouble.cc:165 165 int length = str->length(); (gdb) s java.lang.String.length()int (this=@336e44) at /mnt/gcj/gcj-rtems-work/gcc-trunk/libjava/java/lang/String.java:451 451 return count; (gdb) p count $5 = -268370093 (gdb) p this $6 = (java.lang.String *) 0x0 Thanks, Jie ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-12 17:12 ` Jie Liu @ 2011-08-14 13:04 ` Tom Tromey 0 siblings, 0 replies; 11+ messages in thread From: Tom Tromey @ 2011-08-14 13:04 UTC (permalink / raw) To: Jie Liu; +Cc: java-patches, gcc-patches, Joel Sherrill >>>>> "Jie" == Jie Liu <lj8175@gmail.com> writes: Jie> In my debug, there appears no check for 'this' at start of String.length(): Yeah, I looked for uses of flag_check_references and didn't see one when building a method's body. So I guess I mis-remembered this. In any case, the spot you found is just the tip of the problem. Any method call in any of the C++ code could potentially have the same issue. Tom ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException 2011-08-07 11:00 ` [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException Jie Liu 2011-08-08 14:16 ` Tom Tromey @ 2011-08-15 14:07 ` Andrew Haley 1 sibling, 0 replies; 11+ messages in thread From: Andrew Haley @ 2011-08-15 14:07 UTC (permalink / raw) To: java-patches On 08/07/2011 12:00 PM, Jie Liu wrote: > When I use gcj on an RTOS(RTEMS), Double.parseDouble(null) throw > NumberFormatException, but it should throw NullPointerException. So I > add the patch below: > > Index: natVMDouble.cc > =================================================================== > --- natVMDouble.cc (revision 172224) > +++ natVMDouble.cc (working copy) > @@ -19,6 +19,7 @@ > #include <java/lang/VMDouble.h> > #include <java/lang/Character.h> > #include <java/lang/NumberFormatException.h> > +#include <java/lang/NullPointerException.h> > #include <jvm.h> > > #include <stdio.h> > @@ -162,6 +163,9 @@ > jdouble > java::lang::VMDouble::parseDouble(jstring str) > { > + if(str == NULL) > + throw new NullPointerException(); > + > int length = str->length(); > > while (length > 0 > > The testsuite/Throw_2.java has been PASS after this patch. what do you > think about this patch? This patch is not OK, and must not go in. Look at the comment at the top of Throw_2: // Check that NullPointerExceptions thrown from library code are // caught. This detects a number of failures that can be caused by // libgcj being built incorrectly. In particular, we ensure that a // SEGV in native (i.e. C++) code in libgcj is handled correctly. // Regrettably, we cannot guarantee that Double.parseDouble() will // always be native code, or that it will never be inlined. It could // be argued that we should add a method to libgcj that will be // guaranteed forever to be native, but I'm reluctant to add to the // library for the sole purpose of performing this test. Andrew. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2011-08-15 9:48 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <CABc96T_0THd_tKzdHSjD-xcPd7=dqoFkr0aRwrdvx-h4s2Hnxg@mail.gmail.com> 2011-08-07 11:00 ` [PATCH] [JAVA] Double.parseDouble(null) throw NullPointerException Jie Liu 2011-08-08 14:16 ` Tom Tromey 2011-08-09 17:29 ` Jie Liu 2011-08-10 13:34 ` Tom Tromey 2011-08-10 15:59 ` Jie Liu 2011-08-10 16:41 ` Tom Tromey 2011-08-12 7:39 ` Jie Liu 2011-08-12 16:22 ` Bryce McKinlay 2011-08-12 17:12 ` Jie Liu 2011-08-14 13:04 ` Tom Tromey 2011-08-15 14:07 ` Andrew Haley
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).