[PATCH] Fix PR C/C++/38699, ICE using offsetof with pointer and array accesses

Jakub Jelinek jakub@redhat.com
Wed Jan 7 15:49:00 GMT 2009


On Sat, Jan 03, 2009 at 01:54:55AM -0500, Andrew Pinski wrote:
> * gcc.dg/offsetof-1.c: New testcase.
> * gcc.dg/offsetof-2.c: New test.
> * gcc.dg/offsetof-3.c: New test.
> * g++.dg/parse/offsetof9.C: New test.
> * g++.dg/parse/offsetof10.C: New test.
> * g++.dg/parse/offsetof11.C: New test.
> * g++.dg/parse/offsetof12.C: New test.
> * g++.dg/parse/offsetof13.C: New test.
> * g++.dg/parse/offsetof14.C: New test.

I'd say you just want one testcase for the non-errorneous cases
and one for errorneous, and the former one should be actually a runtime
testcase that verifies offseof returned the expected value
(e.g. by declaring a static variable of that type and subtract
its address from pointer to the field and comparing that to offseof).

> @@ -7599,11 +7600,35 @@ fold_offsetof_1 (tree expr, tree stop_re
>        gcc_assert (integer_zerop (expr));
>        return size_zero_node;
>  
> -    case NOP_EXPR:
>      case INDIRECT_REF:
> -      base = fold_offsetof_1 (TREE_OPERAND (expr, 0), stop_ref);
> -      gcc_assert (base == error_mark_node || base == size_zero_node);
> -      return base;
> +      expr = TREE_OPERAND (expr, 0);
> +      if (expr == stop_ref && TREE_CODE (expr) != ERROR_MARK) 
> +        return size_zero_node;

Why the above 2 lines?  If stop_ref is expr is stop_ref, then
STRIP_NOPS won't do anything on it and so the following if will catch it.
Also, you should use expr != error_mark_node consistently.

> +    case NOP_EXPR:
> +      STRIP_NOPS (expr);
> +      if (expr == stop_ref && TREE_CODE (expr) != ERROR_MARK) 
> +        return size_zero_node;
> +      if (expr != error_mark_node && TREE_CODE (expr) != INTEGER_CST)
> +	{
> +	  while (TREE_CODE (expr) == POINTER_PLUS_EXPR)
> +	    expr = TREE_OPERAND (expr, 0);
> +	  while (handled_component_p (expr) && TREE_CODE (expr) != COMPONENT_REF)
> +	    expr = TREE_OPERAND (expr, 0);
> +	  if (TREE_CODE (expr) == COMPONENT_REF)
> +	    {
> +	      tree oldexpr = expr;
> +	      tree newexpr = TREE_OPERAND (expr, 0);
> +	      while (TREE_CODE (newexpr) == COMPONENT_REF)
> +		oldexpr = newexpr, newexpr = TREE_OPERAND (newexpr, 0);
> +	      error ("cannot apply %<offsetof%> to an array reference of the member %qD",

Is member the right term for C?  Shouldn't that be field?

> +		     TREE_OPERAND (oldexpr, 1));
> +	    }
> +	    /* If we cannot figure out what the field it is, give up. */
> +	  else
> +	    error ("cannot apply %<offsetof%> to an array reference to a pointer");
> +	  return error_mark_node;
> +	}
> +      return size_zero_node;

Also, shouldn't there be gcc_assert (integer_zerop (expr)) if expr is
INTEGER_CST?  For expr == error_mark_node you should probably just return
expr.

	Jakub



More information about the Gcc-patches mailing list