| Message ID | 1513798587-14927-1-git-send-email-dmalcolm@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Wed, Dec 20, 2017 at 2:36 PM, David Malcolm <dmalcolm@redhat.com> wrote: > On Tue, 2017-12-19 at 23:22 -0500, Jason Merrill wrote: >> On Tue, Dec 19, 2017 at 7:53 PM, David Malcolm <dmalcolm@redhat.com> >> wrote: >> > On Tue, 2017-12-19 at 15:35 -0500, Jason Merrill wrote: >> > > On Sat, Dec 16, 2017 at 8:12 PM, David Malcolm <dmalcolm@redhat.c >> > > om> >> > > wrote: >> > > > I rebased the v2 patchkit; here's an extra patch to fix an >> > > > issue >> > > > with it uncovered by a recently-added testcase (in r254990). >> > > > >> > > > With the patch kit, but without this patch, g++'s >> > > > c-c++-common/pr83059.c >> > > > fails to emit the "invalid memory model argument 6" warning. >> > > > >> > > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu, as >> > > > part of the kit. >> > > > >> > > > Is this OK for trunk, assuming the rest of the kit is approved? >> > > > >> > > > gcc/c-family/ChangeLog: >> > > > * c-common.c (get_atomic_generic_size): Call >> > > > fold_for_warn >> > > > on the >> > > > params before checking for INTEGER_CST. >> > > > --- >> > > > gcc/c-family/c-common.c | 2 +- >> > > > 1 file changed, 1 insertion(+), 1 deletion(-) >> > > > >> > > > diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c >> > > > index 3438b87..ab03b7d 100644 >> > > > --- a/gcc/c-family/c-common.c >> > > > +++ b/gcc/c-family/c-common.c >> > > > @@ -6720,7 +6720,7 @@ get_atomic_generic_size (location_t loc, >> > > > tree >> > > > function, >> > > > /* Check memory model parameters for validity. */ >> > > > for (x = n_param - n_model ; x < n_param; x++) >> > > > { >> > > > - tree p = (*params)[x]; >> > > > + tree p = fold_for_warn ((*params)[x]); >> > > > if (TREE_CODE (p) == INTEGER_CST) >> > > > { >> > > > /* memmodel_base masks the low 16 bits, thus ignore >> > > > any >> > > > bits above >> > > >> > > Let's check the error case before we call fold_for_warn. >> > > >> > > Jason >> > >> > Do you mean like this? (bootstrapped; regrtest in progress) >> >> Ah, no, sorry I wasn't clear. I meant to reorder the if/else there, >> so we check for INTEGER_TYPE first and potentially give an error, >> then >> fold, and then potentially warn. >> >> Jason > > Aha - thanks! > > Here's an updated version of the patch. > > Successfully bootstrapped®rtested on x86_64-pc-linux-gnu, as > part of the kit. > > OK for trunk once the rest of the kit is approved? OK. Jason
diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c index 3438b87..7cc749b 100644 --- a/gcc/c-family/c-common.c +++ b/gcc/c-family/c-common.c @@ -6721,8 +6721,15 @@ get_atomic_generic_size (location_t loc, tree function, for (x = n_param - n_model ; x < n_param; x++) { tree p = (*params)[x]; + if (!INTEGRAL_TYPE_P (TREE_TYPE (p))) + { + error_at (loc, "non-integer memory model argument %d of %qE", x + 1, + function); + return 0; + } + p = fold_for_warn (p); if (TREE_CODE (p) == INTEGER_CST) - { + { /* memmodel_base masks the low 16 bits, thus ignore any bits above it by using TREE_INT_CST_LOW instead of tree_to_*hwi. Those high bits will be checked later during expansion in target specific @@ -6732,14 +6739,7 @@ get_atomic_generic_size (location_t loc, tree function, "invalid memory model argument %d of %qE", x + 1, function); } - else - if (!INTEGRAL_TYPE_P (TREE_TYPE (p))) - { - error_at (loc, "non-integer memory model argument %d of %qE", x + 1, - function); - return 0; - } - } + } return size_0; }