Patchwork [Fortran] (2/n) Fix some issues found by Coverity's static-code analysis scan

login
register
mail settings
Submitter Tobias Burnus
Date Sept. 15, 2012, 1:03 p.m.
Message ID <50547CA6.9040006@net-b.de>
Download mbox | patch
Permalink /patch/184078/
State New
Headers show

Comments

Tobias Burnus - Sept. 15, 2012, 1:03 p.m.
That's a second patch which fixes some of the found issues.

Build and regtested on x86-64-gnu-linux.
OK for the trunk?

Tobias
Paul Richard Thomas - Sept. 15, 2012, 3:08 p.m.
Dear Tobias,

> Build and regtested on x86-64-gnu-linux.
> OK for the trunk?

I am not convinced that the gcc_asserts are need in encode_derived but
I guess they do no harm.

OK for trunk.

Thanks for the patch.

Paul

Patch

Fixed issues:

o match.c: One has "stat = tmp;" etc. If "tmp" is not reused, one
  might have a double free in the error case.
  MATCH_ERROR: Unreachable - it is already handled before.

o simplify.c:
  - Double free. 
  - Some duplicated code (failing patch? Copy and paste?)

o target-memory.c: There were several cases where the result could be
  -1 (e.g. if the size is too big for the data type). The scanner thus
  warned that one could pass -1 to, e.g., malloc's size argument. I
  fixed that by adding a couple of asserts and switched to unsigned.
  To increase the chance that the size is sufficiently large, I
  replaced int by HOST_WIDE_INT.

o symbol.c: The first one is again a case where "derived == NULL" is
  checked - but before "derived->" is used.
  And "(sym &&" is pointless if there is there is a "if (!sym || ...) return.


2012-09-15  Tobias Burnus  <burnus@net-b.de>

	* match.c (lock_unlock_statement, sync_statement): If potential
	double freeing.
	(sync_statement): Remove unreachable code.
	* simplify.c (gfc_simplify_bessel_n2): Avoid double freeing.
	(gfc_simplify_repeat): Remove bogus code.
	* target-memory.h (gfc_target_encode_expr): Update prototype.
	* target-memory.c (gfc_target_encode_expr, encode_array,
	encode_derived): Return unsigned HOST_WIDE_INT.
	(gfc_target_interpret_expr): Add assert.
	(gfc_merge_initializers): Fix "== 0" check for mpz_t.
	* symbol.c (gfc_get_typebound_proc): Add assert.
	(gfc_merge_initializers): Remove unreachable check.


diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index cf85d52..d46a495 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -2959,21 +2959,25 @@  done:
   new_st.expr4 = acq_lock;
 
   return MATCH_YES;
 
 syntax:
   gfc_syntax_error (st);
 
 cleanup:
+  if (acq_lock != tmp)
+    gfc_free_expr (acq_lock);
+  if (errmsg != tmp)
+    gfc_free_expr (errmsg);
+  if (stat != tmp)
+    gfc_free_expr (stat);
+
   gfc_free_expr (tmp);
   gfc_free_expr (lockvar);
-  gfc_free_expr (acq_lock);
-  gfc_free_expr (stat);
-  gfc_free_expr (errmsg);
 
   return MATCH_ERROR;
 }
 
 
 match
 gfc_match_lock (void)
 {
@@ -3116,19 +3120,16 @@  sync_statement (gfc_statement st)
 
 	  tmp = NULL;
 	  break;
 	}
 
 	break;
     }
 
-  if (m == MATCH_ERROR)
-    goto syntax;
-
   if (gfc_match (" )%t") != MATCH_YES)
     goto syntax;
 
 done:
   switch (st)
     {
     case ST_SYNC_ALL:
       new_st.op = EXEC_SYNC_ALL;
@@ -3148,20 +3149,23 @@  done:
   new_st.expr3 = errmsg;
 
   return MATCH_YES;
 
 syntax:
   gfc_syntax_error (st);
 
 cleanup:
+  if (stat != tmp)
+    gfc_free_expr (stat);
+  if (errmsg != tmp)
+    gfc_free_expr (errmsg);
+
   gfc_free_expr (tmp);
   gfc_free_expr (imageset);
-  gfc_free_expr (stat);
-  gfc_free_expr (errmsg);
 
   return MATCH_ERROR;
 }
 
 
 /* Match SYNC ALL statement.  */
 
 match
