* 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).