[PATCH] PR fortran/70070 - ICE on initializing character data beyond min/max bound
Dear all,
the attached patch is pretty much self-explaining: check for bounds violation
when initializing a substring in a data statement and treat the resulting error.
If more detailed information should be emitted with the error message, I'm
open for suggestions.
Regtested on x86_64-pc-linux-gnu.
OK for master?
Thanks,
Harald
PR fortran/70070 - ICE on initializing character data beyond min/max bound
Check for initialization of substrings beyond bounds in DATA statements.
gcc/fortran/ChangeLog:
PR fortran/70070
* data.c (create_character_initializer): Check substring indices
against bounds.
(gfc_assign_data_value): Catch error returned from
create_character_initializer.
gcc/testsuite/ChangeLog:
PR fortran/70070
* gfortran.dg/pr70070.f90: New test.
diff --git a/gcc/fortran/data.c b/gcc/fortran/data.c
index 1313b335c86..d9f0b45da9b 100644
--- a/gcc/fortran/data.c
+++ b/gcc/fortran/data.c
@@ -183,6 +183,13 @@ create_character_initializer (gfc_expr *init, gfc_typespec *ts,
}
}
+ if (start < 0 || end > init->value.character.length)
+{
+ gfc_error ("Invalid substring in DATA statement at %L",
+ &ref->u.ss.start->where);
+ return NULL;
+}
+
if (rvalue->ts.type == BT_HOLLERITH)
{
for (size_t i = 0; i < (size_t) len; i++)
@@ -576,6 +583,8 @@ gfc_assign_data_value (gfc_expr *lvalue, gfc_expr *rvalue, mpz_t index,
if (lvalue->ts.u.cl->length == NULL && !(ref && ref->u.ss.length != NULL))
return false;
expr = create_character_initializer (init, last_ts, ref, rvalue);
+ if (!expr)
+ return false;
}
else
{
diff --git a/gcc/testsuite/gfortran.dg/pr70070.f90 b/gcc/testsuite/gfortran.dg/pr70070.f90
new file mode 100644
index 000..c79cd229552
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr70070.f90
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! PR fortran/70070 - ICE on initializing character data beyond min/max bound
+
+program p
+ character(1) :: a, b
+ data (a(i:i),i=0,0) /1*'#'/ ! { dg-error "Invalid substring" }
+ data (b(i:i),i=1,2) /2*'#'/ ! { dg-error "Invalid substring" }
+end
Re: [PATCH v2] c++: Add support for -std=c++2b
On Tue, Jan 19, 2021 at 11:28 PM Jason Merrill wrote:
>
> On 1/10/21 7:28 PM, Paul Fee via Gcc-patches wrote:
> > [PATCH v2] c++: Add support for -std=c++2b
>
> Thanks!
>
> This patch was corrupted by word wrap, so it won't apply; if you can't
> suppress word wrap in your mail client, please send the patch as an
> attachment instead.
Seems that "Plain text mode" in gmail isn't plain enough. I'll use an
attachment for patch v3.
>
> Also remember to use git gcc-verify before sending the patch.
Thanks for the tip. It helps me learn the GCC patch posting process.
>
> > Derived from the changes that added C++2a support in 2017.
> > https://gcc.gnu.org/g:026a79f70cf33f836ea5275eda72d4870a3041e5
> >
> > No C++2b features are added here.
> > Use of -std=c++2b sets __cplusplus to 202100L.
> >
> > $ g++ -std=c++2b -dM -E -x c++ - < /dev/null | grep cplusplus
> > #define __cplusplus 202100L
> >
> > Changes since v1 (8th Jan 2021):
> > * As suggested by Jonathan Wakely:
> >__cplusplus set to 202100L rather than 202101L. Use of a non-existent
> > date
> >helps indicate this is not a true standard, yet is a value greater
> > than 202002L.
> > * As suggested by Jakub Jelinek:
> >Fixed typos and formatting.
> >Added C++23 support to dwarf2out.c, including missing C++20 support
> > in highest_c_language.
>
> > * Regarding suggestion by Marek Polacek to refer to C++23 rather than C++2b.
> >Left the option as -std=c++2b for now. It may be premature to assume
> > the next
> >version of the standard will be named C++23. Use of c++2b also
> > reinforces
> >the experimental nature of GCC's C++23 implementation.
>
> Hmm, I don't think it's that premature; the C++ committee has been very
> serious about time-based releases every three years. I think it makes
> sense for the advertised flag to be c++2b, but let's also go ahead and
> add the c++23 flags as hidden, and use cxx23 internally.
Ok, I'll change from only exposing -std=c++2b to the end goal of
-std=c++23. You pointed out below a flaw in target-supports.exp.
Checking "make check-c++", I found that I'd also broken many tests
while adjusting c++2a to be c++20. I'll take the 2a->20 change out of
this patch. It can be in a separate patch. Seeing the cost of
changing from "2a" to "20", I'll post up patch v3 with support for
-std=c++23 so that a "2b->23" change isn't needed in a few years.
GCC's C++23 will still be experimental, despite the flag be
-std=c++23.
>
> > gcc/
> >
> > Add support for -std=c++2b
> > * doc/cpp.texi (__cplusplus): Document value for -std=c++2b
> > or -std=gnu++2b.
> > * doc/invoke.texi: Document -std=c++2b and -std=gnu++2b.
> >
> > gcc/c-family
> >
> > Add support for -std=c++2b
> > * c-common.h (cxx_dialect): Add cxx2b as a dialect.
> > * c.opt: Add options for -std=c++2b and -std=gnu++2b.
> > * c-opts.c (set_std_cxx2b): New.
> > (c_common_handle_option): Set options when -std=c++2b is enabled.
> > (c_common_post_options): Adjust comments.
> > (set_std_cxx20): Likewise.
> > * dwarf2out.c (highest_c_language): Recognise C++20 and C++23.
> > (gen_compile_unit_die): Recognise C++23.
>
> dwarf2out.c isn't in c-family.
>
> > gcc/testsuite
> >
> > Add support for -std=c++2b
> > * lib/target-supports.exp (check_effective_target_c++2a_only):
> > rename to check_effective_target_c++20_only.
> > (check_effective_target_c++2a): rename to check_effective_target_c++20.
> > (check_effective_target_c++20): Return 1
> > if check_effective_target_c++20_only or
> > if check_effective_target_c++2b.
> > (check_effective_target_c++20_down): New.
> > (check_effective_target_c++2a_only): New.
> > (check_effective_target_c++2a): New.
> > * g++.dg/cpp2b/cplusplus.C: New.
> >
> > libcpp
> > Add support for -std=c++2b
> > * include/cpplib.h (c_lang): Add CXX2B and GNUCXX2B.
> > * init.c (lang_defaults): Add rows for CXX2B and GNUCXX2B.
> > (cpp_init_builtins): Set __cplusplus to 202100L for C++2b.
> > ---
> > gcc/c-family/c-common.h|4 ++-
> > gcc/c-family/c-opts.c | 29 ++--
> > gcc/c-family/c.opt |8 ++
> > gcc/doc/cpp.texi |7 +++--
> > gcc/doc/invoke.texi| 10
> > gcc/dwarf2out.c|7 +
> > gcc/testsuite/g++.dg/cpp2b/cplusplus.C |4 +++
> > gcc/testsuite/lib/target-supports.exp | 39
> > +++--
> > libcpp/include/cpplib.h|3 +-
> > libcpp/init.c |7 +
> > 10 files changed, 98 insertions(+), 20 deletions(-)
> >
> > diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
> > index a65c78f7240..f562cdebf4c 100644
> > --- a/gcc/c-family/c-common.h
> > +++ b/gcc/c-family/c-common.h
> > @@ -738,7 +738,9 @@ enum cxx_dialect {
> >
[PATCH v3] c++: Add support for -std=c++23
Derived from the changes that added C++2a support in 2017.
https://gcc.gnu.org/g:026a79f70cf33f836ea5275eda72d4870a3041e5
No C++23 features are added here.
Use of -std=c++23 sets __cplusplus to 202100L.
$ g++ -std=c++23 -dM -E -x c++ - < /dev/null | grep cplusplus
#define __cplusplus 202100L
Changes since v2 (11th Jan 2021):
* Dropped testsuite change (c++2a to c++20).
* As suggested by Marek Polacek and Jason Merrill
Added -std=c++23 since three year cadence is well established.
Author: Paul Fee
Date: Mon Jan 25 00:12:03 2021 +
Add support for -std=c++23
gcc/
* doc/cpp.texi (__cplusplus): Document value for -std=c++23
or -std=gnu++23.
* doc/invoke.texi: Document -std=c++23 and -std=gnu++23.
* dwarf2out.c (highest_c_language): Recognise C++20 and C++23.
(gen_compile_unit_die): Recognise C++23.
gcc/c-family/
* c-common.h (cxx_dialect): Add cxx23 as a dialect.
* c.opt: Add options for -std=c++23, std=c++2b, -std=gnu++23
and -std=gnu++2b
* c-opts.c (set_std_cxx23): New.
(c_common_handle_option): Set options when -std=c++23 is enabled.
(c_common_post_options): Adjust comments.
(set_std_cxx20): Likewise.
gcc/testsuite/
* lib/target-supports.exp (check_effective_target_c++2a):
Check for C++2a or C++23.
(check_effective_target_c++2a_down): New.
(check_effective_target_c++20_down): New.
(check_effective_target_c++23_only): New.
(check_effective_target_c++23): New.
* g++.dg/cpp23/cplusplus.C: New.
libcpp/
* include/cpplib.h (c_lang): Add CXX23 and GNUCXX23.
* init.c (lang_defaults): Add rows for CXX23 and GNUCXX23.
(cpp_init_builtins): Set __cplusplus to 202100L for C++23.
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index a65c78f7240..f30b6c6ac33 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -738,7 +738,9 @@ enum cxx_dialect {
/* C++17 */
cxx17,
/* C++20 */
- cxx20
+ cxx20,
+ /* C++23 */
+ cxx23
};
/* The C++ dialect being used. C++98 is the default. */
diff --git a/gcc/c-family/c-opts.c b/gcc/c-family/c-opts.c
index 3cdf41bc6e2..bd15b9cd902 100644
--- a/gcc/c-family/c-opts.c
+++ b/gcc/c-family/c-opts.c
@@ -113,6 +113,7 @@ static void set_std_cxx11 (int);
static void set_std_cxx14 (int);
static void set_std_cxx17 (int);
static void set_std_cxx20 (int);
+static void set_std_cxx23 (int);
static void set_std_c89 (int, int);
static void set_std_c99 (int);
static void set_std_c11 (int);
@@ -649,6 +650,12 @@ c_common_handle_option (size_t scode, const char *arg, HOST_WIDE_INT value,
set_std_cxx20 (code == OPT_std_c__20 /* ISO */);
break;
+case OPT_std_c__23:
+case OPT_std_gnu__23:
+ if (!preprocessing_asm_p)
+ set_std_cxx23 (code == OPT_std_c__23 /* ISO */);
+ break;
+
case OPT_std_c90:
case OPT_std_iso9899_199409:
if (!preprocessing_asm_p)
@@ -1019,7 +1026,7 @@ c_common_post_options (const char **pfilename)
warn_narrowing = 1;
/* Unless -f{,no-}ext-numeric-literals has been used explicitly,
- for -std=c++{11,14,17,2a} default to -fno-ext-numeric-literals. */
+ for -std=c++{11,14,17,20,23} default to -fno-ext-numeric-literals. */
if (flag_iso && !global_options_set.x_flag_ext_numeric_literals)
cpp_opts->ext_numeric_literals = 0;
}
@@ -1763,7 +1770,7 @@ set_std_cxx20 (int iso)
flag_no_gnu_keywords = iso;
flag_no_nonansi_builtin = iso;
flag_iso = iso;
- /* C++17 includes the C11 standard library. */
+ /* C++20 includes the C11 standard library. */
flag_isoc94 = 1;
flag_isoc99 = 1;
flag_isoc11 = 1;
@@ -1773,6 +1780,24 @@ set_std_cxx20 (int iso)
lang_hooks.name = "GNU C++20";
}
+/* Set the C++ 2023 standard (without GNU extensions if ISO). */
+static void
+set_std_cxx23 (int iso)
+{
+ cpp_set_lang (parse_in, iso ? CLK_CXX23: CLK_GNUCXX23);
+ flag_no_gnu_keywords = iso;
+ flag_no_nonansi_builtin = iso;
+ flag_iso = iso;
+ /* C++23 includes the C11 standard library. */
+ flag_isoc94 = 1;
+ flag_isoc99 = 1;
+ flag_isoc11 = 1;
+ /* C++23 includes coroutines. */
+ flag_coroutines = true;
+ cxx_dialect = cxx23;
+ lang_hooks.name = "GNU C++23";
+}
+
/* Args to -d specify what to dump. Silently ignore
unrecognized options; they may be aimed at toplev.c. */
static void
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 1766364806e..880ab424d50 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -2214,6 +2214,14 @@ std=c++20
C++ ObjC++
Conform to the ISO 2020 C++ draft standard (experimental and incomplete support).
+std=c++2b
+C++ ObjC++ Alias(std=c++23) Undocumented
+Conform to the ISO 2020 C++ draft standard (experimental and incomplete support).
+
+std=c++23
+C++ ObjC++
+Conform to the ISO 2023 C++ draft standard (experimental and incomplete suppor
Re: [PATCH] fold: Fix up strn{case,}cmp folding [PR98771]
On Fri, 22 Jan 2021, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, the compiler behaves differently during strncmp
> and strncasecmp folding between 32-bit and 64-bit hosts targeting 64-bit
> target. I think that is highly undesirable.
>
> The culprit is the host_size_t_cst_p predicate that is used by
> fold_const_call, which punts if the target size_t constants don't fit into
> host size_t. This patch gets rid of that behavior, instead it punts the
> same when it doesn't fit into uhwi.
>
> The predicate was used for strncmp and strncasecmp folding and for bcmp,
> memcmp and
> memchr folding.
> The constant is in all cases compared to 0, we can do that whether it fits
> into size_t or unsigned HOST_WIDE_INT, then it is used in s2 <= s0 or
> s2 <= s1 comparisons where s0 and s1 already have uhwi type and represent
> the sizes of the objects.
> The important difference is for strn{,case}cmp folding, we pass that s2
> value as the last argument to the host functions comparing the c_getstr
> results. If s2 fits into size_t, then my patch makes no difference,
> but if it is larger, we know the 2 c_getstr objects need to fit into the
> host address space, so larger s2 should just act essentially as strcmp
> or strcasecmp; as none of those objects can occupy 100% of the address
> space, using MIN (SIZE_MAX, s2) achieves that.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2021-01-22 Jakub Jelinek
>
> PR testsuite/98771
> * fold-const-call.c (host_size_t_cst_p): Renamed to ...
> (size_t_cst_p): ... this. Check and store unsigned HOST_WIDE_INT
> value rather than host size_t.
> (fold_const_call): Change type of s2 from size_t to
> unsigned HOST_WIDE_INT. Use size_t_cst_p instead of
> host_size_t_cst_p. For strncmp calls, pass MIN (s2, SIZE_MAX)
> instead of s2 as last argument.
>
> --- gcc/fold-const-call.c.jj 2021-01-04 10:25:37.902244366 +0100
> +++ gcc/fold-const-call.c 2021-01-22 11:23:31.571000548 +0100
> @@ -53,16 +53,15 @@ complex_cst_p (tree t)
>return TREE_CODE (t) == COMPLEX_CST;
> }
>
> -/* Return true if ARG is a constant in the range of the host size_t.
> +/* Return true if ARG is a size_type_node constant.
> Store it in *SIZE_OUT if so. */
>
> static inline bool
> -host_size_t_cst_p (tree t, size_t *size_out)
> +size_t_cst_p (tree t, unsigned HOST_WIDE_INT *size_out)
> {
>if (types_compatible_p (size_type_node, TREE_TYPE (t))
>&& integer_cst_p (t)
> - && (wi::min_precision (wi::to_wide (t), UNSIGNED)
> - <= sizeof (size_t) * CHAR_BIT))
> + && tree_fits_uhwi_p (t))
> {
>*size_out = tree_to_uhwi (t);
>return true;
> @@ -1767,23 +1766,22 @@ fold_const_call (combined_fn fn, tree ty
> {
>const char *p0, *p1;
>char c;
> - unsigned HOST_WIDE_INT s0, s1;
> - size_t s2 = 0;
> + unsigned HOST_WIDE_INT s0, s1, s2 = 0;
>switch (fn)
> {
> case CFN_BUILT_IN_STRNCMP:
> - if (!host_size_t_cst_p (arg2, &s2))
> + if (!size_t_cst_p (arg2, &s2))
> return NULL_TREE;
>if (s2 == 0
> && !TREE_SIDE_EFFECTS (arg0)
> && !TREE_SIDE_EFFECTS (arg1))
> return build_int_cst (type, 0);
>else if ((p0 = c_getstr (arg0)) && (p1 = c_getstr (arg1)))
> - return build_int_cst (type, strncmp (p0, p1, s2));
> + return build_int_cst (type, strncmp (p0, p1, MIN (s2, SIZE_MAX)));
>return NULL_TREE;
>
> case CFN_BUILT_IN_STRNCASECMP:
> - if (!host_size_t_cst_p (arg2, &s2))
> + if (!size_t_cst_p (arg2, &s2))
> return NULL_TREE;
>if (s2 == 0
> && !TREE_SIDE_EFFECTS (arg0)
> @@ -1791,13 +1789,13 @@ fold_const_call (combined_fn fn, tree ty
> return build_int_cst (type, 0);
>else if ((p0 = c_getstr (arg0))
> && (p1 = c_getstr (arg1))
> -&& strncmp (p0, p1, s2) == 0)
> +&& strncmp (p0, p1, MIN (s2, SIZE_MAX)) == 0)
> return build_int_cst (type, 0);
>return NULL_TREE;
>
> case CFN_BUILT_IN_BCMP:
> case CFN_BUILT_IN_MEMCMP:
> - if (!host_size_t_cst_p (arg2, &s2))
> + if (!size_t_cst_p (arg2, &s2))
> return NULL_TREE;
>if (s2 == 0
> && !TREE_SIDE_EFFECTS (arg0)
> @@ -1811,7 +1809,7 @@ fold_const_call (combined_fn fn, tree ty
>return NULL_TREE;
>
> case CFN_BUILT_IN_MEMCHR:
> - if (!host_size_t_cst_p (arg2, &s2))
> + if (!size_t_cst_p (arg2, &s2))
> return NULL_TREE;
>if (s2 == 0
> && !TREE_SIDE_EFFECTS (arg0)
>
> Jakub
>
>
--
Richard Biener
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imendörffer; HRB 36809 (AG Nuernberg)
Re: [PATCH 1/4] unroll: Add middle-end unroll factor estimation
On Fri, 22 Jan 2021, Segher Boessenkool wrote: > On Fri, Jan 22, 2021 at 02:47:06PM +0100, Richard Biener wrote: > > On Thu, 21 Jan 2021, Segher Boessenkool wrote: > > > What is holding up this patch still? Ke Wen has pinged it every month > > > since May, and there has still not been a review. > > Richard Sandiford wrote: > > FAOD (since I'm on cc:), I don't feel qualified to review this. > > Tree-level loop stuff isn't really my area. > > And Richard Biener wrote: > > I don't like it, it feels wrong but I don't have a good suggestion > > that had positive feedback. Since a reviewer / approver is indirectly > > responsible for at least the design I do not want to ack this patch. > > Bin made forward progress on the other parts of the series but clearly > > there's somebody missing with the appropriate privileges who feels > > positive about the patch and its general direction. > > > > Sorry to be of no help here. > > How unfortunate :-( > > So, first off, this will then have to work for next stage 1 to make any > progress. Rats. > > But what could have been done differently that would have helped? Of > course Ke Wen could have written a better patch (aka one that is more > acceptable); either of you could have made your current replies earlier, > so that it is clear help needs to be sought elsewhere; and I could have > pushed people earlier, too. No one really did anything wrong, I'm not > seeking who to blame, I'm just trying to find out how to prevent > deadlocks like this in the future (where one party waits for replies > that will never come). > > Is it just that we have a big gaping hole in reviewers with experience > in such loop optimisations? May be. But what I think is the biggest problem is that we do not have a good way to achieve what the patch tries (if you review the communications you'll see many ideas tossed around) first and foremost because IV selection is happening early on GIMPLE and unrolling happens late on RTL. Both need a quite accurate estimate of costs but unrolling has an ever harder time than IV selection where we've got along with throwing dummy RTL at costing functions. IMHO the patch is the wrong "start" to try fixing the issue and my fear is that wiring this kind of "features" into the current (fundamentally broken) state will make it much harder to rework that state without introducing regressions on said features (I'm there with trying to turn the vectorizer upside down - for three years now, struggling to not regress any of the "features" we've accumulated for various targets where most of them feel a "bolted-on" rather than well-designed ;/). I think IV selection and unrolling (and scheduling FWIW) need to move closer together. I do not have a good idea how that can work out though but I very much believe that this "most wanted" GIMPLE unroller will not be a good way of progressing here. Maybe taking the bullet and moving IV selection back to RTL is the answer. For a "short term" solution I still think that trying to perform unrolling and IV selection (for the D-form case you're targeting) at the same time is a better design, even if it means complicating the IV selection pass (and yeah, it'll still be at GIMPLE and w/o any good idea about scheduling). There are currently 20+ GIMPLE optimization passes and 10+ RTL optimization passes between IV selection and unrolling, the idea that you can have transform decision and transform apply this far apart looks scary. Richard.
