| Message ID | CAAcHv8dNMuLiPK+4SkXGzuvGmGdE6Hv1LBtBRAM1aKFYkVJ+iQ@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | c++, contracts: Fix ICE in create_tmp_var [PR113968] | expand |
Hi Nina, thanks for the patch! > On 5 Jul 2024, at 15:25, Nina Dinka Ranns <dinka.ranns@googlemail.com> wrote: > > Certain places in contract parsing currently do not check for errors. > This results in contracts > with embedded errors which eventually confuse gimplify. Checks for > errors added in > grok_contract() and cp_parser_contract_attribute_spec() to exit early > if an error is encountered. > > Tested on x86_64-pc-linux-gnu this LGTM, but I cannot approve it. Iain > --- > > PR c++/113968 > > gcc/cp/ChangeLog: > > * contracts.cc (grok_contract): Check for error_mark_node early > exit > * parser.cc (cp_parser_contract_attribute_spec): Check for > error_mark_node early exit > > gcc/testsuite/ChangeLog: > > * g++.dg/contracts/pr113968.C: New test. > > Signed-off-by: Nina Ranns <dinka.ranns@gmail.com> > > > --- > gcc/cp/contracts.cc | 7 ++++++ > gcc/cp/parser.cc | 3 +++ > gcc/testsuite/g++.dg/contracts/pr113968.C | 29 +++++++++++++++++++++++ > 3 files changed, 39 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/contracts/pr113968.C > > diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc > index 634e3cf4fa9..a7d0fdacf6e 100644 > --- a/gcc/cp/contracts.cc > +++ b/gcc/cp/contracts.cc > @@ -750,6 +750,9 @@ tree > grok_contract (tree attribute, tree mode, tree result, cp_expr condition, > location_t loc) > { > + if (condition == error_mark_node) > + return error_mark_node; > + > tree_code code; > if (is_attribute_p ("assert", attribute)) > code = ASSERTION_STMT; > @@ -785,6 +788,10 @@ grok_contract (tree attribute, tree mode, tree > result, cp_expr condition, > > /* The condition is converted to bool. */ > condition = finish_contract_condition (condition); > + > + if (condition == error_mark_node) > + return error_mark_node; > + > CONTRACT_CONDITION (contract) = condition; > > return contract; > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 31ae9c2fb54..22c5e760aee 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -30835,6 +30835,9 @@ cp_parser_contract_attribute_spec (cp_parser > *parser, tree attribute) > return error_mark_node; > } > > + if (contract == error_mark_node) > + return error_mark_node; > + > return finish_contract_attribute (attribute, contract); > } > > diff --git a/gcc/testsuite/g++.dg/contracts/pr113968.C > b/gcc/testsuite/g++.dg/contracts/pr113968.C > new file mode 100644 > index 00000000000..fbaad1c930d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/contracts/pr113968.C > @@ -0,0 +1,29 @@ > +// check that an invalid contract condition doesn't cause an ICE > +// { dg-do compile } > +// { dg-options "-std=c++2a -fcontracts " } > + > +struct A > +{ > + A (A&); > +}; > +struct S > +{ > + void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + > + } > +}; > +void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + [[ assert : a ]]; // { dg-error "could not convert" } > + [[ assert : a.b ]];// { dg-error "has no member" } > + } > + > +int > +main () > +{ > +} > -- > 2.45.2
On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > Certain places in contract parsing currently do not check for errors. > This results in contracts > with embedded errors which eventually confuse gimplify. Checks for > errors added in > grok_contract() and cp_parser_contract_attribute_spec() to exit early > if an error is encountered. Thanks for the patch! > Tested on x86_64-pc-linux-gnu > --- > > PR c++/113968 > > gcc/cp/ChangeLog: > > * contracts.cc (grok_contract): Check for error_mark_node early > exit These hunks are OK. > * parser.cc (cp_parser_contract_attribute_spec): Check for > error_mark_node early exit This seems redundant, since finish_contract_attribute already checks for error_mark_node and we're returning its result unchanged. Also, the convention is for wrapped lines in ChangeLog entries to line up with the *, and to finish sentences with a period. > gcc/testsuite/ChangeLog: > > * g++.dg/contracts/pr113968.C: New test. > > Signed-off-by: Nina Ranns <dinka.ranns@gmail.com> > > > --- > gcc/cp/contracts.cc | 7 ++++++ > gcc/cp/parser.cc | 3 +++ > gcc/testsuite/g++.dg/contracts/pr113968.C | 29 +++++++++++++++++++++++ > 3 files changed, 39 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/contracts/pr113968.C > > diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc > index 634e3cf4fa9..a7d0fdacf6e 100644 > --- a/gcc/cp/contracts.cc > +++ b/gcc/cp/contracts.cc > @@ -750,6 +750,9 @@ tree > grok_contract (tree attribute, tree mode, tree result, cp_expr condition, > location_t loc) > { > + if (condition == error_mark_node) > + return error_mark_node; > + > tree_code code; > if (is_attribute_p ("assert", attribute)) > code = ASSERTION_STMT; > @@ -785,6 +788,10 @@ grok_contract (tree attribute, tree mode, tree > result, cp_expr condition, > > /* The condition is converted to bool. */ > condition = finish_contract_condition (condition); > + > + if (condition == error_mark_node) > + return error_mark_node; > + > CONTRACT_CONDITION (contract) = condition; > > return contract; > diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc > index 31ae9c2fb54..22c5e760aee 100644 > --- a/gcc/cp/parser.cc > +++ b/gcc/cp/parser.cc > @@ -30835,6 +30835,9 @@ cp_parser_contract_attribute_spec (cp_parser > *parser, tree attribute) > return error_mark_node; > } > > + if (contract == error_mark_node) > + return error_mark_node; > + > return finish_contract_attribute (attribute, contract); > } > > diff --git a/gcc/testsuite/g++.dg/contracts/pr113968.C > b/gcc/testsuite/g++.dg/contracts/pr113968.C > new file mode 100644 > index 00000000000..fbaad1c930d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/contracts/pr113968.C > @@ -0,0 +1,29 @@ > +// check that an invalid contract condition doesn't cause an ICE > +// { dg-do compile } > +// { dg-options "-std=c++2a -fcontracts " } > + > +struct A > +{ > + A (A&); > +}; > +struct S > +{ > + void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + > + } > +}; > +void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + [[ assert : a ]]; // { dg-error "could not convert" } > + [[ assert : a.b ]];// { dg-error "has no member" } > + } > + > +int > +main () > +{ > +}
HI Jason, On Fri, 5 Jul 2024 at 17:31, Jason Merrill <jason@redhat.com> wrote: > > On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > > Certain places in contract parsing currently do not check for errors. > > This results in contracts > > with embedded errors which eventually confuse gimplify. Checks for > > errors added in > > grok_contract() and cp_parser_contract_attribute_spec() to exit early > > if an error is encountered. > > Thanks for the patch! > > > Tested on x86_64-pc-linux-gnu > > --- > > > > PR c++/113968 > > > > gcc/cp/ChangeLog: > > > > * contracts.cc (grok_contract): Check for error_mark_node early > > exit > > These hunks are OK. > > > * parser.cc (cp_parser_contract_attribute_spec): Check for > > error_mark_node early exit > > This seems redundant, since finish_contract_attribute already checks for > error_mark_node and we're returning its result unchanged. good catch, removed. > > Also, the convention is for wrapped lines in ChangeLog entries to line > up with the *, and to finish sentences with a period. done. Tests re-run on x86_64-pc-linux-gnu , no change. --- PR C++/113968 gcc/cp/ChangeLog: * contracts.cc (grok_contract): Check for error_mark_node early exit. gcc/testsuite/ChangeLog: * g++.dg/contracts/pr113968.C: New test. Signed-off-by: Nina Ranns <dinka.ranns@gmail.com> --- gcc/cp/contracts.cc | 7 ++++++ gcc/testsuite/g++.dg/contracts/pr113968.C | 29 +++++++++++++++++++++++ 2 files changed, 36 insertions(+) create mode 100644 gcc/testsuite/g++.dg/contracts/pr113968.C diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc index 634e3cf4fa9..a7d0fdacf6e 100644 --- a/gcc/cp/contracts.cc +++ b/gcc/cp/contracts.cc @@ -750,6 +750,9 @@ tree grok_contract (tree attribute, tree mode, tree result, cp_expr condition, location_t loc) { + if (condition == error_mark_node) + return error_mark_node; + tree_code code; if (is_attribute_p ("assert", attribute)) code = ASSERTION_STMT; @@ -785,6 +788,10 @@ grok_contract (tree attribute, tree mode, tree result, cp_expr condition, /* The condition is converted to bool. */ condition = finish_contract_condition (condition); + + if (condition == error_mark_node) + return error_mark_node; + CONTRACT_CONDITION (contract) = condition; return contract; diff --git a/gcc/testsuite/g++.dg/contracts/pr113968.C b/gcc/testsuite/g++.dg/contracts/pr113968.C new file mode 100644 index 00000000000..fbaad1c930d --- /dev/null +++ b/gcc/testsuite/g++.dg/contracts/pr113968.C @@ -0,0 +1,29 @@ +// check that an invalid contract condition doesn't cause an ICE +// { dg-do compile } +// { dg-options "-std=c++2a -fcontracts " } + +struct A +{ + A (A&); +}; +struct S +{ + void f(A a) + [[ pre : a]] // { dg-error "could not convert" } + [[ pre : a.b]]// { dg-error "has no member" } + { + + } +}; +void f(A a) + [[ pre : a]] // { dg-error "could not convert" } + [[ pre : a.b]]// { dg-error "has no member" } + { + [[ assert : a ]]; // { dg-error "could not convert" } + [[ assert : a.b ]];// { dg-error "has no member" } + } + +int +main () +{ +} -- 2.45.2
On 7/8/24 7:47 AM, Nina Dinka Ranns wrote: > HI Jason, > > On Fri, 5 Jul 2024 at 17:31, Jason Merrill <jason@redhat.com> wrote: >> >> On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: >>> Certain places in contract parsing currently do not check for errors. >>> This results in contracts >>> with embedded errors which eventually confuse gimplify. Checks for >>> errors added in >>> grok_contract() and cp_parser_contract_attribute_spec() to exit early >>> if an error is encountered. >> >> Thanks for the patch! >> >>> Tested on x86_64-pc-linux-gnu >>> --- >>> >>> PR c++/113968 >>> >>> gcc/cp/ChangeLog: >>> >>> * contracts.cc (grok_contract): Check for error_mark_node early >>> exit >> >> These hunks are OK. >> >>> * parser.cc (cp_parser_contract_attribute_spec): Check for >>> error_mark_node early exit >> >> This seems redundant, since finish_contract_attribute already checks for >> error_mark_node and we're returning its result unchanged. > > good catch, removed. > >> >> Also, the convention is for wrapped lines in ChangeLog entries to line >> up with the *, and to finish sentences with a period. > > done. > > Tests re-run on x86_64-pc-linux-gnu , no change. This looks good, but the patch doesn't apply due to word wrap. To avoid that, I tend to use git send-email; sending the patch as an attachment is also OK. Or see https://www.kernel.org/doc/html/latest/process/email-clients.html for tips on getting various email clients to leave patches alone. > --- > PR C++/113968 > > gcc/cp/ChangeLog: > > * contracts.cc (grok_contract): Check for error_mark_node early > exit. > > gcc/testsuite/ChangeLog: > > * g++.dg/contracts/pr113968.C: New test. > > Signed-off-by: Nina Ranns <dinka.ranns@gmail.com> > > --- > > gcc/cp/contracts.cc | 7 ++++++ > gcc/testsuite/g++.dg/contracts/pr113968.C | 29 +++++++++++++++++++++++ > 2 files changed, 36 insertions(+) > create mode 100644 gcc/testsuite/g++.dg/contracts/pr113968.C > > diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc > index 634e3cf4fa9..a7d0fdacf6e 100644 > --- a/gcc/cp/contracts.cc > +++ b/gcc/cp/contracts.cc > @@ -750,6 +750,9 @@ tree > grok_contract (tree attribute, tree mode, tree result, cp_expr condition, > location_t loc) > { > + if (condition == error_mark_node) > + return error_mark_node; > + > tree_code code; > if (is_attribute_p ("assert", attribute)) > code = ASSERTION_STMT; > @@ -785,6 +788,10 @@ grok_contract (tree attribute, tree mode, tree > result, cp_expr condition, > > /* The condition is converted to bool. */ > condition = finish_contract_condition (condition); > + > + if (condition == error_mark_node) > + return error_mark_node; > + > CONTRACT_CONDITION (contract) = condition; > > return contract; > diff --git a/gcc/testsuite/g++.dg/contracts/pr113968.C > b/gcc/testsuite/g++.dg/contracts/pr113968.C > new file mode 100644 > index 00000000000..fbaad1c930d > --- /dev/null > +++ b/gcc/testsuite/g++.dg/contracts/pr113968.C > @@ -0,0 +1,29 @@ > +// check that an invalid contract condition doesn't cause an ICE > +// { dg-do compile } > +// { dg-options "-std=c++2a -fcontracts " } > + > +struct A > +{ > + A (A&); > +}; > +struct S > +{ > + void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + > + } > +}; > +void f(A a) > + [[ pre : a]] // { dg-error "could not convert" } > + [[ pre : a.b]]// { dg-error "has no member" } > + { > + [[ assert : a ]]; // { dg-error "could not convert" } > + [[ assert : a.b ]];// { dg-error "has no member" } > + } > + > +int > +main () > +{ > +} > -- > 2.45.2 >
Hi Jason, On Mon, 8 Jul 2024 at 16:01, Jason Merrill <jason@redhat.com> wrote: > On 7/8/24 7:47 AM, Nina Dinka Ranns wrote: > > HI Jason, > > > > On Fri, 5 Jul 2024 at 17:31, Jason Merrill <jason@redhat.com> wrote: > >> > >> On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > >>> Certain places in contract parsing currently do not check for errors. > >>> This results in contracts > >>> with embedded errors which eventually confuse gimplify. Checks for > >>> errors added in > >>> grok_contract() and cp_parser_contract_attribute_spec() to exit early > >>> if an error is encountered. > >> > >> Thanks for the patch! > >> > >>> Tested on x86_64-pc-linux-gnu > >>> --- > >>> > >>> PR c++/113968 > >>> > >>> gcc/cp/ChangeLog: > >>> > >>> * contracts.cc (grok_contract): Check for error_mark_node > early > >>> exit > >> > >> These hunks are OK. > >> > >>> * parser.cc (cp_parser_contract_attribute_spec): Check for > >>> error_mark_node early exit > >> > >> This seems redundant, since finish_contract_attribute already checks for > >> error_mark_node and we're returning its result unchanged. > > > > good catch, removed. > > > >> > >> Also, the convention is for wrapped lines in ChangeLog entries to line > >> up with the *, and to finish sentences with a period. > > > > done. > > > > Tests re-run on x86_64-pc-linux-gnu , no change. > > This looks good, but the patch doesn't apply due to word wrap. To avoid > that, I tend to use git send-email; sending the patch as an attachment > is also OK. Or see > > https://www.kernel.org/doc/html/latest/process/email-clients.html > > for tips on getting various email clients to leave patches alone. > ack, thank you for your patience. This time, patch attached to the email. Best, Nina
On 7/9/24 6:41 AM, Nina Dinka Ranns wrote: > On Mon, 8 Jul 2024 at 16:01, Jason Merrill <jason@redhat.com > <mailto:jason@redhat.com>> wrote: > > On 7/8/24 7:47 AM, Nina Dinka Ranns wrote: > > HI Jason, > > > > On Fri, 5 Jul 2024 at 17:31, Jason Merrill <jason@redhat.com > <mailto:jason@redhat.com>> wrote: > >> > >> On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > >>> Certain places in contract parsing currently do not check for > errors. > >>> This results in contracts > >>> with embedded errors which eventually confuse gimplify. Checks for > >>> errors added in > >>> grok_contract() and cp_parser_contract_attribute_spec() to exit > early > >>> if an error is encountered. > >> > >> Thanks for the patch! > >> > >>> Tested on x86_64-pc-linux-gnu > >>> --- > >>> > >>> PR c++/113968 > >>> > >>> gcc/cp/ChangeLog: > >>> > >>> * contracts.cc (grok_contract): Check for > error_mark_node early > >>> exit > >> > >> These hunks are OK. > >> > >>> * parser.cc (cp_parser_contract_attribute_spec): > Check for > >>> error_mark_node early exit > >> > >> This seems redundant, since finish_contract_attribute already > checks for > >> error_mark_node and we're returning its result unchanged. > > > > good catch, removed. > > > >> > >> Also, the convention is for wrapped lines in ChangeLog entries > to line > >> up with the *, and to finish sentences with a period. > > > > done. > > > > Tests re-run on x86_64-pc-linux-gnu , no change. > > This looks good, but the patch doesn't apply due to word wrap. To > avoid > that, I tend to use git send-email; sending the patch as an attachment > is also OK. Or see > > https://www.kernel.org/doc/html/latest/process/email-clients.html > <https://www.kernel.org/doc/html/latest/process/email-clients.html> > > for tips on getting various email clients to leave patches alone. > > > ack, thank you for your patience. > This time, patch attached to the email. It looks like the attached patch reverted to older ChangeLog entries, without the periods, and with the dropped parser.cc change? git gcc-verify also complains > ERR: line should start with a tab: " * contracts.cc (grok_contract): Check for error_mark_node early" > ERR: line should start with a tab: " exit" > ERR: line should start with a tab: " * parser.cc (cp_parser_contract_attribute_spec): Check for" > ERR: line should start with a tab: " error_mark_node early exit" > ERR: line should start with a tab: " * g++.dg/contracts/pr113968.C: New test." > ERR: PR 113968 in subject but not in changelog: "c++, contracts: Fix ICE in create_tmp_var [PR113968]" Jason
On Tue, 9 Jul 2024 at 22:50, Jason Merrill <jason@redhat.com> wrote: > On 7/9/24 6:41 AM, Nina Dinka Ranns wrote: > > On Mon, 8 Jul 2024 at 16:01, Jason Merrill <jason@redhat.com > > <mailto:jason@redhat.com>> wrote: > > > > On 7/8/24 7:47 AM, Nina Dinka Ranns wrote: > > > HI Jason, > > > > > > On Fri, 5 Jul 2024 at 17:31, Jason Merrill <jason@redhat.com > > <mailto:jason@redhat.com>> wrote: > > >> > > >> On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > > >>> Certain places in contract parsing currently do not check for > > errors. > > >>> This results in contracts > > >>> with embedded errors which eventually confuse gimplify. Checks > for > > >>> errors added in > > >>> grok_contract() and cp_parser_contract_attribute_spec() to exit > > early > > >>> if an error is encountered. > > >> > > >> Thanks for the patch! > > >> > > >>> Tested on x86_64-pc-linux-gnu > > >>> --- > > >>> > > >>> PR c++/113968 > > >>> > > >>> gcc/cp/ChangeLog: > > >>> > > >>> * contracts.cc (grok_contract): Check for > > error_mark_node early > > >>> exit > > >> > > >> These hunks are OK. > > >> > > >>> * parser.cc (cp_parser_contract_attribute_spec): > > Check for > > >>> error_mark_node early exit > > >> > > >> This seems redundant, since finish_contract_attribute already > > checks for > > >> error_mark_node and we're returning its result unchanged. > > > > > > good catch, removed. > > > > > >> > > >> Also, the convention is for wrapped lines in ChangeLog entries > > to line > > >> up with the *, and to finish sentences with a period. > > > > > > done. > > > > > > Tests re-run on x86_64-pc-linux-gnu , no change. > > > > This looks good, but the patch doesn't apply due to word wrap. To > > avoid > > that, I tend to use git send-email; sending the patch as an > attachment > > is also OK. Or see > > > > https://www.kernel.org/doc/html/latest/process/email-clients.html > > <https://www.kernel.org/doc/html/latest/process/email-clients.html> > > > > for tips on getting various email clients to leave patches alone. > > > > > > ack, thank you for your patience. > > This time, patch attached to the email. > > It looks like the attached patch reverted to older ChangeLog entries, > without the periods, and with the dropped parser.cc change? > > git gcc-verify also complains > > > ERR: line should start with a tab: " * contracts.cc > (grok_contract): Check for error_mark_node early" > > ERR: line should start with a tab: " exit" > > ERR: line should start with a tab: " * parser.cc > (cp_parser_contract_attribute_spec): Check for" > > ERR: line should start with a tab: " error_mark_node early exit" > > ERR: line should start with a tab: " * > g++.dg/contracts/pr113968.C: New test." > > ERR: PR 113968 in subject but not in changelog: "c++, contracts: Fix ICE > in create_tmp_var [PR113968]" > > Jason > > Apologies. I must have copy pasted something wrong. I've setup gcc-verify and that passes. Let's try again. Patch attached. Thank you, Nina
On 7/10/24 5:37 AM, Nina Dinka Ranns wrote: > > > On Tue, 9 Jul 2024 at 22:50, Jason Merrill <jason@redhat.com > <mailto:jason@redhat.com>> wrote: > > On 7/9/24 6:41 AM, Nina Dinka Ranns wrote: > > On Mon, 8 Jul 2024 at 16:01, Jason Merrill <jason@redhat.com > <mailto:jason@redhat.com> > > <mailto:jason@redhat.com <mailto:jason@redhat.com>>> wrote: > > > > On 7/8/24 7:47 AM, Nina Dinka Ranns wrote: > > > HI Jason, > > > > > > On Fri, 5 Jul 2024 at 17:31, Jason Merrill > <jason@redhat.com <mailto:jason@redhat.com> > > <mailto:jason@redhat.com <mailto:jason@redhat.com>>> wrote: > > >> > > >> On 7/5/24 10:25 AM, Nina Dinka Ranns wrote: > > >>> Certain places in contract parsing currently do not > check for > > errors. > > >>> This results in contracts > > >>> with embedded errors which eventually confuse gimplify. > Checks for > > >>> errors added in > > >>> grok_contract() and cp_parser_contract_attribute_spec() > to exit > > early > > >>> if an error is encountered. > > >> > > >> Thanks for the patch! > > >> > > >>> Tested on x86_64-pc-linux-gnu > > >>> --- > > >>> > > >>> PR c++/113968 > > >>> > > >>> gcc/cp/ChangeLog: > > >>> > > >>> * contracts.cc (grok_contract): Check for > > error_mark_node early > > >>> exit > > >> > > >> These hunks are OK. > > >> > > >>> * parser.cc (cp_parser_contract_attribute_spec): > > Check for > > >>> error_mark_node early exit > > >> > > >> This seems redundant, since finish_contract_attribute already > > checks for > > >> error_mark_node and we're returning its result unchanged. > > > > > > good catch, removed. > > > > > >> > > >> Also, the convention is for wrapped lines in ChangeLog > entries > > to line > > >> up with the *, and to finish sentences with a period. > > > > > > done. > > > > > > Tests re-run on x86_64-pc-linux-gnu , no change. > > > > This looks good, but the patch doesn't apply due to word > wrap. To > > avoid > > that, I tend to use git send-email; sending the patch as an > attachment > > is also OK. Or see > > > > https://www.kernel.org/doc/html/latest/process/email-clients.html > <https://www.kernel.org/doc/html/latest/process/email-clients.html> > > > <https://www.kernel.org/doc/html/latest/process/email-clients.html > <https://www.kernel.org/doc/html/latest/process/email-clients.html>> > > > > for tips on getting various email clients to leave patches alone. > > > > > > ack, thank you for your patience. > > This time, patch attached to the email. > > It looks like the attached patch reverted to older ChangeLog entries, > without the periods, and with the dropped parser.cc change? > > git gcc-verify also complains > > > ERR: line should start with a tab: " * contracts.cc > (grok_contract): Check for error_mark_node early" > > ERR: line should start with a tab: " exit" > > ERR: line should start with a tab: " * parser.cc > (cp_parser_contract_attribute_spec): Check for" > > ERR: line should start with a tab: " error_mark_node > early exit" > > ERR: line should start with a tab: " * > g++.dg/contracts/pr113968.C: New test." > > ERR: PR 113968 in subject but not in changelog: "c++, contracts: > Fix ICE in create_tmp_var [PR113968]" > > Jason > > > Apologies. I must have copy pasted something wrong. I've setup > gcc-verify and that passes. > Let's try again. Patch attached. Pushed, thanks. Jason
diff --git a/gcc/cp/contracts.cc b/gcc/cp/contracts.cc index 634e3cf4fa9..a7d0fdacf6e 100644 --- a/gcc/cp/contracts.cc +++ b/gcc/cp/contracts.cc @@ -750,6 +750,9 @@ tree grok_contract (tree attribute, tree mode, tree result, cp_expr condition, location_t loc) { + if (condition == error_mark_node) + return error_mark_node; + tree_code code; if (is_attribute_p ("assert", attribute)) code = ASSERTION_STMT; @@ -785,6 +788,10 @@ grok_contract (tree attribute, tree mode, tree result, cp_expr condition, /* The condition is converted to bool. */ condition = finish_contract_condition (condition); + + if (condition == error_mark_node) + return error_mark_node; + CONTRACT_CONDITION (contract) = condition; return contract; diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc index 31ae9c2fb54..22c5e760aee 100644 --- a/gcc/cp/parser.cc +++ b/gcc/cp/parser.cc @@ -30835,6 +30835,9 @@ cp_parser_contract_attribute_spec (cp_parser *parser, tree attribute) return error_mark_node; } + if (contract == error_mark_node) + return error_mark_node; + return finish_contract_attribute (attribute, contract); } diff --git a/gcc/testsuite/g++.dg/contracts/pr113968.C b/gcc/testsuite/g++.dg/contracts/pr113968.C new file mode 100644 index 00000000000..fbaad1c930d --- /dev/null +++ b/gcc/testsuite/g++.dg/contracts/pr113968.C @@ -0,0 +1,29 @@ +// check that an invalid contract condition doesn't cause an ICE +// { dg-do compile } +// { dg-options "-std=c++2a -fcontracts " } + +struct A +{ + A (A&); +}; +struct S +{ + void f(A a) + [[ pre : a]] // { dg-error "could not convert" } + [[ pre : a.b]]// { dg-error "has no member" } + { + + } +}; +void f(A a) + [[ pre : a]] // { dg-error "could not convert" } + [[ pre : a.b]]// { dg-error "has no member" } + { + [[ assert : a ]]; // { dg-error "could not convert" } + [[ assert : a.b ]];// { dg-error "has no member" } + } + +int +main () +{ +}
Certain places in contract parsing currently do not check for errors. This results in contracts with embedded errors which eventually confuse gimplify. Checks for errors added in grok_contract() and cp_parser_contract_attribute_spec() to exit early if an error is encountered. Tested on x86_64-pc-linux-gnu --- PR c++/113968 gcc/cp/ChangeLog: * contracts.cc (grok_contract): Check for error_mark_node early exit * parser.cc (cp_parser_contract_attribute_spec): Check for error_mark_node early exit gcc/testsuite/ChangeLog: * g++.dg/contracts/pr113968.C: New test. Signed-off-by: Nina Ranns <dinka.ranns@gmail.com> --- gcc/cp/contracts.cc | 7 ++++++ gcc/cp/parser.cc | 3 +++ gcc/testsuite/g++.dg/contracts/pr113968.C | 29 +++++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 gcc/testsuite/g++.dg/contracts/pr113968.C