diff mbox

PATCH (c,c++) for c++/57793

Message ID 51E1DBE2.3070801@redhat.com
State New
Headers show

Commit Message

Jason Merrill July 13, 2013, 10:59 p.m. UTC 
On 07/10/2013 01:24 AM, Eric Botcazou wrote:
> The idea behind the existing trick is that the reference is within the bounds
> of the base object, i.e. the global offset (offset<<3 + bitpos) is positive,
> but the bitpos part is negative, so we rearrange it into ((offset-c>>3)<<3 +
> (bitpos+c)).  Here the global offset is negative because it has overflowed so
> I'm not sure the rearrangement makes any sense.

I thought the idea of the trick was that when offset<<3 is too big, we 
can use offset as a byte offset and then use bitpos separately in order 
to avoid the overflowing operation.  But I guess I was wrong; we end up 
with a negative number in the assembly output.

So here's a patch that just complains about the class being too big. 
Tested x86_64-pc-linux-gnu, applying to trunk.  Joseph, I'm assuming the 
change makes sense for C as well; let me know if you disagree.

Comments

Kyrylo Tkachov July 17, 2013, 8:50 a.m. UTC | #1 
Hi Jason,

> From: gcc-patches-owner@gcc.gnu.org [mailto:gcc-patches-
> owner@gcc.gnu.org] On Behalf Of Jason Merrill
> Sent: 14 July 2013 00:00
> To: Joseph S. Myers
> Cc: Eric Botcazou; gcc-patches@gcc.gnu.org; Richard Biener
> Subject: PATCH (c,c++) for c++/57793
> 
> On 07/10/2013 01:24 AM, Eric Botcazou wrote:
> > The idea behind the existing trick is that the reference is within the
> bounds
> > of the base object, i.e. the global offset (offset<<3 + bitpos) is
> positive,
> > but the bitpos part is negative, so we rearrange it into ((offset-
> c>>3)<<3 +
> > (bitpos+c)).  Here the global offset is negative because it has
> overflowed so
> > I'm not sure the rearrangement makes any sense.
> 
> I thought the idea of the trick was that when offset<<3 is too big, we
> can use offset as a byte offset and then use bitpos separately in order
> to avoid the overflowing operation.  But I guess I was wrong; we end up
> with a negative number in the assembly output.
> 
> So here's a patch that just complains about the class being too big.
> Tested x86_64-pc-linux-gnu, applying to trunk.  Joseph, I'm assuming the
> change makes sense for C as well; let me know if you disagree.

With this patch we now get PASS->FAIL: gcc.dg/pr42611.c, because the error
message now appears on the line of the definition of the struct. This patch
fixes the testcase.
Ok to apply to trunk?

Thanks,
Kyrill

2013-07-17  Kyrylo Tkachov  <kyrylo.tkachov@arm.com>

	* gcc.dg/pr42611.c: Move dg-error to correct line.
Jason Merrill July 17, 2013, 9:19 p.m. UTC | #2 
On 07/17/2013 04:50 AM, Kyrylo Tkachov wrote:
> With this patch we now get PASS->FAIL: gcc.dg/pr42611.c, because the error
> message now appears on the line of the definition of the struct. This patch
> fixes the testcase.
> Ok to apply to trunk?

Please.

Jason
diff mbox

Patch

commit b3639b7c7ebca9ee1e2f1adf1b9a41451cbfbe22
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jul 12 17:45:59 2013 -0700

    	PR c++/57793
    c/
    	* c-decl.c (finish_struct): Check for too-large class.
    cp/
    	* class.c (layout_class_type): Check for too-large class.

diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 8170a80..f7ae648 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -7210,6 +7210,12 @@  finish_struct (location_t loc, tree t, tree fieldlist, tree attributes,
 
   layout_type (t);
 
+  if (TYPE_SIZE_UNIT (t)
+      && TREE_CODE (TYPE_SIZE_UNIT (t)) == INTEGER_CST
+      && !TREE_OVERFLOW (TYPE_SIZE_UNIT (t))
+      && !valid_constant_size_p (TYPE_SIZE_UNIT (t)))
+    error ("type %qT is too large", t);
+
   /* Give bit-fields their proper types.  */
   {
     tree *fieldlistp = &fieldlist;
diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index e516632..45652a6 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -6237,6 +6237,12 @@  layout_class_type (tree t, tree *virtuals_p)
   /* Let the back end lay out the type.  */
   finish_record_layout (rli, /*free_p=*/true);
 
+  if (TYPE_SIZE_UNIT (t)
+      && TREE_CODE (TYPE_SIZE_UNIT (t)) == INTEGER_CST
+      && !TREE_OVERFLOW (TYPE_SIZE_UNIT (t))
+      && !valid_constant_size_p (TYPE_SIZE_UNIT (t)))
+    error ("type %qT is too large", t);
+
   /* Warn about bases that can't be talked about due to ambiguity.  */
   warn_about_ambiguous_bases (t);
 
diff --git a/gcc/testsuite/c-c++-common/pr57793.c b/gcc/testsuite/c-c++-common/pr57793.c
new file mode 100644
index 0000000..d66fada
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/pr57793.c
@@ -0,0 +1,23 @@ 
+/* PR c++/57793 */
+
+struct A { unsigned a : 1; unsigned b : 1; };
+struct B     /* { dg-error "type .B. is too large" "" { target { c++ && ilp32 } } } */
+{
+  unsigned char c[0x40000000];
+  unsigned char d[0x40000ff0];
+  struct A e;
+}; /* { dg-error "type .struct B. is too large" "" { target { c && ilp32 } } } */
+
+void *foo (struct B *p)
+{
+  if (p->e.a)
+    return (void *) 0;
+  p->e.b = 1;
+  return p->c;
+}
+
+void
+bar (struct B *p)
+{
+  foo (p);
+}