This is the mail archive of the java@gcc.gnu.org mailing list for the Java project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: patch for PR java/1208, plus other try-finally cleanups


This is a revised patch, which also handles G19990210_1.java.

In an earlier message, I said it looked like the generated bytecode for
G19990210_1.java was ok, thus the verifier was at fault.  Well, that was
only half true.  Technically, the generated bytecode may be correct
(though I won't say for sure), but it is highly question able.  That
is because the exception range handled by the finally handler includes
not just the try clause, but also the following jsr that calls the
finalizer and the goto to the end.  Now we've can't actually get an
exception while the PC is at the jsr or goto, but they should certainly
not be considered part of the the try-clause.  Rather than trying to fix
the verifier (which gets into some fairly tricky code), I thought it
better to fix the code generator.

Note *all* try-finally statements have had this questionable code.

I've checked this into both the trunk and the branch.

I2001-03-23  Per Bothner  <per@bothner.com>

	* verify.c (verify_jvm_instructions):  Replace 3 pop_type by POP_TYPE
	macro for better error pin-pointing.
	* java-tree.h:  Fix typo in comment.

	* jcf-write.c (generate_bytecode_insns):  Changes to TRY_FINALLY_EXPR.
	Don't include jsr/goto in exception range.
	Check if start and end of exception range are the same (also TRY_EXPR).
	Don't emit jsr after try_block if CAN_COMPLETE_NORMALLY is false.
	However, do emit the following goto even if try_block is empty.
	Defer freeing exception_decl until after the finalizer, to make
	sure the local isn't reused in the finalizer.  Fixes PR java/1208.

	* parse.y (java_complete_lhs):  If the try-clause is empty, just
	return the finally-clause and vice versa.

ndex: jcf-write.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/jcf-write.c,v
retrieving revision 1.74
diff -u -p -r1.74 jcf-write.c
--- jcf-write.c	2001/03/23 19:42:25	1.74
+++ jcf-write.c	2001/03/24 01:07:09
@@ -111,7 +111,7 @@ struct chunk
 struct jcf_block
 {
   /* For blocks that that are defined, the next block (in pc order).
-     For blocks that are the not-yet-defined end label of a LABELED_BLOCK_EXPR
+     For blocks that are not-yet-defined the end label of a LABELED_BLOCK_EXPR
      or a cleanup expression (from a WITH_CLEANUP_EXPR),
      this is the next (outer) such end label, in a stack headed by
      labeled_blocks in jcf_partial. */
@@ -131,8 +131,8 @@ struct jcf_block
 
   int linenumber;
 
-  /* After finish_jcf_block is called, The actual instructions
-     contained in this block.  Before than NULL, and the instructions
+  /* After finish_jcf_block is called, the actual instructions
+     contained in this block.  Before that NULL, and the instructions
      are in state->bytecode. */
   union {
     struct chunk *chunk;
@@ -2311,6 +2311,8 @@ generate_bytecode_insns (exp, target, st
 	  abort ();
 	generate_bytecode_insns (try_clause, IGNORE_TARGET, state);
 	end_label = get_jcf_label_here (state);
+	if (end_label == start_label)
+	  break;
 	if (CAN_COMPLETE_NORMALLY (try_clause))
 	  emit_goto (finished_label, state);
 	while (clause != NULL_TREE)
@@ -2332,61 +2334,53 @@ generate_bytecode_insns (exp, target, st
       break;
     case TRY_FINALLY_EXPR:
       {
-	struct jcf_block *finished_label, *finally_label, *start_label;
+	struct jcf_block *finished_label,
+	  *finally_label, *start_label, *end_label;
 	struct jcf_handler *handler;
-	int worthwhile_finally = 1;
 	tree try_block = TREE_OPERAND (exp, 0);
 	tree finally = TREE_OPERAND (exp, 1);
-	tree return_link, exception_decl;
+	tree return_link = NULL_TREE, exception_decl = NULL_TREE;
 
-	finally_label = start_label = NULL;
-	return_link = exception_decl = NULL_TREE;
-	finished_label = gen_jcf_label (state);
+	tree exception_type;
 
-	/* If the finally clause happens to be empty, set a flag so we
-           remember to just skip it. */
-	if (BLOCK_EXPR_BODY (finally) == empty_stmt_node)
-	  worthwhile_finally = 0;
+	finally_label = gen_jcf_label (state);
+	start_label = get_jcf_label_here (state);
+	finally_label->pc = PENDING_CLEANUP_PC;
+	finally_label->next = state->labeled_blocks;
+	state->labeled_blocks = finally_label;
+	state->num_finalizers++;
 
-	if (worthwhile_finally)
-	  {
-	    tree exception_type;
-	    return_link = build_decl (VAR_DECL, NULL_TREE,
-				      return_address_type_node);
-	    exception_type = build_pointer_type (throwable_type_node);
-	    exception_decl = build_decl (VAR_DECL, NULL_TREE, exception_type);
+	generate_bytecode_insns (try_block, target, state);
+	if (state->labeled_blocks != finally_label)
+	  abort();
+	state->labeled_blocks = finally_label->next;
+	end_label = get_jcf_label_here (state);
 
-	    finally_label = gen_jcf_label (state);
-	    start_label = get_jcf_label_here (state);
-	    finally_label->pc = PENDING_CLEANUP_PC;
-	    finally_label->next = state->labeled_blocks;
-	    state->labeled_blocks = finally_label;
-	    state->num_finalizers++;
+	if (end_label == start_label)
+	  {
+	    state->num_finalizers--;
+	    define_jcf_label (finally_label, state);
+	    generate_bytecode_insns (finally, IGNORE_TARGET, state);
+	    break;
 	  }
 
-	generate_bytecode_insns (try_block, target, state);
+	return_link = build_decl (VAR_DECL, NULL_TREE,
+				  return_address_type_node);
+	finished_label = gen_jcf_label (state);
 
-	if (worthwhile_finally)
+
+	if (CAN_COMPLETE_NORMALLY (try_block))
 	  {
-	    if (state->labeled_blocks != finally_label)
-	      abort();
-	    state->labeled_blocks = finally_label->next;
 	    emit_jsr (finally_label, state);
+	    emit_goto (finished_label, state);
 	  }
 
-	if (CAN_COMPLETE_NORMALLY (try_block)
-	    && TREE_CODE (try_block) == BLOCK
-	    && BLOCK_EXPR_BODY (try_block) != empty_stmt_node)
-	  emit_goto (finished_label, state);
-
 	/* Handle exceptions. */
-
-	if (!worthwhile_finally)
-	  break;
 
+	exception_type = build_pointer_type (throwable_type_node);
+	exception_decl = build_decl (VAR_DECL, NULL_TREE, exception_type);
 	localvar_alloc (return_link, state);
-	handler = alloc_handler (start_label, NULL_PTR, state);
-	handler->end_label = handler->handler_label;
+	handler = alloc_handler (start_label, end_label, state);
 	handler->type = NULL_TREE;
 	localvar_alloc (exception_decl, state);
 	NOTE_PUSH (1);
@@ -2396,7 +2390,6 @@ generate_bytecode_insns (exp, target, st
 	RESERVE (1);
 	OP1 (OPCODE_athrow);
 	NOTE_POP (1);
-	localvar_free (exception_decl, state);
 
 	/* The finally block.  First save return PC into return_link. */
 	define_jcf_label (finally_label, state);
@@ -2405,6 +2398,7 @@ generate_bytecode_insns (exp, target, st
 
 	generate_bytecode_insns (finally, IGNORE_TARGET, state);
 	maybe_wide (OPCODE_ret, DECL_LOCAL_INDEX (return_link), state);
+	localvar_free (exception_decl, state);
 	localvar_free (return_link, state);
 	define_jcf_label (finished_label, state);
       }
Index: parse.y
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/parse.y,v
retrieving revision 1.265
diff -u -p -r1.265 parse.y
--- parse.y	2001/03/23 17:31:42	1.265
+++ parse.y	2001/03/24 01:07:18
@@ -11185,6 +11185,10 @@ java_complete_lhs (node)
     case TRY_FINALLY_EXPR:
       COMPLETE_CHECK_OP_0 (node);
       COMPLETE_CHECK_OP_1 (node);
+      if (TREE_OPERAND (node, 0) == empty_stmt_node)
+	return TREE_OPERAND (node, 1);
+      if (TREE_OPERAND (node, 1) == empty_stmt_node)
+	return TREE_OPERAND (node, 0);
       CAN_COMPLETE_NORMALLY (node)
 	= (CAN_COMPLETE_NORMALLY (TREE_OPERAND (node, 0))
 	   && CAN_COMPLETE_NORMALLY (TREE_OPERAND (node, 1)));
Index: verify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/verify.c,v
retrieving revision 1.40
diff -u -p -r1.40 verify.c
--- verify.c	2001/03/16 04:16:54	1.40
+++ verify.c	2001/03/24 01:07:19
@@ -1137,7 +1137,7 @@ verify_jvm_instructions (jcf, byte_ops, 
 
 	case OPCODE_athrow:
 	  /* FIXME: athrow also empties the stack. */
-	  pop_type (throwable_type_node);
+	  POP_TYPE (throwable_type_node, "missing throwable at athrow" );
 	  INVALIDATE_PC;
 	  break;
 
@@ -1156,7 +1156,7 @@ verify_jvm_instructions (jcf, byte_ops, 
 	  {
 	    jint low, high;
 
-	    pop_type (int_type_node);
+	    POP_TYPE (int_type_node, "missing int for tableswitch");
 	    while (PC%4)
 	      {
 	        if (byte_ops[PC++])
@@ -1179,7 +1179,7 @@ verify_jvm_instructions (jcf, byte_ops, 
 	  {
 	    jint npairs, last = 0, not_registered = 1;
 
-	    pop_type (int_type_node);
+	    POP_TYPE (int_type_node, "missing int for lookupswitch");
 	    while (PC%4)
 	      {
 	        if (byte_ops[PC++])
Index: java-tree.h
===================================================================
RCS file: /cvs/gcc/gcc/gcc/java/java-tree.h,v
retrieving revision 1.105
diff -u -p -r1.105 java-tree.h
--- java-tree.h	2001/03/23 19:42:25	1.105
+++ java-tree.h	2001/03/24 01:07:20
@@ -743,7 +743,7 @@ struct lang_identifier
 #define LABEL_RETURN_TYPE_STATE(NODE) LABEL_TYPE_STATE (LABEL_RETURN_LABEL (NODE))
 
 /* In a TREE_VEC for a LABEL_RETURN_TYPE_STATE, notes that
-   TREE_VEC_LENGTH has been adjust to the correct stack size. */
+   TREE_VEC_LENGTH has been adjusted to the correct stack size. */
 #define RETURN_MAP_ADJUSTED(NODE) TREE_LANG_FLAG_2(NODE)
 
 /* In the label of a sub-routine, a chain of the return location labels. */

-- 
	--Per Bothner
per@bothner.com   http://www.bothner.com/~per/


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]