Message ID | 20180516151529.GA3573@redhat.com |
---|---|
State | New |
Headers | show |
Series | C++ PATCH for c++/85363, wrong-code with throwing list-initializer | expand |
On Wed, May 16, 2018 at 11:15 AM, Marek Polacek <polacek@redhat.com> wrote: > This PR has been on my mind for quite some time but I've not found a solution > that I'd like. Maybe one of you can think of something better. > > The problem in this test is that in C++11, .eh optimizes out the catch, > so the exception is never caught. That is because lower_catch doesn't > think that the region may throw (eh_region_may_contain_throw). That's > so because P::P () is marked as TREE_NOTHROW, which is wrong, because > it calls X::X() which calls init() with throw. TREE_NOTHROW is set in > finish_function: > > /* If this function can't throw any exceptions, remember that. */ > if (!processing_template_decl > && !cp_function_chain->can_throw > && !flag_non_call_exceptions > && !decl_replaceable_p (fndecl)) > TREE_NOTHROW (fndecl) = 1; > > P::P() should have been marked as can_throw in set_flags_from_callee, but when > processing X::X() cfun is null, so we can't set it. P::P() is created only > later via implicitly_declare_fn. This should be handled by bot_manip (under break_out_target_exprs, under get_nsdmi), but it seems that it currently only calls set_flags_from_callee for CALL_EXPR, not for AGGR_INIT_EXPR as we have in this case. Jason
On Wed, May 16, 2018 at 11:35:56AM -0400, Jason Merrill wrote: > On Wed, May 16, 2018 at 11:15 AM, Marek Polacek <polacek@redhat.com> wrote: > > This PR has been on my mind for quite some time but I've not found a solution > > that I'd like. Maybe one of you can think of something better. > > > > The problem in this test is that in C++11, .eh optimizes out the catch, > > so the exception is never caught. That is because lower_catch doesn't > > think that the region may throw (eh_region_may_contain_throw). That's > > so because P::P () is marked as TREE_NOTHROW, which is wrong, because > > it calls X::X() which calls init() with throw. TREE_NOTHROW is set in > > finish_function: > > > > /* If this function can't throw any exceptions, remember that. */ > > if (!processing_template_decl > > && !cp_function_chain->can_throw > > && !flag_non_call_exceptions > > && !decl_replaceable_p (fndecl)) > > TREE_NOTHROW (fndecl) = 1; > > > > P::P() should have been marked as can_throw in set_flags_from_callee, but when > > processing X::X() cfun is null, so we can't set it. P::P() is created only > > later via implicitly_declare_fn. > > This should be handled by bot_manip (under break_out_target_exprs, > under get_nsdmi), but it seems that it currently only calls > set_flags_from_callee for CALL_EXPR, not for AGGR_INIT_EXPR as we have > in this case. Ah, nice! So, this tweaks set_flags_from_callee to also work for AGGR_INIT_EXPRs. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-05-16 Marek Polacek <polacek@redhat.com> PR c++/85363 * call.c (set_flags_from_callee): Handle AGGR_INIT_EXPRs too. * tree.c (bot_manip): Call set_flags_from_callee for AGGR_INIT_EXPRs too. * g++.dg/cpp0x/initlist-throw1.C: New test. * g++.dg/cpp0x/initlist-throw2.C: New test. diff --git gcc/cp/call.c gcc/cp/call.c index 09a3618b007..11b40747932 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -319,16 +319,23 @@ build_call_n (tree function, int n, ...) void set_flags_from_callee (tree call) { - bool nothrow; - tree decl = get_callee_fndecl (call); + /* Handle both CALL_EXPRs and AGGR_INIT_EXPRs. */ + tree decl = cp_get_callee_fndecl_nofold (call); /* We check both the decl and the type; a function may be known not to throw without being declared throw(). */ - nothrow = decl && TREE_NOTHROW (decl); - if (CALL_EXPR_FN (call)) - nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); - else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) - nothrow = true; + bool nothrow = decl && TREE_NOTHROW (decl); + if (TREE_CODE (call) == CALL_EXPR) + { + if (CALL_EXPR_FN (call)) + nothrow + |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); + else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) + nothrow = true; + } + else if (AGGR_INIT_EXPR_FN (call)) + nothrow + |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (AGGR_INIT_EXPR_FN (call)))); if (!nothrow && at_function_scope_p () && cfun && cp_function_chain) cp_function_chain->can_throw = 1; diff --git gcc/cp/tree.c gcc/cp/tree.c index ecb88df23b9..db81da91676 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -2987,7 +2987,7 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_) /* Make a copy of this node. */ t = copy_tree_r (tp, walk_subtrees, NULL); - if (TREE_CODE (*tp) == CALL_EXPR) + if (TREE_CODE (*tp) == CALL_EXPR || TREE_CODE (*tp) == AGGR_INIT_EXPR) set_flags_from_callee (*tp); if (data.clear_location && EXPR_HAS_LOCATION (*tp)) SET_EXPR_LOCATION (*tp, input_location); diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C index e69de29bb2d..264c6c7a7a0 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C @@ -0,0 +1,29 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X (int f) : n {init (f)} {} + int n; +}; + +struct P { + X x{20}; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +} diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C index e69de29bb2d..2bb05834d9e 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C @@ -0,0 +1,33 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X () : n {init (42)} {} + int n; +}; + +struct P { + struct R { + struct Q { + X x = {}; + } q; + } r; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +}
On Wed, May 16, 2018 at 1:37 PM, Marek Polacek <polacek@redhat.com> wrote: > On Wed, May 16, 2018 at 11:35:56AM -0400, Jason Merrill wrote: >> On Wed, May 16, 2018 at 11:15 AM, Marek Polacek <polacek@redhat.com> wrote: >> > This PR has been on my mind for quite some time but I've not found a solution >> > that I'd like. Maybe one of you can think of something better. >> > >> > The problem in this test is that in C++11, .eh optimizes out the catch, >> > so the exception is never caught. That is because lower_catch doesn't >> > think that the region may throw (eh_region_may_contain_throw). That's >> > so because P::P () is marked as TREE_NOTHROW, which is wrong, because >> > it calls X::X() which calls init() with throw. TREE_NOTHROW is set in >> > finish_function: >> > >> > /* If this function can't throw any exceptions, remember that. */ >> > if (!processing_template_decl >> > && !cp_function_chain->can_throw >> > && !flag_non_call_exceptions >> > && !decl_replaceable_p (fndecl)) >> > TREE_NOTHROW (fndecl) = 1; >> > >> > P::P() should have been marked as can_throw in set_flags_from_callee, but when >> > processing X::X() cfun is null, so we can't set it. P::P() is created only >> > later via implicitly_declare_fn. >> >> This should be handled by bot_manip (under break_out_target_exprs, >> under get_nsdmi), but it seems that it currently only calls >> set_flags_from_callee for CALL_EXPR, not for AGGR_INIT_EXPR as we have >> in this case. > > Ah, nice! So, this tweaks set_flags_from_callee to also work for > AGGR_INIT_EXPRs. > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-05-16 Marek Polacek <polacek@redhat.com> > > PR c++/85363 > * call.c (set_flags_from_callee): Handle AGGR_INIT_EXPRs too. > * tree.c (bot_manip): Call set_flags_from_callee for > AGGR_INIT_EXPRs too. > > * g++.dg/cpp0x/initlist-throw1.C: New test. > * g++.dg/cpp0x/initlist-throw2.C: New test. > > diff --git gcc/cp/call.c gcc/cp/call.c > index 09a3618b007..11b40747932 100644 > --- gcc/cp/call.c > +++ gcc/cp/call.c > @@ -319,16 +319,23 @@ build_call_n (tree function, int n, ...) > void > set_flags_from_callee (tree call) > { > - bool nothrow; > - tree decl = get_callee_fndecl (call); > + /* Handle both CALL_EXPRs and AGGR_INIT_EXPRs. */ > + tree decl = cp_get_callee_fndecl_nofold (call); > > /* We check both the decl and the type; a function may be known not to > throw without being declared throw(). */ > - nothrow = decl && TREE_NOTHROW (decl); > - if (CALL_EXPR_FN (call)) > - nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); > - else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) > - nothrow = true; > + bool nothrow = decl && TREE_NOTHROW (decl); > + if (TREE_CODE (call) == CALL_EXPR) > + { > + if (CALL_EXPR_FN (call)) > + nothrow > + |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); > + else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) > + nothrow = true; > + } > + else if (AGGR_INIT_EXPR_FN (call)) > + nothrow > + |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (AGGR_INIT_EXPR_FN (call)))); You should be able to avoid duplication here by using cp_get_callee rather than *_FN. Jason
On Wed, May 16, 2018 at 01:53:50PM -0400, Jason Merrill wrote: > You should be able to avoid duplication here by using cp_get_callee > rather than *_FN. Even better, thanks! Bootstrapped/regtested on x86_64-linux, ok for trunk? 2018-05-16 Marek Polacek <polacek@redhat.com> PR c++/85363 * call.c (set_flags_from_callee): Handle AGGR_INIT_EXPRs too. * tree.c (bot_manip): Call set_flags_from_callee for AGGR_INIT_EXPRs too. * g++.dg/cpp0x/initlist-throw1.C: New test. * g++.dg/cpp0x/initlist-throw2.C: New test. diff --git gcc/cp/call.c gcc/cp/call.c index 09a3618b007..4d04785f2b9 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -319,15 +319,17 @@ build_call_n (tree function, int n, ...) void set_flags_from_callee (tree call) { - bool nothrow; - tree decl = get_callee_fndecl (call); + /* Handle both CALL_EXPRs and AGGR_INIT_EXPRs. */ + tree decl = cp_get_callee_fndecl_nofold (call); /* We check both the decl and the type; a function may be known not to throw without being declared throw(). */ - nothrow = decl && TREE_NOTHROW (decl); - if (CALL_EXPR_FN (call)) - nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); - else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) + bool nothrow = decl && TREE_NOTHROW (decl); + tree callee = cp_get_callee (call); + if (callee) + nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (callee))); + else if (TREE_CODE (call) == CALL_EXPR + && internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) nothrow = true; if (!nothrow && at_function_scope_p () && cfun && cp_function_chain) diff --git gcc/cp/tree.c gcc/cp/tree.c index ecb88df23b9..db81da91676 100644 --- gcc/cp/tree.c +++ gcc/cp/tree.c @@ -2987,7 +2987,7 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_) /* Make a copy of this node. */ t = copy_tree_r (tp, walk_subtrees, NULL); - if (TREE_CODE (*tp) == CALL_EXPR) + if (TREE_CODE (*tp) == CALL_EXPR || TREE_CODE (*tp) == AGGR_INIT_EXPR) set_flags_from_callee (*tp); if (data.clear_location && EXPR_HAS_LOCATION (*tp)) SET_EXPR_LOCATION (*tp, input_location); diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C index e69de29bb2d..264c6c7a7a0 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C @@ -0,0 +1,29 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X (int f) : n {init (f)} {} + int n; +}; + +struct P { + X x{20}; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +} diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C index e69de29bb2d..2bb05834d9e 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C @@ -0,0 +1,33 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X () : n {init (42)} {} + int n; +}; + +struct P { + struct R { + struct Q { + X x = {}; + } q; + } r; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +}
OK. On Wed, May 16, 2018 at 2:44 PM, Marek Polacek <polacek@redhat.com> wrote: > On Wed, May 16, 2018 at 01:53:50PM -0400, Jason Merrill wrote: >> You should be able to avoid duplication here by using cp_get_callee >> rather than *_FN. > > Even better, thanks! > > Bootstrapped/regtested on x86_64-linux, ok for trunk? > > 2018-05-16 Marek Polacek <polacek@redhat.com> > > PR c++/85363 > * call.c (set_flags_from_callee): Handle AGGR_INIT_EXPRs too. > * tree.c (bot_manip): Call set_flags_from_callee for > AGGR_INIT_EXPRs too. > > * g++.dg/cpp0x/initlist-throw1.C: New test. > * g++.dg/cpp0x/initlist-throw2.C: New test. > > diff --git gcc/cp/call.c gcc/cp/call.c > index 09a3618b007..4d04785f2b9 100644 > --- gcc/cp/call.c > +++ gcc/cp/call.c > @@ -319,15 +319,17 @@ build_call_n (tree function, int n, ...) > void > set_flags_from_callee (tree call) > { > - bool nothrow; > - tree decl = get_callee_fndecl (call); > + /* Handle both CALL_EXPRs and AGGR_INIT_EXPRs. */ > + tree decl = cp_get_callee_fndecl_nofold (call); > > /* We check both the decl and the type; a function may be known not to > throw without being declared throw(). */ > - nothrow = decl && TREE_NOTHROW (decl); > - if (CALL_EXPR_FN (call)) > - nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (CALL_EXPR_FN (call)))); > - else if (internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) > + bool nothrow = decl && TREE_NOTHROW (decl); > + tree callee = cp_get_callee (call); > + if (callee) > + nothrow |= TYPE_NOTHROW_P (TREE_TYPE (TREE_TYPE (callee))); > + else if (TREE_CODE (call) == CALL_EXPR > + && internal_fn_flags (CALL_EXPR_IFN (call)) & ECF_NOTHROW) > nothrow = true; > > if (!nothrow && at_function_scope_p () && cfun && cp_function_chain) > diff --git gcc/cp/tree.c gcc/cp/tree.c > index ecb88df23b9..db81da91676 100644 > --- gcc/cp/tree.c > +++ gcc/cp/tree.c > @@ -2987,7 +2987,7 @@ bot_manip (tree* tp, int* walk_subtrees, void* data_) > > /* Make a copy of this node. */ > t = copy_tree_r (tp, walk_subtrees, NULL); > - if (TREE_CODE (*tp) == CALL_EXPR) > + if (TREE_CODE (*tp) == CALL_EXPR || TREE_CODE (*tp) == AGGR_INIT_EXPR) > set_flags_from_callee (*tp); > if (data.clear_location && EXPR_HAS_LOCATION (*tp)) > SET_EXPR_LOCATION (*tp, input_location); > diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C > index e69de29bb2d..264c6c7a7a0 100644 > --- gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C > +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C > @@ -0,0 +1,29 @@ > +// PR c++/85363 > +// { dg-do run { target c++11 } } > + > +int > +init (int f) > +{ > + throw f; > +} > + > +struct X { > + X (int f) : n {init (f)} {} > + int n; > +}; > + > +struct P { > + X x{20}; > +}; > + > +int > +main () > +{ > + try { > + P p {}; > + } > + catch (int n) { > + return 0; > + } > + return 1; > +} > diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C > index e69de29bb2d..2bb05834d9e 100644 > --- gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C > +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C > @@ -0,0 +1,33 @@ > +// PR c++/85363 > +// { dg-do run { target c++11 } } > + > +int > +init (int f) > +{ > + throw f; > +} > + > +struct X { > + X () : n {init (42)} {} > + int n; > +}; > + > +struct P { > + struct R { > + struct Q { > + X x = {}; > + } q; > + } r; > +}; > + > +int > +main () > +{ > + try { > + P p {}; > + } > + catch (int n) { > + return 0; > + } > + return 1; > +}
diff --git gcc/cp/call.c gcc/cp/call.c index 09a3618b007..f839d943443 100644 --- gcc/cp/call.c +++ gcc/cp/call.c @@ -332,6 +332,8 @@ set_flags_from_callee (tree call) if (!nothrow && at_function_scope_p () && cfun && cp_function_chain) cp_function_chain->can_throw = 1; + else if (!nothrow && at_class_scope_p () && decl && DECL_CONSTRUCTOR_P (decl)) + CLASSTYPE_CTOR_MAY_THROW (current_class_type) = true; if (decl && TREE_THIS_VOLATILE (decl) && cfun && cp_function_chain) current_function_returns_abnormally = 1; diff --git gcc/cp/cp-tree.h gcc/cp/cp-tree.h index cab926028b8..7a781a9a8a3 100644 --- gcc/cp/cp-tree.h +++ gcc/cp/cp-tree.h @@ -2025,6 +2025,7 @@ struct GTY(()) lang_type { unsigned has_constexpr_ctor : 1; unsigned unique_obj_representations : 1; unsigned unique_obj_representations_set : 1; + unsigned ctor_may_throw : 1; /* When adding a flag here, consider whether or not it ought to apply to a template instance if it applies to the template. If @@ -2033,7 +2034,7 @@ struct GTY(()) lang_type { /* There are some bits left to fill out a 32-bit word. Keep track of this by updating the size of this bitfield whenever you add or remove a flag. */ - unsigned dummy : 4; + unsigned dummy : 3; tree primary_base; vec<tree_pair_s, va_gc> *vcall_indices; @@ -2105,6 +2106,10 @@ struct GTY(()) lang_type { #define CLASSTYPE_LAZY_DESTRUCTOR(NODE) \ (LANG_TYPE_CLASS_CHECK (NODE)->lazy_destructor) +/* Nonzero means that NODE (a class type) has a constructor that can throw. */ +#define CLASSTYPE_CTOR_MAY_THROW(NODE) \ + (LANG_TYPE_CLASS_CHECK (NODE)->ctor_may_throw) + /* Nonzero means that NODE (a class type) is final */ #define CLASSTYPE_FINAL(NODE) \ TYPE_FINAL_P (NODE) diff --git gcc/cp/decl.c gcc/cp/decl.c index 10e3079beed..c5799459210 100644 --- gcc/cp/decl.c +++ gcc/cp/decl.c @@ -15651,7 +15651,11 @@ finish_function (bool inline_p) if (!processing_template_decl && !cp_function_chain->can_throw && !flag_non_call_exceptions - && !decl_replaceable_p (fndecl)) + && !decl_replaceable_p (fndecl) + /* If FNDECL is a constructor of a class that can call a throwing + constructor, don't mark it as non-throwing. */ + && (!DECL_CONSTRUCTOR_P (fndecl) + || !CLASSTYPE_CTOR_MAY_THROW (DECL_CONTEXT (fndecl)))) TREE_NOTHROW (fndecl) = 1; /* This must come after expand_function_end because cleanups might diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C index e69de29bb2d..264c6c7a7a0 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw1.C @@ -0,0 +1,29 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X (int f) : n {init (f)} {} + int n; +}; + +struct P { + X x{20}; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +} diff --git gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C index e69de29bb2d..24906374fa4 100644 --- gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C +++ gcc/testsuite/g++.dg/cpp0x/initlist-throw2.C @@ -0,0 +1,33 @@ +// PR c++/85363 +// { dg-do run { target c++11 } } + +int +init (int f) +{ + throw f; +} + +struct X { + X () : n {init (42)} {} + int n; +}; + +struct P { + struct R { + struct Q { + X x = {}; + } q; + } r; +}; + +int +main () +{ + try { + P p {}; + } + catch (int n) { + return 0; + } + return 1; +}