Unspecified behaviour in java/gjavah.c

Mon Nov 25 03:19:00 GMT 2002

After the latest merge from basic-improvements to tree-ssa, I
started getting failures building libjava:
-----------------------------------------------------------------------------
+ ../../gcc/gcjh -classpath '' -bootclasspath . java/lang/ThreadLocal
Premature end of .class file ./java/lang/ThreadLocal.class.
-----------------------------------------------------------------------------
I tracked down the problem to java/jcf-reader.c:skip_attribute. The
.i file contains this:
-----------------------------------------------------------------------------
 1	static void
 2	skip_attribute (jcf, number_of_attribute)
 3	 JCF *jcf;
 4	 int number_of_attribute;
 5	{
 6	 while (number_of_attribute--)
 7	 {
 8	 ((jcf)->read_end-(jcf)->read_ptr >= (6) ? 0 : (*(jcf)->filbuf)(jcf, 6));
 9	 (void) ((jcf)->read_ptr += 2, ((((jcf)->read_ptr-2)[0] << 8) | (((jcf)->read_ptr-2)[1])));
 10	 ((jcf)->read_ptr += (((jcf)->read_ptr += 4, (((unsigned long)((jcf)->read_ptr-4)[0] << 24) | ((unsigned long)((jcf)->read_ptr-4)[1] << 16) | ((unsigned long)((jcf)->read_ptr-4)[2] << 8) | ((unsigned long)((jcf)->read_ptr-4)[3])))));
 11	 }
 12	}
-----------------------------------------------------------------------------
The problematic line is 10. When we convert it to GIMPLE form,
the pre-merge version works fine, but the post-merge version
causes gjh to mess up the file pointer. After discussing it with
a few folks, the consensus is that the code is invoking
unspecified behaviour.
Below is a minimal test case that exhibits the same problem:
-----------------------------------------------------------------------------
struct JCF {
 char *read_ptr;
} *jcf;
foo()
{
 jcf->read_ptr += (jcf->read_ptr += 4, 5);
}
-----------------------------------------------------------------------------
This is how the gimplified version looks like with the pre-merged
(good, on the left) and post-merged (bad, on the right) branch:
-----------------------------------------------------------------------------
$ diff -y -W80 good.c.t04.simple bad.c.t04.simple 
foo() foo()
{ {
 char * T.1; char * T.1;
 char * T.2; char * T.2;
 T.1 = jcf->read_ptr; T.1 = jcf->read_ptr;
 jcf->read_ptr = T.1 + 4B; <
 T.2 = jcf->read_ptr; T.2 = jcf->read_ptr;
 jcf->read_ptr = T.2 + 5B | jcf->read_ptr = T.2 + 4B;
 > jcf->read_ptr = T.1 + 5B
} }
-----------------------------------------------------------------------------
The reason why the two versions get gimplified differently is
because the front end is actually emitting different trees. This
is the tree that the pre-merge gimplifier receives:

--- Pre-merge ---------------------------------------------------------------
Breakpoint 1, simplify_function_tree (fndecl=0x40079740)
 at /home/dnovillo/tree-ssa/src/gcc/gimplify.c:122
[ ... ]
(gdb) call print_c_tree(stdout,fnbody)
{ 
 jcf->read_ptr = jcf->read_ptr = jcf->read_ptr + 4B, jcf->read_ptr + 5B;
}
-----------------------------------------------------------------------------
And this is the tree that the post-merge gimplifier receives:
--- Post-merge --------------------------------------------------------------
Breakpoint 1, simplify_function_tree (fndecl=0x4007c4fc)
 at /home/dnovillo/merge-tree-ssa/src/gcc/gimplify.c:122
[ ... ]
(gdb) call print_c_tree (stdout,fnbody)
{
 jcf->read_ptr = jcf->read_ptr + (jcf->read_ptr = jcf->read_ptr + 4B, 5B);
}
-----------------------------------------------------------------------------
I have modified jcf-reader.c to avoid this, but I don't know the
code very well. Is the patch below appropriate?
Thanks. Diego.
	* java/jcf-reader.c: Don't expand JCF_readu4 inside the
	expansion of JCF_SKIP.
Index: jcf-reader.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/jcf-reader.c,v
retrieving revision 1.15
diff -d -u -p -r1.15 jcf-reader.c
--- jcf-reader.c	23 Mar 2001 19:42:25 -0000	1.15
+++ jcf-reader.c	25 Nov 2002 00:27:23 -0000
@@ -96,9 +96,11 @@ skip_attribute (jcf, number_of_attribute
 {
 while (number_of_attribute--)
 {
+ JCF_u4 N;
 JCF_FILL (jcf, 6);
 (void) JCF_readu2 (jcf);
- JCF_SKIP (jcf, JCF_readu4 (jcf));
+ N = JCF_readu4 (jcf);
+ JCF_SKIP (jcf, N);
 }
 }
 #endif
