| Message ID | CAAgBjMn42wpBL4nvrDVhCo8B0B70V4wo5BKEH6JBF9_eSPYdkw@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
On Wed, 15 Feb 2017, Prathamesh Kulkarni wrote: > Hi, > For the following (invalid) test-case: > > void __GIMPLE () foo (int a) > { > int t0; > int _1; > _1 = a; > t0_1 = a; > } > > results in following ICE: > gimplefe-error-4.c: In function ‘foo’: > gimplefe-error-4.c:20:1: error: SSA_NAME_DEF_STMT is wrong > } > ^ > Expected definition statement: > _1 = a_2(D); > > Actual definition statement: > _1 = a_2(D); > gimplefe-error-4.c:20:1: internal compiler error: verify_ssa failed > 0xe1458b verify_ssa(bool, bool) > ../../gcc/gcc/tree-ssa.c:1184 > 0xb0d1ed execute_function_todo > ../../gcc/gcc/passes.c:1973 > 0xb0dad5 execute_todo > ../../gcc/gcc/passes.c:2016 > > The reason for ICE is that in c_parser_parse_ssa_name, ssa_name (1) > returns tree node for _1, and "t0_1" gets replaced by "_1" > resulting in multiple definitions for _1. > > The attached patch checks if multiple ssa names have same version > number and emits a diagnostic in that case, for the above case: > gimplefe-error-4.c: In function ‘foo’: > gimplefe-error-4.c:10:3: error: ssa version ‘1’ used anonymously and in ‘t0’ > t0_1 = a; > ^~~~ > > OK to commit after bootstrap+test ? Hmm, I'd rather (if at all -- I consider these kind of verification errors appropriate for valid GIMPLE FE input) do sth like basically at the point we emit a SSA def diagnose existing ones. Should be split out into a verify_def () function, and the diagnostic should be more helpful of course. Richard. > > Thanks, > Prathamesh > -- Richard Biener <rguenther@suse.de> SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)Index: gcc/c/gimple-parser.c =================================================================== --- gcc/c/gimple-parser.c (revision 245501) +++ gcc/c/gimple-parser.c (working copy) @@ -315,6 +315,12 @@ c_parser_gimple_statement (c_parser *par else if (! FLOAT_TYPE_P (TREE_TYPE (lhs.value)) && FLOAT_TYPE_P (TREE_TYPE (rhs.value))) code = FIX_TRUNC_EXPR; + if (TREE_CODE (lhs.value) == SSA_NAME + && SSA_NAME_DEF_STMT (lhs.value)) + { + c_parser_error (parser, "duplicate definition of SSA name"); + /* point at previous definition, do not emit stmt */ + } assign = gimple_build_assign (lhs.value, code, rhs.value); gimple_seq_add_stmt (seq, assign); gimple_set_location (assign, loc); @@ -347,6 +353,13 @@ c_parser_gimple_statement (c_parser *par rhs = c_parser_gimple_unary_expression (parser); if (rhs.value != error_mark_node) { + if (TREE_CODE (lhs.value) == SSA_NAME + && SSA_NAME_DEF_STMT (lhs.value)) + { + c_parser_error (parser, "duplicate definition of SSA name"); + /* point at previous definition, do not emit stmt */ + } + assign = gimple_build_assign (lhs.value, rhs.value); gimple_set_location (assign, loc); gimple_seq_add_stmt (seq, assign); @@ -420,6 +433,13 @@ c_parser_gimple_statement (c_parser *par if (lhs.value != error_mark_node && rhs.value != error_mark_node) { + if (TREE_CODE (lhs.value) == SSA_NAME + && SSA_NAME_DEF_STMT (lhs.value)) + { + c_parser_error (parser, "duplicate definition of SSA name"); + /* point at previous definition, do not emit stmt */ + } + assign = gimple_build_assign (lhs.value, rhs.value); gimple_seq_add_stmt (seq, assign); gimple_set_location (assign, loc); @@ -692,8 +712,7 @@ c_parser_parse_ssa_name (c_parser *parse if (VECTOR_TYPE_P (TREE_TYPE (parent)) || TREE_CODE (TREE_TYPE (parent)) == COMPLEX_TYPE) DECL_GIMPLE_REG_P (parent) = 1; - name = make_ssa_name_fn (cfun, parent, - gimple_build_nop (), version); + name = make_ssa_name_fn (cfun, parent, NULL, version); } }
On 16 February 2017 at 14:01, Richard Biener <rguenther@suse.de> wrote: > On Wed, 15 Feb 2017, Prathamesh Kulkarni wrote: > >> Hi, >> For the following (invalid) test-case: >> >> void __GIMPLE () foo (int a) >> { >> int t0; >> int _1; >> _1 = a; >> t0_1 = a; >> } >> >> results in following ICE: >> gimplefe-error-4.c: In function ‘foo’: >> gimplefe-error-4.c:20:1: error: SSA_NAME_DEF_STMT is wrong >> } >> ^ >> Expected definition statement: >> _1 = a_2(D); >> >> Actual definition statement: >> _1 = a_2(D); >> gimplefe-error-4.c:20:1: internal compiler error: verify_ssa failed >> 0xe1458b verify_ssa(bool, bool) >> ../../gcc/gcc/tree-ssa.c:1184 >> 0xb0d1ed execute_function_todo >> ../../gcc/gcc/passes.c:1973 >> 0xb0dad5 execute_todo >> ../../gcc/gcc/passes.c:2016 >> >> The reason for ICE is that in c_parser_parse_ssa_name, ssa_name (1) >> returns tree node for _1, and "t0_1" gets replaced by "_1" >> resulting in multiple definitions for _1. >> >> The attached patch checks if multiple ssa names have same version >> number and emits a diagnostic in that case, for the above case: >> gimplefe-error-4.c: In function ‘foo’: >> gimplefe-error-4.c:10:3: error: ssa version ‘1’ used anonymously and in ‘t0’ >> t0_1 = a; >> ^~~~ >> >> OK to commit after bootstrap+test ? > > Hmm, I'd rather (if at all -- I consider these kind of verification errors > appropriate for valid GIMPLE FE input) do sth like > > Index: gcc/c/gimple-parser.c > =================================================================== > --- gcc/c/gimple-parser.c (revision 245501) > +++ gcc/c/gimple-parser.c (working copy) > @@ -315,6 +315,12 @@ c_parser_gimple_statement (c_parser *par > else if (! FLOAT_TYPE_P (TREE_TYPE (lhs.value)) > && FLOAT_TYPE_P (TREE_TYPE (rhs.value))) > code = FIX_TRUNC_EXPR; > + if (TREE_CODE (lhs.value) == SSA_NAME > + && SSA_NAME_DEF_STMT (lhs.value)) > + { > + c_parser_error (parser, "duplicate definition of SSA name"); > + /* point at previous definition, do not emit stmt */ > + } > assign = gimple_build_assign (lhs.value, code, rhs.value); > gimple_seq_add_stmt (seq, assign); > gimple_set_location (assign, loc); > @@ -347,6 +353,13 @@ c_parser_gimple_statement (c_parser *par > rhs = c_parser_gimple_unary_expression (parser); > if (rhs.value != error_mark_node) > { > + if (TREE_CODE (lhs.value) == SSA_NAME > + && SSA_NAME_DEF_STMT (lhs.value)) > + { > + c_parser_error (parser, "duplicate definition of SSA name"); > + /* point at previous definition, do not emit stmt */ > + } > + > assign = gimple_build_assign (lhs.value, rhs.value); > gimple_set_location (assign, loc); > gimple_seq_add_stmt (seq, assign); > @@ -420,6 +433,13 @@ c_parser_gimple_statement (c_parser *par > if (lhs.value != error_mark_node > && rhs.value != error_mark_node) > { > + if (TREE_CODE (lhs.value) == SSA_NAME > + && SSA_NAME_DEF_STMT (lhs.value)) > + { > + c_parser_error (parser, "duplicate definition of SSA name"); > + /* point at previous definition, do not emit stmt */ > + } > + > assign = gimple_build_assign (lhs.value, rhs.value); > gimple_seq_add_stmt (seq, assign); > gimple_set_location (assign, loc); > @@ -692,8 +712,7 @@ c_parser_parse_ssa_name (c_parser *parse > if (VECTOR_TYPE_P (TREE_TYPE (parent)) > || TREE_CODE (TREE_TYPE (parent)) == COMPLEX_TYPE) > DECL_GIMPLE_REG_P (parent) = 1; > - name = make_ssa_name_fn (cfun, parent, > - gimple_build_nop (), version); > + name = make_ssa_name_fn (cfun, parent, NULL, version); > } > } > > > basically at the point we emit a SSA def diagnose existing ones. > Should be split out into a verify_def () function, and the diagnostic > should be more helpful of course. Hi Richard, Um IIUC, the issue is not about multiple definitions but when multiple names are used for same version, we pick the first version. For eg, the following invalid test-case is accepted by FE, and would not get caught by gimple-verifiers because the FE generates valid gimple but does not match the source. int __GIMPLE () foo (int a) { int t0; int _1; _1 = a; return t0_1; } the ssa pass dump shows: int __GIMPLE () foo (int a) { int _1; bb_2: _1 = a_2(D); return _1; } This happens because c_parser_parse_ssa_name() calls lookup_name (1) and since we have anonymous ssa name associated with version number 1 that is returned, and t0_1 gets replaced by _1 in return stmt. The patch would reject the above test-case. Thanks, Prathamesh > > Richard. > >> >> Thanks, >> Prathamesh >> > > -- > Richard Biener <rguenther@suse.de> > SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
diff --git a/gcc/c/gimple-parser.c b/gcc/c/gimple-parser.c index d959877..2e163f4 100644 --- a/gcc/c/gimple-parser.c +++ b/gcc/c/gimple-parser.c @@ -672,29 +672,49 @@ c_parser_parse_ssa_name (c_parser *parser, } else { + /* Separate var name from version. */ + char *var_name = XNEWVEC (char, ver_offset + 1); + memcpy (var_name, token, ver_offset); + var_name[ver_offset] = '\0'; + /* lookup for parent decl. */ + id = get_identifier (var_name); + tree parent = lookup_name (id); + if (! parent || parent == error_mark_node) + { + c_parser_error (parser, "base variable or SSA name undeclared"); + XDELETEVEC (var_name); + return error_mark_node; + } if (version < num_ssa_names) name = ssa_name (version); if (! name) { - /* Separate var name from version. */ - char *var_name = XNEWVEC (char, ver_offset + 1); - memcpy (var_name, token, ver_offset); - var_name[ver_offset] = '\0'; - /* lookup for parent decl. */ - id = get_identifier (var_name); - tree parent = lookup_name (id); - XDELETEVEC (var_name); - if (! parent || parent == error_mark_node) - { - c_parser_error (parser, "base variable or SSA name undeclared"); - return error_mark_node; - } if (VECTOR_TYPE_P (TREE_TYPE (parent)) || TREE_CODE (TREE_TYPE (parent)) == COMPLEX_TYPE) DECL_GIMPLE_REG_P (parent) = 1; name = make_ssa_name_fn (cfun, parent, gimple_build_nop (), version); } + else if (!SSA_NAME_IDENTIFIER (name)) + { + error_at (input_location, "ssa version %<%d%> used anonymously" + " and in %<%s%>", version, var_name); + XDELETEVEC (var_name); + return error_mark_node; + } + else + { + const char *ssaname = IDENTIFIER_POINTER (SSA_NAME_IDENTIFIER (name)); + if (strcmp (ssaname, var_name)) + { + error_at (input_location, "ssa version %<%d%> used for" + " multiple names %<%s%>, %<%s%>", version, + ssaname, var_name); + XDELETEVEC (var_name); + return error_mark_node; + } + } + XDELETEVEC (var_name); } return name; diff --git a/gcc/testsuite/gcc.dg/gimplefe-error-4.c b/gcc/testsuite/gcc.dg/gimplefe-error-4.c new file mode 100644 index 0000000..a3c652e --- /dev/null +++ b/gcc/testsuite/gcc.dg/gimplefe-error-4.c @@ -0,0 +1,20 @@ +/* { dg-do compile } */ +/* { dg-options "-fgimple" } */ + +void __GIMPLE () foo (int a) +{ + int t0; + int _1; + + _1 = a; + t0_1 = a; /* { dg-error "used anonymously and in 't0'" } */ +} + +void __GIMPLE () bar (int a) +{ + int t0; + int t1; + + t0_1 = a; + t1_1 = a; /* { dg-error "multiple names 't0', 't1'" } */ +}
Hi, For the following (invalid) test-case: void __GIMPLE () foo (int a) { int t0; int _1; _1 = a; t0_1 = a; } results in following ICE: gimplefe-error-4.c: In function ‘foo’: gimplefe-error-4.c:20:1: error: SSA_NAME_DEF_STMT is wrong } ^ Expected definition statement: _1 = a_2(D); Actual definition statement: _1 = a_2(D); gimplefe-error-4.c:20:1: internal compiler error: verify_ssa failed 0xe1458b verify_ssa(bool, bool) ../../gcc/gcc/tree-ssa.c:1184 0xb0d1ed execute_function_todo ../../gcc/gcc/passes.c:1973 0xb0dad5 execute_todo ../../gcc/gcc/passes.c:2016 The reason for ICE is that in c_parser_parse_ssa_name, ssa_name (1) returns tree node for _1, and "t0_1" gets replaced by "_1" resulting in multiple definitions for _1. The attached patch checks if multiple ssa names have same version number and emits a diagnostic in that case, for the above case: gimplefe-error-4.c: In function ‘foo’: gimplefe-error-4.c:10:3: error: ssa version ‘1’ used anonymously and in ‘t0’ t0_1 = a; ^~~~ OK to commit after bootstrap+test ? Thanks, Prathamesh 2017-02-15 Prathamesh Kulkarni <prathamesh.kulkarni@linaro.org> c/ * gimple-parser.c (c_parser_parse_ssa_name): Emit diagnostic if same ssa version is used with multiple names. testsuite/ * gcc.dg/gimplefe-error-4.c: New test.