Patch: FYI: one more stab at PR 20056
Tom Tromey
tromey@redhat.com
Wed Feb 23 01:06:00 GMT 2005
I'm checking this in.
My previous PR 20056 failed because it didn't properly handle another
case. And, the test suite failed to actually test the bug -- we don't
test -findirect-dispatch (so the new verifier is not exercised), and
on my machine the interpreter tests don't run (possible dejagnu
problem? Last time I looked, nothing made sense).
I ran all the usual tests with this patch, plus rebuilt eclipse, plus
rebuilt the failing jonas .jar with -findirect-dispatch, plus ran the
PR 20056 test case (both original and the one in the test suite) by
hand (through both gcj -findirect-dispatch and gij).
This patch also tests that we don't try to set a field on an
uninitialized object other than our own 'this'. You can't write java
code to test this, but you could write invalid bytecode that tries
this.
We really need to set up VM-level testing. Automated testing for the
verifier (i.e., flesh out the mauve verify module and run it
automatically from the libgj test suite) is also needed. The current
situation is unstable.
Tom
Index: gcc/java/ChangeLog
from Tom Tromey <tromey@redhat.com>
PR java/20056:
* verify-impl.c (EITHER): New define.
(types_compatible): Handle it.
(check_field_constant): Use it.
Index: libjava/ChangeLog
from Tom Tromey <tromey@redhat.com>
PR java/20056:
* verify.cc (type::EITHER): New constant.
(check_field_constant): Use it.
(type::compatible): Handle it.
Index: gcc/java/verify-impl.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/verify-impl.c,v
retrieving revision 1.6
diff -u -r1.6 verify-impl.c
--- gcc/java/verify-impl.c 19 Feb 2005 04:02:09 -0000 1.6
+++ gcc/java/verify-impl.c 22 Feb 2005 17:56:05 -0000
@@ -539,16 +539,19 @@
First, when constructing a new object, it is the PC of the
`new' instruction which created the object. We use the special
- value UNINIT to mean that this is uninitialized, and the
- special value SELF for the case where the current method is
- itself the <init> method.
-
+ value UNINIT to mean that this is uninitialized. The special
+ value SELF is used for the case where the current method is
+ itself the <init> method. the special value EITHER is used
+ when we may optionally allow either an uninitialized or
+ initialized reference to match.
+
Second, when the key is return_address_type, this holds the PC
of the instruction following the `jsr'. */
int pc;
- #define UNINIT -2
- #define SELF -1
+#define UNINIT -2
+#define SELF -1
+#define EITHER -3
};
#if 0
@@ -721,19 +724,33 @@
if (k->klass == NULL)
verify_fail ("programmer error in type::compatible");
- /* An initialized type and an uninitialized type are not
- compatible. */
- if (type_initialized (t) != type_initialized (k))
- return false;
-
- /* Two uninitialized objects are compatible if either:
- * The PCs are identical, or
- * One PC is UNINIT. */
- if (type_initialized (t))
- {
- if (t->pc != k->pc && t->pc != UNINIT && k->pc != UNINIT)
+ /* Handle the special 'EITHER' case, which is only used in a
+ special case of 'putfield'. Note that we only need to handle
+ this on the LHS of a check. */
+ if (! type_initialized (t) && t->pc == EITHER)
+ {
+ /* If the RHS is uninitialized, it must be an uninitialized
+ 'this'. */
+ if (! type_initialized (k) && k->pc != SELF)
return false;
}
+ else if (type_initialized (t) != type_initialized (k))
+ {
+ /* An initialized type and an uninitialized type are not
+ otherwise compatible. */
+ return false;
+ }
+ else
+ {
+ /* Two uninitialized objects are compatible if either:
+ * The PCs are identical, or
+ * One PC is UNINIT. */
+ if (type_initialized (t))
+ {
+ if (t->pc != k->pc && t->pc != UNINIT && k->pc != UNINIT)
+ return false;
+ }
+ }
return ref_compatible (t->klass, k->klass);
}
@@ -2162,7 +2179,11 @@
&& vfr->current_state->this_type.pc == SELF
&& types_equal (&vfr->current_state->this_type, &ct)
&& vfy_class_has_field (vfr->current_class, name, field_type))
- type_set_uninitialized (class_type, SELF);
+ /* Note that we don't actually know whether we're going to match
+ against 'this' or some other object of the same type. So,
+ here we set things up so that it doesn't matter. This relies
+ on knowing what our caller is up to. */
+ type_set_uninitialized (class_type, EITHER);
return t;
}
Index: libjava/verify.cc
===================================================================
RCS file: /cvs/gcc/gcc/libjava/verify.cc,v
retrieving revision 1.67
diff -u -r1.67 verify.cc
--- libjava/verify.cc 19 Feb 2005 03:57:19 -0000 1.67
+++ libjava/verify.cc 22 Feb 2005 17:56:12 -0000
@@ -578,9 +578,11 @@
//
// First, when constructing a new object, it is the PC of the
// `new' instruction which created the object. We use the special
- // value UNINIT to mean that this is uninitialized, and the
- // special value SELF for the case where the current method is
- // itself the <init> method.
+ // value UNINIT to mean that this is uninitialized. The special
+ // value SELF is used for the case where the current method is
+ // itself the <init> method. the special value EITHER is used
+ // when we may optionally allow either an uninitialized or
+ // initialized reference to match.
//
// Second, when the key is return_address_type, this holds the PC
// of the instruction following the `jsr'.
@@ -588,6 +590,7 @@
static const int UNINIT = -2;
static const int SELF = -1;
+ static const int EITHER = -3;
// Basic constructor.
type ()
@@ -734,19 +737,33 @@
if (k.klass == NULL)
verifier->verify_fail ("programmer error in type::compatible");
- // An initialized type and an uninitialized type are not
- // compatible.
- if (isinitialized () != k.isinitialized ())
- return false;
-
- // Two uninitialized objects are compatible if either:
- // * The PCs are identical, or
- // * One PC is UNINIT.
- if (! isinitialized ())
+ // Handle the special 'EITHER' case, which is only used in a
+ // special case of 'putfield'. Note that we only need to handle
+ // this on the LHS of a check.
+ if (! isinitialized () && pc == EITHER)
{
- if (pc != k.pc && pc != UNINIT && k.pc != UNINIT)
+ // If the RHS is uninitialized, it must be an uninitialized
+ // 'this'.
+ if (! k.isinitialized () && k.pc != SELF)
return false;
}
+ else if (isinitialized () != k.isinitialized ())
+ {
+ // An initialized type and an uninitialized type are not
+ // otherwise compatible.
+ return false;
+ }
+ else
+ {
+ // Two uninitialized objects are compatible if either:
+ // * The PCs are identical, or
+ // * One PC is UNINIT.
+ if (! isinitialized ())
+ {
+ if (pc != k.pc && pc != UNINIT && k.pc != UNINIT)
+ return false;
+ }
+ }
return klass->compatible(k.klass, verifier);
}
@@ -2003,7 +2020,11 @@
// We don't look at the signature, figuring that if it is
// wrong we will fail during linking. FIXME?
&& _Jv_Linker::has_field_p (current_class, name))
- class_type->set_uninitialized (type::SELF, this);
+ // Note that we don't actually know whether we're going to match
+ // against 'this' or some other object of the same type. So,
+ // here we set things up so that it doesn't matter. This relies
+ // on knowing what our caller is up to.
+ class_type->set_uninitialized (type::EITHER, this);
return result;
}
More information about the Gcc-patches
mailing list