diff --git a/gcc/fortran/simplify.c b/gcc/fortran/simplify.c
index 07aeee8..5aa2704 100644
--- a/gcc/fortran/simplify.c
+++ b/gcc/fortran/simplify.c
@@ -1365,17 +1365,21 @@  gfc_simplify_bessel_n2 (gfc_expr *order1, gfc_expr *order2, gfc_expr *x,
 	}
 
       mpfr_mul_si (e->value.real, x2rev, jn ? (n2-i+1) : (n1+i-1),
 		   GFC_RND_MODE);
       mpfr_mul (e->value.real, e->value.real, last2, GFC_RND_MODE);
       mpfr_sub (e->value.real, e->value.real, last1, GFC_RND_MODE);
 
       if (range_check (e, jn ? "BESSEL_JN" : "BESSEL_YN") == &gfc_bad_expr)
-	goto error;
+	{
+	  /* Range_check frees "e" in that case.  */
+	  e = NULL;
+	  goto error;
+	}
 
       if (jn)
 	gfc_constructor_insert_expr (&result->value.constructor, e, &x->where,
 				     -i-1);
       else
 	gfc_constructor_append_expr (&result->value.constructor, e, &x->where);
 
       mpfr_set (last1, last2, GFC_RND_MODE);
@@ -4925,21 +4929,16 @@  gfc_simplify_repeat (gfc_expr *e, gfc_expr *n)
        mpz_sgn (e->ts.u.cl->length->value.integer)) != 0)
     {
       const char *res = gfc_extract_int (n, &ncop);
       gcc_assert (res == NULL);
     }
   else
     ncop = 0;
 
-  len = e->value.character.length;
-  nlen = ncop * len;
-
-  result = gfc_get_constant_expr (BT_CHARACTER, e->ts.kind, &e->where);
-
   if (ncop == 0)
     return gfc_get_character_expr (e->ts.kind, &e->where, NULL, 0);
 
   len = e->value.character.length;
   nlen = ncop * len;
 
   result = gfc_get_character_expr (e->ts.kind, &e->where, NULL, nlen);
   for (i = 0; i < ncop; i++)
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 1f4a735..d68208d 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4841,17 +4841,19 @@  gfc_get_typebound_proc (gfc_typebound_proc *tb0)
 }
 
 
 /* Get the super-type of a given derived type.  */
 
 gfc_symbol*
 gfc_get_derived_super_type (gfc_symbol* derived)
 {
-  if (derived && derived->attr.generic)
+  gcc_assert (derived);
+
+  if (derived->attr.generic)
     derived = gfc_find_dt_in_generic (derived);
 
   if (!derived->attr.extension)
     return NULL;
 
   gcc_assert (derived->components);
   gcc_assert (derived->components->ts.type == BT_DERIVED);
   gcc_assert (derived->components->ts.u.derived);
@@ -4963,13 +4965,13 @@  gfc_symbol *
 gfc_find_dt_in_generic (gfc_symbol *sym)
 {
   gfc_interface *intr = NULL;
 
   if (!sym || sym->attr.flavor == FL_DERIVED)
     return sym;
 
   if (sym->attr.generic)
-    for (intr = (sym ? sym->generic : NULL); intr; intr = intr->next)
+    for (intr = sym->generic; intr; intr = intr->next)
       if (intr->sym->attr.flavor == FL_DERIVED)
         break;
   return intr ? intr->sym : NULL;
 }
diff --git a/gcc/fortran/target-memory.c b/gcc/fortran/target-memory.c
index 637811e..bedc668 100644
--- a/gcc/fortran/target-memory.c
+++ b/gcc/fortran/target-memory.c
@@ -135,17 +135,17 @@  gfc_target_expr_size (gfc_expr *e)
     }
 }
 
 
 /* The encode_* functions export a value into a buffer, and 
    return the number of bytes of the buffer that have been
    used.  */
 
