* reject decl with incomplete struct/union type in check_global_declaration()
@ 2016-01-14 16:34 Prathamesh Kulkarni
2016-01-14 21:57 ` Joseph Myers
0 siblings, 1 reply; 10+ messages in thread
From: Prathamesh Kulkarni @ 2016-01-14 16:34 UTC (permalink / raw)
To: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 1148 bytes --]
Hi,
For test-case containing only the following declaration:
static struct undefined_struct object;
gcc rejects it at -O0 in assemble_variable() with error "storage size
of <var> is unknown",
however no error is reported when compiled with -O2.
AFAIU that happens because at -O2, analyze_function() removes the
symbol "object" from symbol table and assemble_variable() has no
chance to process it.
g++ rejects it during parsing. I tried similarly in C FE by adding a
check for decl with incomplete struct/union type in finish_decl(),
however that fails to compile the following case:
typedef struct foo foo_t;
foo_t x;
struct foo { int i; };
g++ rejects the above case as well but gcc accepts it.
Do C and C++ standards differ in this regard ?
So instead of finish_decl(), I added check for incomplete struct/union
in check_global_declaration(),
which is called by analyze_function() before removing unused nodes.
The patch regresses Wcxx-compat-8.c and declspecs-1.c because the error
"storage size of <var> is unknown" shows up in these test-cases. I
modified the test-cases
to accept that error.
Does the patch look OK ?
Thank you,
Prathamesh
[-- Attachment #2: patch.diff --]
[-- Type: text/plain, Size: 2481 bytes --]
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 950f6c5..1560a78 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -974,6 +974,12 @@ check_global_declaration (symtab_node *snode)
? OPT_Wunused_const_variable
: OPT_Wunused_variable),
"%qD defined but not used", decl);
+
+ if (VAR_P (decl) && !DECL_EXTERNAL (decl)
+ && RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl)) && DECL_SIZE (decl) == 0)
+ {
+ error ("storage size of %q+D isn%'t known", decl);
+ }
}
/* Discover all functions and variables that are trivially needed, analyze
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
index f7e8c55..4e9ddc1 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
@@ -33,6 +33,7 @@ enum e3
__typeof__ (struct s5 { int i; }) v5; /* { dg-warning "invalid in C\[+\]\[+\]" } */
__typeof__ (struct t5) w5; /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+ /* { dg-error "storage size of 'w5' isn't known" "" { target *-*-* } 35 } */
int
f1 (struct s1 *p)
@@ -64,4 +65,4 @@ f5 ()
return &((struct t8) { }); /* { dg-warning "invalid in C\[+\]\[+\]" } */
}
-/* { dg-error "invalid use of undefined type" "" { target *-*-* } 64 } */
+/* { dg-error "invalid use of undefined type" "" { target *-*-* } 65 } */
diff --git a/gcc/testsuite/gcc.dg/declspec-1.c b/gcc/testsuite/gcc.dg/declspec-1.c
index c19f107..9113076 100644
--- a/gcc/testsuite/gcc.dg/declspec-1.c
+++ b/gcc/testsuite/gcc.dg/declspec-1.c
@@ -9,7 +9,8 @@ typedef int t;
/* These should all be diagnosed, but only once, not for every
identifier declared. */
struct s0 int x0, /* { dg-error "two or more data types" } */
-x1;
+/* { dg-error "storage size of 'x0' isn't known" "" { target *-*-* } 11 } */
+x1; /* { dg-error "storage size of 'x1' isn't known" } */
char union u0 x2, /* { dg-error "two or more data types" } */
x3;
diff --git a/gcc/testsuite/gcc.dg/struct-incompl-2.c b/gcc/testsuite/gcc.dg/struct-incompl-2.c
new file mode 100644
index 0000000..f3957eb
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/struct-incompl-2.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-O2" } */
+
+static struct foo x; /* { dg-error "storage size of 'x' isn't known" } */
+static union bar y; /* { dg-error "storage size of 'y' isn't known" } */
+
+typedef struct P p;
+static p p_obj; /* { dg-error "storage size of 'p_obj' isn't known" } */
+
+extern struct undefined_object object;
[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 337 bytes --]
2016-01-14 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
gcc/
* cgraphunit.c (check_global_declaration): Check for decl having
incomplete struct/union type.
testsuite/
* gcc.dg/struct-incompl-2.c: New test.
* gcc.dg/Wcxx-compat-8.c: Adjust to accept error due to
incomplete struct type.
* gcc.dg/declspec-1.c: Likewise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-14 16:34 reject decl with incomplete struct/union type in check_global_declaration() Prathamesh Kulkarni
@ 2016-01-14 21:57 ` Joseph Myers
2016-01-14 22:08 ` Nathan Sidwell
2016-01-15 13:52 ` Prathamesh Kulkarni
0 siblings, 2 replies; 10+ messages in thread
From: Joseph Myers @ 2016-01-14 21:57 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: gcc Patches
On Thu, 14 Jan 2016, Prathamesh Kulkarni wrote:
> Hi,
> For test-case containing only the following declaration:
> static struct undefined_struct object;
> gcc rejects it at -O0 in assemble_variable() with error "storage size
> of <var> is unknown",
> however no error is reported when compiled with -O2.
Cf bug 24293 (for the -fsyntax-only case) - does this patch fix that?
> g++ rejects it during parsing. I tried similarly in C FE by adding a
> check for decl with incomplete struct/union type in finish_decl(),
> however that fails to compile the following case:
> typedef struct foo foo_t;
> foo_t x;
> struct foo { int i; };
> g++ rejects the above case as well but gcc accepts it.
> Do C and C++ standards differ in this regard ?
I don't know about C++, but this sort of thing is valid C if the type is
complete at the end of the translation unit or is an incomplete array type
(but not in the case where the variable is static - such a case with
static, if supported, is an extension).
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-14 21:57 ` Joseph Myers
@ 2016-01-14 22:08 ` Nathan Sidwell
2016-01-15 13:52 ` Prathamesh Kulkarni
1 sibling, 0 replies; 10+ messages in thread
From: Nathan Sidwell @ 2016-01-14 22:08 UTC (permalink / raw)
To: Joseph Myers, Prathamesh Kulkarni; +Cc: gcc Patches
On 01/14/16 16:57, Joseph Myers wrote:
> On Thu, 14 Jan 2016, Prathamesh Kulkarni wrote:
>> g++ rejects it during parsing. I tried similarly in C FE by adding a
>> check for decl with incomplete struct/union type in finish_decl(),
>> however that fails to compile the following case:
>> typedef struct foo foo_t;
>> foo_t x;
>> struct foo { int i; };
>> g++ rejects the above case as well but gcc accepts it.
>> Do C and C++ standards differ in this regard ?
>
> I don't know about C++, but this sort of thing is valid C if the type is
> complete at the end of the translation unit or is an incomplete array type
> (but not in the case where the variable is static - such a case with
> static, if supported, is an extension).
It's ill-formed C++.
7.1.1/8 allows you to use a declared but undefined struct with 'extern', but not
without it.
'The name of a declared but undefined class can be used in an extern
declaration. Such a declaration can only be used in ways that do not require a
complete class type'
(Although it doesn't seem to explicitly say that such a name can be used without
an 'extern', the implication is that it cannot).
nathan
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-14 21:57 ` Joseph Myers
2016-01-14 22:08 ` Nathan Sidwell
@ 2016-01-15 13:52 ` Prathamesh Kulkarni
2016-01-15 21:26 ` Joseph Myers
1 sibling, 1 reply; 10+ messages in thread
From: Prathamesh Kulkarni @ 2016-01-15 13:52 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 1811 bytes --]
On 15 January 2016 at 03:27, Joseph Myers <joseph@codesourcery.com> wrote:
> On Thu, 14 Jan 2016, Prathamesh Kulkarni wrote:
>
>> Hi,
>> For test-case containing only the following declaration:
>> static struct undefined_struct object;
>> gcc rejects it at -O0 in assemble_variable() with error "storage size
>> of <var> is unknown",
>> however no error is reported when compiled with -O2.
>
> Cf bug 24293 (for the -fsyntax-only case) - does this patch fix that?
Ah this doesn't fix PR24293, it seems analyze_function() doesn't get
called for -fsyntax-only.
I don't have a good solution for this. I assume varpool won't be
populated for -fsyntax-only ?
And we need to walk over decls with incomplete struct/union types
after parsing the whole translation unit.
In the attached patch, I kept a global vec<tree> incomplete_record_decls;
In finish_decl(), if the decl is static, has type struct/union and
size 0 then it is appened to incomplete_record_decls.
In c_parser_translation_unit(), iterate over incomplete_record_decls
and if report error if any decl has size zero.
The patch passes testsuite.
Thanks,
Prathamesh
>
>> g++ rejects it during parsing. I tried similarly in C FE by adding a
>> check for decl with incomplete struct/union type in finish_decl(),
>> however that fails to compile the following case:
>> typedef struct foo foo_t;
>> foo_t x;
>> struct foo { int i; };
>> g++ rejects the above case as well but gcc accepts it.
>> Do C and C++ standards differ in this regard ?
>
> I don't know about C++, but this sort of thing is valid C if the type is
> complete at the end of the translation unit or is an incomplete array type
> (but not in the case where the variable is static - such a case with
> static, if supported, is an extension).
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
[-- Attachment #2: patch-2.diff --]
[-- Type: text/plain, Size: 3535 bytes --]
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 915376d..ac542e1 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4791,6 +4791,12 @@ finish_decl (tree decl, location_t init_loc, tree init,
TREE_TYPE (decl) = error_mark_node;
}
+ if (RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
+ && DECL_SIZE (decl) == 0 && TREE_STATIC (decl))
+ {
+ incomplete_record_decls.safe_push (decl);
+ }
+
if (is_global_var (decl) && DECL_SIZE (decl) != 0)
{
if (TREE_CODE (DECL_SIZE (decl)) == INTEGER_CST)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a0e0052..8af04f7 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -59,6 +59,8 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-expr.h"
#include "context.h"
+vec<tree> incomplete_record_decls = vNULL;
+
void
set_c_expr_source_range (c_expr *expr,
location_t start, location_t finish)
@@ -1421,6 +1423,13 @@ c_parser_translation_unit (c_parser *parser)
}
while (c_parser_next_token_is_not (parser, CPP_EOF));
}
+
+ for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
+ {
+ tree decl = incomplete_record_decls[i];
+ if (DECL_SIZE (decl) == 0)
+ error ("storage size of %q+D isn%'t known", decl);
+ }
}
/* Parse an external declaration (C90 6.7, C99 6.9).
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 81a3d58..cf79ba7 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -731,4 +731,6 @@ set_c_expr_source_range (c_expr *expr,
/* In c-fold.c */
extern tree decl_constant_value_for_optimization (tree);
+extern vec<tree> incomplete_record_decls;
+
#endif /* ! GCC_C_TREE_H */
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
index f7e8c55..4e9ddc1 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
@@ -33,6 +33,7 @@ enum e3
__typeof__ (struct s5 { int i; }) v5; /* { dg-warning "invalid in C\[+\]\[+\]" } */
__typeof__ (struct t5) w5; /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+ /* { dg-error "storage size of 'w5' isn't known" "" { target *-*-* } 35 } */
int
f1 (struct s1 *p)
@@ -64,4 +65,4 @@ f5 ()
return &((struct t8) { }); /* { dg-warning "invalid in C\[+\]\[+\]" } */
}
-/* { dg-error "invalid use of undefined type" "" { target *-*-* } 64 } */
+/* { dg-error "invalid use of undefined type" "" { target *-*-* } 65 } */
diff --git a/gcc/testsuite/gcc.dg/declspec-1.c b/gcc/testsuite/gcc.dg/declspec-1.c
index c19f107..9113076 100644
--- a/gcc/testsuite/gcc.dg/declspec-1.c
+++ b/gcc/testsuite/gcc.dg/declspec-1.c
@@ -9,7 +9,8 @@ typedef int t;
/* These should all be diagnosed, but only once, not for every
identifier declared. */
struct s0 int x0, /* { dg-error "two or more data types" } */
-x1;
+/* { dg-error "storage size of 'x0' isn't known" "" { target *-*-* } 11 } */
+x1; /* { dg-error "storage size of 'x1' isn't known" } */
char union u0 x2, /* { dg-error "two or more data types" } */
x3;
diff --git a/gcc/testsuite/gcc.dg/pr24293.c b/gcc/testsuite/gcc.dg/pr24293.c
new file mode 100644
index 0000000..33285e7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr24293.c
@@ -0,0 +1,10 @@
+/* { dg-do compile } */
+/* { dg-options "-fsyntax-only" } */
+
+static struct foo x; /* { dg-error "storage size of 'x' isn't known" } */
+static union bar y; /* { dg-error "storage size of 'y' isn't known" } */
+
+typedef struct P p;
+static p p_obj; /* { dg-error "storage size of 'p_obj' isn't known" } */
+
+extern struct undefined_object object;
[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 560 bytes --]
2016-01-15 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
PR c/24293
* c-tree.h (incomplete_record_decls): Declare.
* c-parser.c (incomplete_record_decls): Define.
(c_parser_translation_unit): Iterate through incomplete_record_decls and
report error if any decl has zero size.
* c-decl.c (finish_decl): Append static decl with incomplete struct/union
type to incomplete_record_decls.
testsuite/
* gcc.dg/pr24293.c: New test.
* gcc.dg/Wcxx-compat-8.c: Adjust to accept error due to
incomplete struct type.
* gcc.dg/declspec-1.c: Likewise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-15 13:52 ` Prathamesh Kulkarni
@ 2016-01-15 21:26 ` Joseph Myers
2016-01-16 10:15 ` Prathamesh Kulkarni
0 siblings, 1 reply; 10+ messages in thread
From: Joseph Myers @ 2016-01-15 21:26 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: gcc Patches
On Fri, 15 Jan 2016, Prathamesh Kulkarni wrote:
> On 15 January 2016 at 03:27, Joseph Myers <joseph@codesourcery.com> wrote:
> > On Thu, 14 Jan 2016, Prathamesh Kulkarni wrote:
> >
> >> Hi,
> >> For test-case containing only the following declaration:
> >> static struct undefined_struct object;
> >> gcc rejects it at -O0 in assemble_variable() with error "storage size
> >> of <var> is unknown",
> >> however no error is reported when compiled with -O2.
> >
> > Cf bug 24293 (for the -fsyntax-only case) - does this patch fix that?
> Ah this doesn't fix PR24293, it seems analyze_function() doesn't get
> called for -fsyntax-only.
> I don't have a good solution for this. I assume varpool won't be
> populated for -fsyntax-only ?
> And we need to walk over decls with incomplete struct/union types
> after parsing the whole translation unit.
> In the attached patch, I kept a global vec<tree> incomplete_record_decls;
> In finish_decl(), if the decl is static, has type struct/union and
> size 0 then it is appened to incomplete_record_decls.
> In c_parser_translation_unit(), iterate over incomplete_record_decls
> and if report error if any decl has size zero.
> The patch passes testsuite.
There's a GNU C extension allowing forward declarations of enums, and it
seems that
static enum e x;
doesn't get diagnosed either with -fsyntax-only. Thus I think you should
cover that case as well.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-15 21:26 ` Joseph Myers
@ 2016-01-16 10:15 ` Prathamesh Kulkarni
2016-01-19 0:17 ` Joseph Myers
2016-01-19 11:19 ` Marek Polacek
0 siblings, 2 replies; 10+ messages in thread
From: Prathamesh Kulkarni @ 2016-01-16 10:15 UTC (permalink / raw)
To: Joseph Myers; +Cc: gcc Patches
[-- Attachment #1: Type: text/plain, Size: 1729 bytes --]
On 16 January 2016 at 02:56, Joseph Myers <joseph@codesourcery.com> wrote:
> On Fri, 15 Jan 2016, Prathamesh Kulkarni wrote:
>
>> On 15 January 2016 at 03:27, Joseph Myers <joseph@codesourcery.com> wrote:
>> > On Thu, 14 Jan 2016, Prathamesh Kulkarni wrote:
>> >
>> >> Hi,
>> >> For test-case containing only the following declaration:
>> >> static struct undefined_struct object;
>> >> gcc rejects it at -O0 in assemble_variable() with error "storage size
>> >> of <var> is unknown",
>> >> however no error is reported when compiled with -O2.
>> >
>> > Cf bug 24293 (for the -fsyntax-only case) - does this patch fix that?
>> Ah this doesn't fix PR24293, it seems analyze_function() doesn't get
>> called for -fsyntax-only.
>> I don't have a good solution for this. I assume varpool won't be
>> populated for -fsyntax-only ?
>> And we need to walk over decls with incomplete struct/union types
>> after parsing the whole translation unit.
>> In the attached patch, I kept a global vec<tree> incomplete_record_decls;
>> In finish_decl(), if the decl is static, has type struct/union and
>> size 0 then it is appened to incomplete_record_decls.
>> In c_parser_translation_unit(), iterate over incomplete_record_decls
>> and if report error if any decl has size zero.
>> The patch passes testsuite.
>
> There's a GNU C extension allowing forward declarations of enums, and it
> seems that
>
> static enum e x;
>
> doesn't get diagnosed either with -fsyntax-only. Thus I think you should
> cover that case as well.
Done in the attached patch. It regresses pr63549.c because the error
showed up there, adjusted
the test-case to accept the error.
OK to commit ?
Thanks,
Prathamesh
>
> --
> Joseph S. Myers
> joseph@codesourcery.com
[-- Attachment #2: patch-3.diff --]
[-- Type: text/plain, Size: 4425 bytes --]
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 915376d..d36fc67 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4791,6 +4791,13 @@ finish_decl (tree decl, location_t init_loc, tree init,
TREE_TYPE (decl) = error_mark_node;
}
+ if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
+ || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE)
+ && DECL_SIZE (decl) == 0 && TREE_STATIC (decl))
+ {
+ incomplete_record_decls.safe_push (decl);
+ }
+
if (is_global_var (decl) && DECL_SIZE (decl) != 0)
{
if (TREE_CODE (DECL_SIZE (decl)) == INTEGER_CST)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index a0e0052..3c8a496 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -59,6 +59,8 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-expr.h"
#include "context.h"
+vec<tree> incomplete_record_decls = vNULL;
+
void
set_c_expr_source_range (c_expr *expr,
location_t start, location_t finish)
@@ -1421,6 +1423,16 @@ c_parser_translation_unit (c_parser *parser)
}
while (c_parser_next_token_is_not (parser, CPP_EOF));
}
+
+ for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
+ {
+ tree decl = incomplete_record_decls[i];
+ if (DECL_SIZE (decl) == 0 && TREE_TYPE (decl) != error_mark_node)
+ {
+ error ("storage size of %q+D isn%'t known", decl);
+ TREE_TYPE (decl) = error_mark_node;
+ }
+ }
}
/* Parse an external declaration (C90 6.7, C99 6.9).
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 81a3d58..cf79ba7 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -731,4 +731,6 @@ set_c_expr_source_range (c_expr *expr,
/* In c-fold.c */
extern tree decl_constant_value_for_optimization (tree);
+extern vec<tree> incomplete_record_decls;
+
#endif /* ! GCC_C_TREE_H */
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
index f7e8c55..4e9ddc1 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
@@ -33,6 +33,7 @@ enum e3
__typeof__ (struct s5 { int i; }) v5; /* { dg-warning "invalid in C\[+\]\[+\]" } */
__typeof__ (struct t5) w5; /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+ /* { dg-error "storage size of 'w5' isn't known" "" { target *-*-* } 35 } */
int
f1 (struct s1 *p)
@@ -64,4 +65,4 @@ f5 ()
return &((struct t8) { }); /* { dg-warning "invalid in C\[+\]\[+\]" } */
}
-/* { dg-error "invalid use of undefined type" "" { target *-*-* } 64 } */
+/* { dg-error "invalid use of undefined type" "" { target *-*-* } 65 } */
diff --git a/gcc/testsuite/gcc.dg/declspec-1.c b/gcc/testsuite/gcc.dg/declspec-1.c
index c19f107..b024601 100644
--- a/gcc/testsuite/gcc.dg/declspec-1.c
+++ b/gcc/testsuite/gcc.dg/declspec-1.c
@@ -9,13 +9,15 @@ typedef int t;
/* These should all be diagnosed, but only once, not for every
identifier declared. */
struct s0 int x0, /* { dg-error "two or more data types" } */
-x1;
+/* { dg-error "storage size of 'x0' isn't known" "" { target *-*-* } 11 } */
+x1; /* { dg-error "storage size of 'x1' isn't known" } */
char union u0 x2, /* { dg-error "two or more data types" } */
x3;
enum e0 struct s1 x4, /* { dg-error "two or more data types" } */
-x5;
+ /* { dg-error "storage size of 'x4' isn't known" "" { target *-*-* } 18 } */
+x5; /* { dg-error "storage size of 'x5' isn't known" } */
short short x6, /* { dg-error "duplicate" } */
x7;
diff --git a/gcc/testsuite/gcc.dg/pr24293.c b/gcc/testsuite/gcc.dg/pr24293.c
new file mode 100644
index 0000000..5bf7ad17
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr24293.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fsyntax-only" } */
+
+static struct foo x; /* { dg-error "storage size of 'x' isn't known" } */
+static union bar y; /* { dg-error "storage size of 'y' isn't known" } */
+
+typedef struct P p;
+static p p_obj; /* { dg-error "storage size of 'p_obj' isn't known" } */
+
+static enum e e_var; /* { dg-error "storage size of 'e_var' isn't known" } */
+
+extern struct undefined_object object;
diff --git a/gcc/testsuite/gcc.dg/pr63549.c b/gcc/testsuite/gcc.dg/pr63549.c
index c9b1718..bd0b706 100644
--- a/gcc/testsuite/gcc.dg/pr63549.c
+++ b/gcc/testsuite/gcc.dg/pr63549.c
@@ -2,6 +2,6 @@
/* { dg-do compile } */
/* { dg-options "" } */
-enum E e;
+enum E e; /* { dg-error "storage size of 'e' isn't known" } */
int a[10];
int i = a[e]; /* { dg-error "has an incomplete type" } */
[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 599 bytes --]
2016-01-15 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
PR c/24293
* c-tree.h (incomplete_record_decls): Declare.
* c-parser.c (incomplete_record_decls): Define.
(c_parser_translation_unit): Iterate through incomplete_record_decls and
report error if any decl has zero size.
* c-decl.c (finish_decl): Append static decl with incomplete struct/union
or enum type to incomplete_record_decls.
testsuite/
* gcc.dg/pr24293.c: New test.
* gcc.dg/Wcxx-compat-8.c: Adjust to accept error due to
incomplete struct type.
* gcc.dg/declspec-1.c: Likewise.
* gcc.dg/pr63549.c: Likewise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-16 10:15 ` Prathamesh Kulkarni
@ 2016-01-19 0:17 ` Joseph Myers
2016-01-19 11:19 ` Marek Polacek
1 sibling, 0 replies; 10+ messages in thread
From: Joseph Myers @ 2016-01-19 0:17 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: gcc Patches
On Sat, 16 Jan 2016, Prathamesh Kulkarni wrote:
> > There's a GNU C extension allowing forward declarations of enums, and it
> > seems that
> >
> > static enum e x;
> >
> > doesn't get diagnosed either with -fsyntax-only. Thus I think you should
> > cover that case as well.
> Done in the attached patch. It regresses pr63549.c because the error
> showed up there, adjusted
> the test-case to accept the error.
> OK to commit ?
OK.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-16 10:15 ` Prathamesh Kulkarni
2016-01-19 0:17 ` Joseph Myers
@ 2016-01-19 11:19 ` Marek Polacek
2016-01-20 13:22 ` Prathamesh Kulkarni
1 sibling, 1 reply; 10+ messages in thread
From: Marek Polacek @ 2016-01-19 11:19 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: Joseph Myers, gcc Patches
Sorry for speaking up late, but I think we could do better with formatting
in this patch:
On Sat, Jan 16, 2016 at 03:45:22PM +0530, Prathamesh Kulkarni wrote:
> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
> index 915376d..d36fc67 100644
> --- a/gcc/c/c-decl.c
> +++ b/gcc/c/c-decl.c
> @@ -4791,6 +4791,13 @@ finish_decl (tree decl, location_t init_loc, tree init,
> TREE_TYPE (decl) = error_mark_node;
> }
>
> + if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
> + || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE)
> + && DECL_SIZE (decl) == 0 && TREE_STATIC (decl))
DECL_SIZE yields a tree, so I'd rather see NULL_TREE instead of 0 here (yeah,
the enclosing code uses 0s :(). The "&& TREE_STATIC..." should be on its own
line.
> + {
> + incomplete_record_decls.safe_push (decl);
> + }
> +
Redundant braces.
> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
> index a0e0052..3c8a496 100644
> --- a/gcc/c/c-parser.c
> +++ b/gcc/c/c-parser.c
> @@ -59,6 +59,8 @@ along with GCC; see the file COPYING3. If not see
> #include "gimple-expr.h"
> #include "context.h"
>
> +vec<tree> incomplete_record_decls = vNULL;
This could use a comment.
> +
> + for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
> + {
> + tree decl = incomplete_record_decls[i];
> + if (DECL_SIZE (decl) == 0 && TREE_TYPE (decl) != error_mark_node)
I'd s/0/NULL_TREE/.
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-19 11:19 ` Marek Polacek
@ 2016-01-20 13:22 ` Prathamesh Kulkarni
2016-01-20 15:51 ` Marek Polacek
0 siblings, 1 reply; 10+ messages in thread
From: Prathamesh Kulkarni @ 2016-01-20 13:22 UTC (permalink / raw)
To: Marek Polacek; +Cc: Joseph Myers, gcc Patches
[-- Attachment #1: Type: text/plain, Size: 1686 bytes --]
On 19 January 2016 at 16:49, Marek Polacek <polacek@redhat.com> wrote:
> Sorry for speaking up late, but I think we could do better with formatting
> in this patch:
>
> On Sat, Jan 16, 2016 at 03:45:22PM +0530, Prathamesh Kulkarni wrote:
>> diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
>> index 915376d..d36fc67 100644
>> --- a/gcc/c/c-decl.c
>> +++ b/gcc/c/c-decl.c
>> @@ -4791,6 +4791,13 @@ finish_decl (tree decl, location_t init_loc, tree init,
>> TREE_TYPE (decl) = error_mark_node;
>> }
>>
>> + if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
>> + || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE)
>> + && DECL_SIZE (decl) == 0 && TREE_STATIC (decl))
>
> DECL_SIZE yields a tree, so I'd rather see NULL_TREE instead of 0 here (yeah,
> the enclosing code uses 0s :(). The "&& TREE_STATIC..." should be on its own
> line.
>
>> + {
>> + incomplete_record_decls.safe_push (decl);
>> + }
>> +
>
> Redundant braces.
>
>> diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
>> index a0e0052..3c8a496 100644
>> --- a/gcc/c/c-parser.c
>> +++ b/gcc/c/c-parser.c
>> @@ -59,6 +59,8 @@ along with GCC; see the file COPYING3. If not see
>> #include "gimple-expr.h"
>> #include "context.h"
>>
>> +vec<tree> incomplete_record_decls = vNULL;
>
> This could use a comment.
>
>> +
>> + for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
>> + {
>> + tree decl = incomplete_record_decls[i];
>> + if (DECL_SIZE (decl) == 0 && TREE_TYPE (decl) != error_mark_node)
>
> I'd s/0/NULL_TREE/.
Thanks for the review, I have done the suggested changes in this
version of the patch.
Ok for trunk ?
Thanks,
Prathamesh
>
> Marek
[-- Attachment #2: patch-4.diff --]
[-- Type: text/plain, Size: 4829 bytes --]
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 5830e22..1ec6042 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -4791,6 +4791,12 @@ finish_decl (tree decl, location_t init_loc, tree init,
TREE_TYPE (decl) = error_mark_node;
}
+ if ((RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
+ || TREE_CODE (TREE_TYPE (decl)) == ENUMERAL_TYPE)
+ && DECL_SIZE (decl) == NULL_TREE
+ && TREE_STATIC (decl))
+ incomplete_record_decls.safe_push (decl);
+
if (is_global_var (decl) && DECL_SIZE (decl) != 0)
{
if (TREE_CODE (DECL_SIZE (decl)) == INTEGER_CST)
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 919680a..1d3b9e1 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -59,6 +59,15 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-expr.h"
#include "context.h"
+/* We need to walk over decls with incomplete struct/union/enum types
+ after parsing the whole translation unit.
+ In finish_decl(), if the decl is static, has incomplete
+ struct/union/enum type, it is appened to incomplete_record_decls.
+ In c_parser_translation_unit(), we iterate over incomplete_record_decls
+ and report error if any of the decls are still incomplete. */
+
+vec<tree> incomplete_record_decls = vNULL;
+
void
set_c_expr_source_range (c_expr *expr,
location_t start, location_t finish)
@@ -1421,6 +1430,16 @@ c_parser_translation_unit (c_parser *parser)
}
while (c_parser_next_token_is_not (parser, CPP_EOF));
}
+
+ for (unsigned i = 0; i < incomplete_record_decls.length (); ++i)
+ {
+ tree decl = incomplete_record_decls[i];
+ if (DECL_SIZE (decl) == NULL_TREE && TREE_TYPE (decl) != error_mark_node)
+ {
+ error ("storage size of %q+D isn%'t known", decl);
+ TREE_TYPE (decl) = error_mark_node;
+ }
+ }
}
/* Parse an external declaration (C90 6.7, C99 6.9).
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index 81a3d58..cf79ba7 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -731,4 +731,6 @@ set_c_expr_source_range (c_expr *expr,
/* In c-fold.c */
extern tree decl_constant_value_for_optimization (tree);
+extern vec<tree> incomplete_record_decls;
+
#endif /* ! GCC_C_TREE_H */
diff --git a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
index f7e8c55..4e9ddc1 100644
--- a/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
+++ b/gcc/testsuite/gcc.dg/Wcxx-compat-8.c
@@ -33,6 +33,7 @@ enum e3
__typeof__ (struct s5 { int i; }) v5; /* { dg-warning "invalid in C\[+\]\[+\]" } */
__typeof__ (struct t5) w5; /* { dg-bogus "invalid in C\[+\]\[+\]" } */
+ /* { dg-error "storage size of 'w5' isn't known" "" { target *-*-* } 35 } */
int
f1 (struct s1 *p)
@@ -64,4 +65,4 @@ f5 ()
return &((struct t8) { }); /* { dg-warning "invalid in C\[+\]\[+\]" } */
}
-/* { dg-error "invalid use of undefined type" "" { target *-*-* } 64 } */
+/* { dg-error "invalid use of undefined type" "" { target *-*-* } 65 } */
diff --git a/gcc/testsuite/gcc.dg/declspec-1.c b/gcc/testsuite/gcc.dg/declspec-1.c
index c19f107..b024601 100644
--- a/gcc/testsuite/gcc.dg/declspec-1.c
+++ b/gcc/testsuite/gcc.dg/declspec-1.c
@@ -9,13 +9,15 @@ typedef int t;
/* These should all be diagnosed, but only once, not for every
identifier declared. */
struct s0 int x0, /* { dg-error "two or more data types" } */
-x1;
+/* { dg-error "storage size of 'x0' isn't known" "" { target *-*-* } 11 } */
+x1; /* { dg-error "storage size of 'x1' isn't known" } */
char union u0 x2, /* { dg-error "two or more data types" } */
x3;
enum e0 struct s1 x4, /* { dg-error "two or more data types" } */
-x5;
+ /* { dg-error "storage size of 'x4' isn't known" "" { target *-*-* } 18 } */
+x5; /* { dg-error "storage size of 'x5' isn't known" } */
short short x6, /* { dg-error "duplicate" } */
x7;
diff --git a/gcc/testsuite/gcc.dg/pr24293.c b/gcc/testsuite/gcc.dg/pr24293.c
new file mode 100644
index 0000000..5bf7ad17
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr24293.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-fsyntax-only" } */
+
+static struct foo x; /* { dg-error "storage size of 'x' isn't known" } */
+static union bar y; /* { dg-error "storage size of 'y' isn't known" } */
+
+typedef struct P p;
+static p p_obj; /* { dg-error "storage size of 'p_obj' isn't known" } */
+
+static enum e e_var; /* { dg-error "storage size of 'e_var' isn't known" } */
+
+extern struct undefined_object object;
diff --git a/gcc/testsuite/gcc.dg/pr63549.c b/gcc/testsuite/gcc.dg/pr63549.c
index c9b1718..bd0b706 100644
--- a/gcc/testsuite/gcc.dg/pr63549.c
+++ b/gcc/testsuite/gcc.dg/pr63549.c
@@ -2,6 +2,6 @@
/* { dg-do compile } */
/* { dg-options "" } */
-enum E e;
+enum E e; /* { dg-error "storage size of 'e' isn't known" } */
int a[10];
int i = a[e]; /* { dg-error "has an incomplete type" } */
[-- Attachment #3: ChangeLog --]
[-- Type: application/octet-stream, Size: 599 bytes --]
2016-01-15 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org>
PR c/24293
* c-tree.h (incomplete_record_decls): Declare.
* c-parser.c (incomplete_record_decls): Define.
(c_parser_translation_unit): Iterate through incomplete_record_decls and
report error if any decl has zero size.
* c-decl.c (finish_decl): Append static decl with incomplete struct/union
or enum type to incomplete_record_decls.
testsuite/
* gcc.dg/pr24293.c: New test.
* gcc.dg/Wcxx-compat-8.c: Adjust to accept error due to
incomplete struct type.
* gcc.dg/declspec-1.c: Likewise.
* gcc.dg/pr63549.c: Likewise.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: reject decl with incomplete struct/union type in check_global_declaration()
2016-01-20 13:22 ` Prathamesh Kulkarni
@ 2016-01-20 15:51 ` Marek Polacek
0 siblings, 0 replies; 10+ messages in thread
From: Marek Polacek @ 2016-01-20 15:51 UTC (permalink / raw)
To: Prathamesh Kulkarni; +Cc: Joseph Myers, gcc Patches
On Wed, Jan 20, 2016 at 06:52:48PM +0530, Prathamesh Kulkarni wrote:
> Thanks for the review, I have done the suggested changes in this
> version of the patch.
> Ok for trunk ?
Given that Joseph already approved substance of the patch, this is ok
(but you might want to correct a typo below), thanks.
> +/* We need to walk over decls with incomplete struct/union/enum types
> + after parsing the whole translation unit.
> + In finish_decl(), if the decl is static, has incomplete
> + struct/union/enum type, it is appened to incomplete_record_decls.
"appended"
> + In c_parser_translation_unit(), we iterate over incomplete_record_decls
> + and report error if any of the decls are still incomplete. */
Marek
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-01-20 15:51 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-14 16:34 reject decl with incomplete struct/union type in check_global_declaration() Prathamesh Kulkarni
2016-01-14 21:57 ` Joseph Myers
2016-01-14 22:08 ` Nathan Sidwell
2016-01-15 13:52 ` Prathamesh Kulkarni
2016-01-15 21:26 ` Joseph Myers
2016-01-16 10:15 ` Prathamesh Kulkarni
2016-01-19 0:17 ` Joseph Myers
2016-01-19 11:19 ` Marek Polacek
2016-01-20 13:22 ` Prathamesh Kulkarni
2016-01-20 15:51 ` Marek Polacek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).