Bug 38699 - [4.3/4.4/4.5 regression] ICE using offsetof with pointer and array accesses
Summary: [4.3/4.4/4.5 regression] ICE using offsetof with pointer and array accesses
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: 4.4.0
: P2 normal
Target Milestone: 4.4.3
Assignee: Dodji Seketeli
URL: http://gcc.gnu.org/ml/gcc-patches/200...
Keywords: accepts-invalid, ice-on-invalid-code, monitored, patch
Depends on:
Blocks:
 
Reported: 2009-01-02 14:39 UTC by Volker Reichelt
Modified: 2009-11-09 21:42 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work: 4.1.2
Known to fail:
Last reconfirmed: 2009-10-28 15:58:51


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Volker Reichelt 2009-01-02 14:39:52 UTC
The following valid code snippet triggers an ICE since GCC 4.2.0:

======================================
struct A
{
  const char* p;
};

void foo()
{
  __builtin_offsetof(struct A, p[0]);
}
======================================

bug.cc: In function 'void foo()':
bug.cc:8: internal compiler error: in fold_offsetof_1, at c-common.c:7647
Please submit a full bug report, [etc.]

The code is accepted by the C frontend.
Comment 1 Jakub Jelinek 2009-01-02 15:20:17 UTC
That seems invalid, not valid.
E.g. C says that for offsetof (type, member) for
static type t;
&(t.member) evaluates to an address constant, which is not the case for
const char *p; field and p[0].
Comment 2 Andrew Pinski 2009-01-02 20:06:49 UTC
Here is a testcase which ICEs with both the C and C++ front-end and is not rejected by either of them:
struct A
{
  const char* p;
};

void foo()
{
  __builtin_offsetof(struct A, p[1]);
}
--- CUT ---
This is invalid code as p[1] is not a member of struct A, only p is a member.
Comment 3 Andrew Pinski 2009-01-02 22:24:33 UTC
I have a patch at least for the C front-end, working on the C++ front-end now.
Comment 4 Andrew Pinski 2009-01-03 00:12:56 UTC
C is so much easier because templates.
Comment 5 Andrew Pinski 2009-01-03 05:59:04 UTC
Ok, I have a simpler patch to just c-common.c to error out in this case.
Comment 6 Paolo Bonzini 2009-02-04 07:53:00 UTC
andrew, ping the patch :-)
Comment 7 Joseph S. Myers 2009-03-31 21:06:09 UTC
Closing 4.2 branch.
Comment 8 Richard Biener 2009-08-04 12:29:43 UTC
GCC 4.3.4 is being released, adjusting target milestone.
Comment 9 Andrew Pinski 2009-09-20 20:12:26 UTC
I am no longer working on this patch ...
Comment 10 Dodji Seketeli 2009-10-28 15:42:28 UTC
Subject: Re:  [4.3/4.4/4.5 regression] ICE using offsetof
	with pointer and array accesses

I am testing the patch below.

I am not sure the approach is the right one though. Comments welcome.

diff --git a/gcc/c-common.c b/gcc/c-common.c
index 8a6d15b..54e551f 100644
--- a/gcc/c-common.c
+++ b/gcc/c-common.c
@@ -8341,6 +8341,32 @@ fold_offsetof_1 (tree expr, tree stop_ref)
 
     case NOP_EXPR:
     case INDIRECT_REF:
+      if (TREE_CODE (expr) == INDIRECT_REF)
+	{
+	  tree r = TREE_OPERAND (expr, 0);
+
+	  if ((TREE_CODE (r) == NON_LVALUE_EXPR
+	       && TREE_CODE (TREE_TYPE (r)) == POINTER_TYPE)
+	      ||
+	      (TREE_CODE (r) == POINTER_PLUS_EXPR))
+	    {
+	      /* We are trying something like:
+		 struct A
+		 {
+		   char *p;
+		 };
+		 void f ()
+		 {
+		   __builtin_offsetof(struct A, p[1]);
+		 }
+		 But the C spec says that if t is of type A, then
+		  &(t.p[1])" should evaluate to a constant address.
+		  And &(t.p[1]) does not evaluate to a constant address here.
+		 */
+	      error ("cannot apply %<offsetof%> to a non constant address");
+	      return error_mark_node;
+	    }
+	}
       base = fold_offsetof_1 (TREE_OPERAND (expr, 0), stop_ref);
       gcc_assert (base == error_mark_node || base == size_zero_node);
       return base;
@@ -8361,6 +8387,16 @@ fold_offsetof_1 (tree expr, tree stop_ref)
 			    size_int (tree_low_cst (DECL_FIELD_BIT_OFFSET (t),
 						    1)
 				      / BITS_PER_UNIT));