-static int
+static unsigned HOST_WIDE_INT
 encode_array (gfc_expr *expr, unsigned char *buffer, size_t buffer_size)
 {
   mpz_t array_size;
   int i;
   int ptr = 0;
 
   gfc_constructor_base ctor = expr->value.constructor;
 
@@ -212,51 +212,57 @@  gfc_encode_character (int kind, int length, const gfc_char_t *string,
   for (i = 0; i < length; i++)
     native_encode_expr (build_int_cst (type, string[i]), &buffer[i*elsize],
 			elsize);
 
   return length;
 }
 
 
-static int
+static unsigned HOST_WIDE_INT
 encode_derived (gfc_expr *source, unsigned char *buffer, size_t buffer_size)
 {
   gfc_constructor *c;
   gfc_component *cmp;
   int ptr;
   tree type;
+  HOST_WIDE_INT size;
 
   type = gfc_typenode_for_spec (&source->ts);
 
   for (c = gfc_constructor_first (source->value.constructor),
        cmp = source->ts.u.derived->components;
        c;
        c = gfc_constructor_next (c), cmp = cmp->next)
     {
       gcc_assert (cmp);
       if (!c->expr)
 	continue;
       ptr = TREE_INT_CST_LOW(DECL_FIELD_OFFSET(cmp->backend_decl))
 	    + TREE_INT_CST_LOW(DECL_FIELD_BIT_OFFSET(cmp->backend_decl))/8;
 
       if (c->expr->expr_type == EXPR_NULL)
- 	memset (&buffer[ptr], 0,
-		int_size_in_bytes (TREE_TYPE (cmp->backend_decl)));
+	{
+	  size = int_size_in_bytes (TREE_TYPE (cmp->backend_decl));
+	  gcc_assert (size >= 0);
+	  memset (&buffer[ptr], 0, size);
+	}
       else
 	gfc_target_encode_expr (c->expr, &buffer[ptr],
 				buffer_size - ptr);
     }
 
-  return int_size_in_bytes (type);
+  size = int_size_in_bytes (type);
+  gcc_assert (size >= 0);
+  return size;
 }
 
 
 /* Write a constant expression in binary form to a buffer.  */
-int
+unsigned HOST_WIDE_INT
 gfc_target_encode_expr (gfc_expr *source, unsigned char *buffer,
 			size_t buffer_size)
 {
   if (source == NULL)
     return 0;
 
   if (source->expr_type == EXPR_ARRAY)
     return encode_array (source, buffer, buffer_size);
@@ -562,16 +568,17 @@  gfc_target_interpret_expr (unsigned char *buffer, size_t buffer_size,
     case BT_CHARACTER:
       result->representation.length = 
         gfc_interpret_character (buffer, buffer_size, result);
       break;
 
     case BT_DERIVED:
       result->representation.length = 
         gfc_interpret_derived (buffer, buffer_size, result);
+      gcc_assert (result->representation.length >= 0);
       break;
 
     default:
       gfc_internal_error ("Invalid expression in gfc_target_interpret_expr.");
       break;
     }
 
   if (result->ts.type == BT_CHARACTER && convert_widechar)
@@ -673,17 +680,17 @@  gfc_merge_initializers (gfc_typespec ts, gfc_expr *e, unsigned char *data,
       break;
 
     case EXPR_ARRAY:
       for (c = gfc_constructor_first (e->value.constructor);
 	   c; c = gfc_constructor_next (c))
 	{
 	  size_t elt_size = gfc_target_expr_size (c->expr);
 
-	  if (c->offset)
+	  if (mpz_cmp_si (c->offset, 0) != 0)
 	    len = elt_size * (size_t)mpz_get_si (c->offset);
 
 	  len = len + gfc_merge_initializers (ts, c->expr, &data[len],
 					      &chk[len], length - len);
 	}
       break;
 
     default:
diff --git a/gcc/fortran/target-memory.h b/gcc/fortran/target-memory.h
index 6ebffe8..cba2ea2 100644
--- a/gcc/fortran/target-memory.h
+++ b/gcc/fortran/target-memory.h
@@ -26,17 +26,18 @@  along with GCC; see the file COPYING3.  If not see
 bool gfc_convert_boz (gfc_expr *, gfc_typespec *);
 
 /* Return the size of an expression in its target representation.  */
 size_t gfc_target_expr_size (gfc_expr *);
 
 /* Write a constant expression in binary form to a target buffer.  */
 int gfc_encode_character (int, int, const gfc_char_t *, unsigned char *,
 			  size_t);
-int gfc_target_encode_expr (gfc_expr *, unsigned char *, size_t);
+unsigned HOST_WIDE_INT gfc_target_encode_expr (gfc_expr *, unsigned char *,
+					       size_t);
 
 /* Read a target buffer into a constant expression.  */
 
 int gfc_interpret_integer (int, unsigned char *, size_t, mpz_t);
 int gfc_interpret_float (int, unsigned char *, size_t, mpfr_t);
 int gfc_interpret_complex (int, unsigned char *, size_t, mpc_t);
 int gfc_interpret_logical (int, unsigned char *, size_t, int *);
 int gfc_interpret_character (unsigned char *, size_t, gfc_expr *);