Assertions and System Assertions in GNU Classpath

[Josef Eisl-2]

Hi.

Attached is a patch that enables GNU Classpath to distinguish between
normal assertions and assertions for system classes (i.e. null
classloader) as discussed on the Classpath mailing list [1].

Note that the new method
'java/lang/VMClassLoader.getSystemAssertionStatus()' must be implemented
otherwise an exception will occur.

josef

[1]:
http://developer.classpath.org/pipermail/classpath/2012-April/003195.html

On 04/24/2012 02:29 AM, Andrew Hughes wrote:

> ----- Original Message -----
>> Hi there.
>>
>> I want to reanimate a discussion from 2007 [1,2] about the
>> Assertion/System Assertion handling. As far as I know GNU Classpath
>> can't distinguish between normal assertions (-ea/-eanableassertions)
>> and
>> assertions for system classes (-esa/-enablesystemassertions, i.e.
>> classes with no classloader [3]).
>> The only interface for the VM is the method
>> VMClassLoader.defaultAssertionStatus(). It is not clear if this is
>> supposed to return the system assertion status or the normal
>> assertion
>> status (vm integration doc [4] vs. javadoc [5]). On the one hand, it
>> is
>> used to set the defaultAssertionStatus for java.lang.ClassLoader [6]
>> which would imply that it is the normal assertion status. On the
>> other
>> hand, the very same flag is used for setting the assertion status for
>> system class in java.lang.Class (desiredAssertionStatus)[7].
>>
>> I think we need two dedicated methods in VMClassLoader.
>> getDefaultAssertionStatus() for normal assertions (i.e. -ea) and
>> getDefaultSystemAssertionStatus() for system assertions (i.e. -esa).
>> ClassLoader should use VMClassLoader.getDefaultAssertionStatus() as
>> its
>> default value and Class.getDesiredAssertionStatus() should use
>> VMClassLoader.defaultSystemAssertionStatus() for system classes.
>>
>> I understand that this would be a rather big change because all VMs
>> using GNU Classpath will fail if they do not adopt their
>> VMClassLoader
>> accordingly.
>>
>> I want to revitalize the discussion on this topic and I'm ready to
>> prepare a patch for this issue but things should be discussed first
>> ;).
>>
>> I hope my explanations are somehow understandable ;).
>>
>> - josef
>>
>> PS: I think the patch from 2007 is does not fix this issue as it does
>> not modify java.lang.Class and java.lang.Classloader receptively.
>>
>>
>> [1]:
>> http://developer.classpath.org/pipermail/classpath-patches/2007-August/005601.html
>> [2]:
>> http://developer.classpath.org/pipermail/classpath-patches/2007-September/005605.html
>> [3]:
>> http://docs.oracle.com/javase/1.4.2/docs/guide/lang/assert.html#enable-disable
>> [4]:
>> http://www.gnu.org/software/classpath/docs/cp-vmintegration.html#SEC7
>> [5]:
>> http://git.savannah.gnu.org/cgit/classpath.git/tree/vm/reference/java/lang/VMClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n334
>> [6]:
>> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/ClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n220
>> [7]:
>> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/Class.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n1233
>>
>>
>
> You're right.  The issue is here:
>
> "But if you use them with no arguments (-ea or -da), they do not apply to system classes."
>
> (from http://java.sun.com/developer/technicalArticles/JavaLP/assertions/)
>
> Both when the classloader is null (a system class) and when there is no class or package-specific setting, the same default setting
> from VMClassLoader.defaultAssertionStatus() is used.
>
> There should instead a separate method for system classes as you mention: VMClassLoader.getSystemAssertionStatus()
> (I don't think it needs to be called default because it can't be overridden by the class/package settings).
>
> As we're just after a release (0.99), this would be an ideal time to add that method.

[system-assertion.patch]

---

From b3beaa47685cd75e25d5d35a483618708c7dc132 Mon Sep 17 00:00:00 2001
From: Josef Eisl <zapster@...>
Date: Tue, 24 Apr 2012 17:21:51 +0200
Subject: [PATCH] Added java/lang/VMClassLoader.getSystemAssertionStatus().

This patch adds the possibility to distinguish between
assertions for normal classes and assertion for system
classes (i.e. classes with null ClassLoader).
---
 ChangeLog                                 |    6 ++++++
 doc/cp-vmintegration.texinfo              |    3 +++
 java/lang/Class.java                      |   14 +++++++++++---
 vm/reference/java/lang/VMClassLoader.java |   16 ++++++++++++++--
 4 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 062298b..d733367 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
[phone]  Josef Eisl  <zapster@...>
+
+ * vm/reference/java/lang/VMClassLoader.java: Added getSystemAssertionStatus().
+ * java/lang/Class.java: Using getSystemAssertionStatus().
+ * doc/cp-vmintegration.texinfo: Updated documentation.
+
 2012-04-03  Andrew John Hughes  <ahughes@...>
 
  * .gitignore: Renamed from .cvsignore.
diff --git a/doc/cp-vmintegration.texinfo b/doc/cp-vmintegration.texinfo
index 0b2d78c..672a262 100644
--- a/doc/cp-vmintegration.texinfo
+++ b/doc/cp-vmintegration.texinfo
@@ -414,6 +414,9 @@ exists (returning an empty array) which may be replaced if support is
 required.
 @item @code{defaultAssertionStatus()} -- A stub which can be implemented
 by VMs providing assertion support.  At present, it always returns @code{true}.
+@item @code{getSystemAssertionStatus()} -- A stub which can be implemented
+by VMs providing support for system assertions.  At present, it always returns
+@code{true}.
 @item @code{packageAssertionStatus()} -- Much the same status as the above.
 The method should return a map converting package names to boolean status
 values.  The stub implementation provides an empty map.
diff --git a/java/lang/Class.java b/java/lang/Class.java
index af0a0a2..b7059cc 100644
--- a/java/lang/Class.java
+++ b/java/lang/Class.java
@@ -1,5 +1,5 @@
 /* Class.java -- Representation of a Java class.
-   Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007
+   Copyright (C) 1998, 1999, 2000, 2002, 2003, 2004, 2005, 2006, 2007, 2012
    Free Software Foundation
 
 This file is part of GNU Classpath.
@@ -1229,8 +1229,16 @@ public final class Class<T>
   {
     ClassLoader c = getClassLoader();
     Object status;
+    boolean ea = false;
+
     if (c == null)
-      return VMClassLoader.defaultAssertionStatus();
+      {
+        ea = VMClassLoader.getSystemAssertionStatus();
+        c = VMClassLoader.getSystemClassLoader();
+      }
+    else
+      ea = c.defaultAssertionStatus;
+
     if (c.classAssertionStatus != null)
       synchronized (c)
         {
@@ -1278,7 +1286,7 @@ public final class Class<T>
         if (status != null)
           return status.equals(Boolean.TRUE);
       }
-    return c.defaultAssertionStatus;
+    return ea;
   }
 
   /**
diff --git a/vm/reference/java/lang/VMClassLoader.java b/vm/reference/java/lang/VMClassLoader.java
index c2817d3..eb5e5c5 100644
--- a/vm/reference/java/lang/VMClassLoader.java
+++ b/vm/reference/java/lang/VMClassLoader.java
@@ -1,6 +1,6 @@
 /* VMClassLoader.java -- Reference implementation of native interface
    required by ClassLoader
-   Copyright (C) 1998, 2001, 2002, 2004, 2005, 2006, 2010
+   Copyright (C) 1998, 2001, 2002, 2004, 2005, 2006, 2010, 2012
    Free Software Foundation, Inc.
 
 This file is part of GNU Classpath.
@@ -333,7 +333,19 @@ final class VMClassLoader
 
   /**
    * The system default for assertion status. This is used for all system
-   * classes (those with a null ClassLoader), as well as the initial value for
+   * classes (those with a null ClassLoader).
+   *
+   * XXX - Not implemented yet; this requires native help.
+   *
+   * @return the system-wide default assertion status
+   */
+  static final boolean getSystemAssertionStatus()
+  {
+    return true;
+  }
+
+  /**
+   * The default for assertion status. This is used as the initial value for
    * every ClassLoader's default assertion status.
    *
    * XXX - Not implemented yet; this requires native help.
--
1.7.7

---

[Pekka Enberg-4]

On Tue, Apr 24, 2012 at 9:40 PM, Josef Eisl
<zapster@...> wrote:
> Attached is a patch that enables GNU Classpath to distinguish between
> normal assertions and assertions for system classes (i.e. null
> classloader) as discussed on the Classpath mailing list [1].

Acked-by: Pekka Enberg <penberg@...>

On Tue, Apr 24, 2012 at 9:40 PM, Josef Eisl
<zapster@...> wrote:
> Note that the new method
> 'java/lang/VMClassLoader.getSystemAssertionStatus()' must be implemented
> otherwise an exception will occur.

Sure, but only for VMs that override GNU Classpath VM native classes.
Jato, for example, doesn't.

[Josef Eisl-2]

On 04/24/2012 08:52 PM, Pekka Enberg wrote:

> On Tue, Apr 24, 2012 at 9:40 PM, Josef Eisl
> <zapster@...> wrote:
>> Attached is a patch that enables GNU Classpath to distinguish between
>> normal assertions and assertions for system classes (i.e. null
>> classloader) as discussed on the Classpath mailing list [1].
>
> Acked-by: Pekka Enberg <penberg@...>
>
> On Tue, Apr 24, 2012 at 9:40 PM, Josef Eisl
> <zapster@...> wrote:
>> Note that the new method
>> 'java/lang/VMClassLoader.getSystemAssertionStatus()' must be implemented
>> otherwise an exception will occur.
>
> Sure, but only for VMs that override GNU Classpath VM native classes.
> Jato, for example, doesn't.

Oh, yeah. That's true. If the reference implementation is used
everything should be fine ;).

[ahughes-2]

----- Original Message -----

> Hi.
>
> Attached is a patch that enables GNU Classpath to distinguish between
> normal assertions and assertions for system classes (i.e. null
> classloader) as discussed on the Classpath mailing list [1].
>
> Note that the new method
> 'java/lang/VMClassLoader.getSystemAssertionStatus()' must be
> implemented
> otherwise an exception will occur.
>

I'm confused about something in this patch which perhaps you can explain.

I was under the impression that all system classes had assertions enabled
and disabled as 'system assertions' (-esa/-dsa).  So I was expecting
the patch to just return this value if the classloader was null.  Why does
it instead retrieve a system classloader and probe that on a per-package/class
basis?  How would these values be set?

As quoted before, the Oracle documentation explicitly states that -ea does
not turn on system assertions.  This patch would imply that e.g. assertions
for java.lang could be turned on by -ea:java.lang.  Is this intentional?

On the administrative side, AFAICS you haven't contributed before.  Do you have
a copyright assignment with the FSF?

> josef
>
> [1]:
> http://developer.classpath.org/pipermail/classpath/2012-April/003195.html
>
> On 04/24/2012 02:29 AM, Andrew Hughes wrote:
> > ----- Original Message -----
> >> Hi there.
> >>
> >> I want to reanimate a discussion from 2007 [1,2] about the
> >> Assertion/System Assertion handling. As far as I know GNU
> >> Classpath
> >> can't distinguish between normal assertions
> >> (-ea/-eanableassertions)
> >> and
> >> assertions for system classes (-esa/-enablesystemassertions, i.e.
> >> classes with no classloader [3]).
> >> The only interface for the VM is the method
> >> VMClassLoader.defaultAssertionStatus(). It is not clear if this is
> >> supposed to return the system assertion status or the normal
> >> assertion
> >> status (vm integration doc [4] vs. javadoc [5]). On the one hand,
> >> it
> >> is
> >> used to set the defaultAssertionStatus for java.lang.ClassLoader
> >> [6]
> >> which would imply that it is the normal assertion status. On the
> >> other
> >> hand, the very same flag is used for setting the assertion status
> >> for
> >> system class in java.lang.Class (desiredAssertionStatus)[7].
> >>
> >> I think we need two dedicated methods in VMClassLoader.
> >> getDefaultAssertionStatus() for normal assertions (i.e. -ea) and
> >> getDefaultSystemAssertionStatus() for system assertions (i.e.
> >> -esa).
> >> ClassLoader should use VMClassLoader.getDefaultAssertionStatus()
> >> as
> >> its
> >> default value and Class.getDesiredAssertionStatus() should use
> >> VMClassLoader.defaultSystemAssertionStatus() for system classes.
> >>
> >> I understand that this would be a rather big change because all
> >> VMs
> >> using GNU Classpath will fail if they do not adopt their
> >> VMClassLoader
> >> accordingly.
> >>
> >> I want to revitalize the discussion on this topic and I'm ready to
> >> prepare a patch for this issue but things should be discussed
> >> first
> >> ;).
> >>
> >> I hope my explanations are somehow understandable ;).
> >>
> >> - josef
> >>
> >> PS: I think the patch from 2007 is does not fix this issue as it
> >> does
> >> not modify java.lang.Class and java.lang.Classloader receptively.
> >>
> >>
> >> [1]:
> >> http://developer.classpath.org/pipermail/classpath-patches/2007-August/005601.html
> >> [2]:
> >> http://developer.classpath.org/pipermail/classpath-patches/2007-September/005605.html
> >> [3]:
> >> http://docs.oracle.com/javase/1.4.2/docs/guide/lang/assert.html#enable-disable
> >> [4]:
> >> http://www.gnu.org/software/classpath/docs/cp-vmintegration.html#SEC7
> >> [5]:
> >> http://git.savannah.gnu.org/cgit/classpath.git/tree/vm/reference/java/lang/VMClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n334
> >> [6]:
> >> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/ClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n220
> >> [7]:
> >> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/Class.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n1233
> >>
> >>
> >
> > You're right.  The issue is here:
> >
> > "But if you use them with no arguments (-ea or -da), they do not
> > apply to system classes."
> >
> > (from
> > http://java.sun.com/developer/technicalArticles/JavaLP/assertions/)
> >
> > Both when the classloader is null (a system class) and when there
> > is no class or package-specific setting, the same default setting
> > from VMClassLoader.defaultAssertionStatus() is used.
> >
> > There should instead a separate method for system classes as you
> > mention: VMClassLoader.getSystemAssertionStatus()
> > (I don't think it needs to be called default because it can't be
> > overridden by the class/package settings).
> >
> > As we're just after a release (0.99), this would be an ideal time
> > to add that method.
>
>

Thanks,
--
Andrew :)

Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)

PGP Key: 248BDC07 (https://keys.indymedia.org/)
Fingerprint = EC5A 1F5E C0AD 1D15 8F1F  8F91 3B96 A578 248B DC07

[Brian Jones-17]

> On the administrative side, AFAICS you haven't contributed before.  Do you have
> a copyright assignment with the FSF?

The change appears to be minor enough to not require the paperwork to
me.  Mostly comments, very little code.  But hey, if you're gonna be
contributing better to get that started sooner than later.

Brian

[Josef Eisl-2]

On 04/25/2012 02:07 AM, Andrew Hughes wrote:

> ----- Original Message -----
>> Hi.
>>
>> Attached is a patch that enables GNU Classpath to distinguish between
>> normal assertions and assertions for system classes (i.e. null
>> classloader) as discussed on the Classpath mailing list [1].
>>
>> Note that the new method
>> 'java/lang/VMClassLoader.getSystemAssertionStatus()' must be
>> implemented
>> otherwise an exception will occur.
>>
>
> I'm confused about something in this patch which perhaps you can explain.
>
> I was under the impression that all system classes had assertions enabled
> and disabled as 'system assertions' (-esa/-dsa).  So I was expecting
> the patch to just return this value if the classloader was null.  Why does
> it instead retrieve a system classloader and probe that on a per-package/class
> basis?  How would these values be set?
>
> As quoted before, the Oracle documentation explicitly states that -ea does
> not turn on system assertions.  This patch would imply that e.g. assertions
> for java.lang could be turned on by -ea:java.lang.  Is this intentional?

As far as I understood the documentation (in my case
http://docs.oracle.com/javase/6/docs/technotes/guides/language/assert.html#enable-disable
as a reference) it is true that '-ea' (no argument) only applies to
normal classes. The flag '-ea:*' does apply all classes:

"The above switches apply to all class loaders. With one exception, they
also apply to system classes (which do not have an explicit class
loader). The exception concerns the switches with no arguments, which
(as indicated above) do not apply to system classes."

Did I miss anything?

>
> On the administrative side, AFAICS you haven't contributed before.  Do you have
> a copyright assignment with the FSF?
>
>> josef
>>
>> [1]:
>> http://developer.classpath.org/pipermail/classpath/2012-April/003195.html
>>
>> On 04/24/2012 02:29 AM, Andrew Hughes wrote:
>>> ----- Original Message -----
>>>> Hi there.
>>>>
>>>> I want to reanimate a discussion from 2007 [1,2] about the
>>>> Assertion/System Assertion handling. As far as I know GNU
>>>> Classpath
>>>> can't distinguish between normal assertions
>>>> (-ea/-eanableassertions)
>>>> and
>>>> assertions for system classes (-esa/-enablesystemassertions, i.e.
>>>> classes with no classloader [3]).
>>>> The only interface for the VM is the method
>>>> VMClassLoader.defaultAssertionStatus(). It is not clear if this is
>>>> supposed to return the system assertion status or the normal
>>>> assertion
>>>> status (vm integration doc [4] vs. javadoc [5]). On the one hand,
>>>> it
>>>> is
>>>> used to set the defaultAssertionStatus for java.lang.ClassLoader
>>>> [6]
>>>> which would imply that it is the normal assertion status. On the
>>>> other
>>>> hand, the very same flag is used for setting the assertion status
>>>> for
>>>> system class in java.lang.Class (desiredAssertionStatus)[7].
>>>>
>>>> I think we need two dedicated methods in VMClassLoader.
>>>> getDefaultAssertionStatus() for normal assertions (i.e. -ea) and
>>>> getDefaultSystemAssertionStatus() for system assertions (i.e.
>>>> -esa).
>>>> ClassLoader should use VMClassLoader.getDefaultAssertionStatus()
>>>> as
>>>> its
>>>> default value and Class.getDesiredAssertionStatus() should use
>>>> VMClassLoader.defaultSystemAssertionStatus() for system classes.
>>>>
>>>> I understand that this would be a rather big change because all
>>>> VMs
>>>> using GNU Classpath will fail if they do not adopt their
>>>> VMClassLoader
>>>> accordingly.
>>>>
>>>> I want to revitalize the discussion on this topic and I'm ready to
>>>> prepare a patch for this issue but things should be discussed
>>>> first
>>>> ;).
>>>>
>>>> I hope my explanations are somehow understandable ;).
>>>>
>>>> - josef
>>>>
>>>> PS: I think the patch from 2007 is does not fix this issue as it
>>>> does
>>>> not modify java.lang.Class and java.lang.Classloader receptively.
>>>>
>>>>
>>>> [1]:
>>>> http://developer.classpath.org/pipermail/classpath-patches/2007-August/005601.html
>>>> [2]:
>>>> http://developer.classpath.org/pipermail/classpath-patches/2007-September/005605.html
>>>> [3]:
>>>> http://docs.oracle.com/javase/1.4.2/docs/guide/lang/assert.html#enable-disable
>>>> [4]:
>>>> http://www.gnu.org/software/classpath/docs/cp-vmintegration.html#SEC7
>>>> [5]:
>>>> http://git.savannah.gnu.org/cgit/classpath.git/tree/vm/reference/java/lang/VMClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n334
>>>> [6]:
>>>> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/ClassLoader.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n220
>>>> [7]:
>>>> http://git.savannah.gnu.org/cgit/classpath.git/tree/java/lang/Class.java?id=18f4bdd925d1a78d11598fb23dcaf1110772dcae#n1233
>>>>
>>>>
>>>
>>> You're right.  The issue is here:
>>>
>>> "But if you use them with no arguments (-ea or -da), they do not
>>> apply to system classes."
>>>
>>> (from
>>> http://java.sun.com/developer/technicalArticles/JavaLP/assertions/)
>>>
>>> Both when the classloader is null (a system class) and when there
>>> is no class or package-specific setting, the same default setting
>>> from VMClassLoader.defaultAssertionStatus() is used.
>>>
>>> There should instead a separate method for system classes as you
>>> mention: VMClassLoader.getSystemAssertionStatus()
>>> (I don't think it needs to be called default because it can't be
>>> overridden by the class/package settings).
>>>
>>> As we're just after a release (0.99), this would be an ideal time
>>> to add that method.
>>
>>
>
> Thanks,