+      /* Check if we the offset goes beyond the bound of the struct.  */
+      if (int_cst_value (off)
+	  >= (int_cst_value (TYPE_SIZE (TREE_TYPE (TREE_OPERAND (expr, 0))))
+	      / BITS_PER_UNIT))
+	{
+	  error_at (EXPR_LOCATION (t),
+		    "expression %qE denotes an offset greater than size of %qT",
+		    t, TREE_TYPE (TREE_OPERAND (expr, 0)));
+	  return error_mark_node;
+	}
       break;
 
     case ARRAY_REF:
@@ -8376,6 +8412,17 @@ fold_offsetof_1 (tree expr, tree stop_ref)
 	}
       t = convert (sizetype, t);
       off = size_binop (MULT_EXPR, TYPE_SIZE_UNIT (TREE_TYPE (expr)), t);
+
+      /* Check if we the indice of the array goes beyond the bound.  */
+      if (int_cst_value (off)
+	  >= (int_cst_value (TYPE_SIZE (TREE_TYPE (TREE_OPERAND (expr, 0))))
+	      / BITS_PER_UNIT))
+	{
+	  error_at (EXPR_LOCATION (expr),
+		    "indice %ld denotes an offset greater than size of %qT",
+		    int_cst_value (t), TREE_TYPE (TREE_OPERAND (expr, 0)));
+	  return error_mark_node;
+	}
       break;
 
     case COMPOUND_EXPR:

Comment 11 Dodji Seketeli 2009-10-29 21:18:00 UTC
Sent an updated patch to http://gcc.gnu.org/ml/gcc-patches/2009-10/msg01746.html
Comment 12 Dodji Seketeli 2009-11-03 10:44:50 UTC
Subject: Bug 38699

Author: dodji
Date: Tue Nov  3 10:44:36 2009
New Revision: 153843

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153843
Log:
Fix PR c++/38699

gcc/ChangeLog:

	PR c++/38699
	* c-common.c (fold_offsetof_1): Issue errors when the
	member designator of the offsetoff expression is not legitimate.

gcc/testsuite/ChangeLog:

	* c-c++-common/dfp/builtin-offsetof.c: New test.
	* g++.dg/other/offsetof6.C: Likewise.

Added:
    trunk/gcc/testsuite/c-c++-common/dfp/builtin-offsetof.c
    trunk/gcc/testsuite/g++.dg/other/offsetof6.C
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/c-common.c
    trunk/gcc/testsuite/ChangeLog

Comment 13 Dodji Seketeli 2009-11-03 10:46:14 UTC
Subject: Bug 38699

Author: dodji
Date: Tue Nov  3 10:46:00 2009
New Revision: 153844

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153844
Log:
Fix PR c++/38699

gcc/ChangeLog:

	PR c++/38699
	* c-common.c (fold_offsetof_1): Issue errors when the member designator of
	the offsetoff expression is not legitimate.

gcc/testsuite/ChangeLog:

	* c-c++-common/dfp/builtin-offsetof.c: New test.
	* g++.dg/other/offsetof6.C: Likewise.

Added:
    branches/gcc-4_4-branch/gcc/testsuite/c-c++-common/
    branches/gcc-4_4-branch/gcc/testsuite/c-c++-common/dfp/
    branches/gcc-4_4-branch/gcc/testsuite/c-c++-common/dfp/builtin-offsetof.c
    branches/gcc-4_4-branch/gcc/testsuite/g++.dg/other/offsetof6.C
Modified:
    branches/gcc-4_4-branch/gcc/ChangeLog
    branches/gcc-4_4-branch/gcc/c-common.c
    branches/gcc-4_4-branch/gcc/testsuite/ChangeLog

Comment 14 Dodji Seketeli 2009-11-03 13:20:55 UTC
Subject: Bug 38699

Author: dodji
Date: Tue Nov  3 13:20:08 2009
New Revision: 153848

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=153848
Log:
Move builtin-offsetof.c test

	PR c++/38699
	* c-c++-common/dfp/builtin-offsetof.c: Moved to ...
	* c-c++-common/builtin-offsetof.c: ... here.

Added:
    trunk/gcc/testsuite/c-c++-common/builtin-offsetof.c
      - copied, changed from r153846, trunk/gcc/testsuite/c-c++-common/dfp/builtin-offsetof.c
Removed:
    trunk/gcc/testsuite/c-c++-common/dfp/builtin-offsetof.c
Modified:
    trunk/gcc/testsuite/ChangeLog

Comment 15 Volker Reichelt 2009-11-09 21:42:29 UTC
The bug is not a regression, because the code snippet is invalid and was never correctly rejected since the introduction of __builtin_offsetof.
Since it's fixed now we can close it.