[Patch][v2] OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
Hi Jakub, hi all,
updated patch included, i.e. avoiding 'count' for 'j' when a 'j.0' would
do (i.e. only local var without the different step calculation). I also
now reject if there is a non-unit step on the loop using an outer var.
Eventually still to be done: replace the 'sorry' by working code, i.e.
implement the suggestions to handle some/all non-unit iteration steps as
proposed in this thread.
On 20.01.23 18:39, Jakub Jelinek wrote:
I think instead of non-unity etc. it is better to talk about constant
step 1 or -1.
I concur.
The actual problem with non-simple loops for non-rectangular loops is
both in case it is an inner loop which uses some outer loop's iterator,
or if it is outer loop whose iterator is used, both of those cases
will not be handled properly.
I have now added a check for the other case as well.
Just to confirm, the following is fine, isn't it?
!$omp simd collapse(4)
do i = 1, 10, 2
do outer_var = 1, 10 ! step = + 1
do j = 1, 10, 2
do inner_var = 1, outer_var ! step = 1
i.e. both the inner_var and outer_var have 'step = 1',
even if other loops in the 'collapse' have step != 1.
I think it should be fine.
OK mainline?
Tobias
-
Siemens Electronic Design Automation GmbH; Anschrift: Arnulfstraße 201, 80634
München; Gesellschaft mit beschränkter Haftung; Geschäftsführer: Thomas
Heurung, Frank Thürauf; Sitz der Gesellschaft: München; Registergericht
München, HRB 106955
OpenMP/Fortran: Partially fix non-rect loop nests [PR107424]
This patch ensures that loop bounds depending on outer loop vars use the
proper TREE_VEC format. It additionally gives a sorry if such an outer
var has a non-one/non-minus-one increment as currently a count variable
is used in this case (see PR).
Finally, it avoids 'count' and just uses a local loop variable if the
step increment is +/-1.
PR fortran/107424
gcc/fortran/ChangeLog:
* trans-openmp.cc (struct dovar_init_d): Add 'sym' and
'non_unit_incr' members.
(gfc_nonrect_loop_expr): New.
(gfc_trans_omp_do): Call it; use normal loop bounds
for unit stride - and only create local loop var.
libgomp/ChangeLog:
* testsuite/libgomp.fortran/non-rectangular-loop-1.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-1a.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-2.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-3.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-4.f90: New test.
* testsuite/libgomp.fortran/non-rectangular-loop-5.f90: New test.
gcc/testsuite/ChangeLog:
* gfortran.dg/goacc/privatization-1-compute-loop.f90: Update dg-note.
* gfortran.dg/goacc/privatization-1-routine_gang-loop.f90: Likewise.
gcc/fortran/trans-openmp.cc| 238 ++--
.../goacc/privatization-1-compute-loop.f90 | 6 +-
.../goacc/privatization-1-routine_gang-loop.f90| 3 +-
.../libgomp.fortran/non-rectangular-loop-1.f90 | 637 +
.../libgomp.fortran/non-rectangular-loop-1a.f90| 374
.../libgomp.fortran/non-rectangular-loop-2.f90 | 243
.../libgomp.fortran/non-rectangular-loop-3.f90 | 186 ++
.../libgomp.fortran/non-rectangular-loop-4.f90 | 188 ++
.../libgomp.fortran/non-rectangular-loop-5.f90 | 28 +
9 files changed, 1854 insertions(+), 49 deletions(-)
diff --git a/gcc/fortran/trans-openmp.cc b/gcc/fortran/trans-openmp.cc
index 87213de0918..ccee9e16648 100644
--- a/gcc/fortran/trans-openmp.cc
+++ b/gcc/fortran/trans-openmp.cc
@@ -5116,10 +5116,135 @@ gfc_trans_omp_critical (gfc_code *code)
}
typedef struct dovar_init_d {
+ gfc_symbol *sym;
tree var;
tree init;
+ bool non_unit_iter;
} dovar_init;
+static bool
+gfc_nonrect_loop_expr (stmtblock_t *pblock, gfc_se *sep, int loop_n,
+ gfc_code *code, gfc_expr *expr, vec *inits,
+ int simple, gfc_expr *curr_loop_var)
+{
+ int i;
+ for (i = 0; i < loop_n; i++)
+{
+ gcc_assert (code->ext.iterator->var->expr_type == EXPR_VARIABLE);
+ if (gfc_find_sym_in_expr (code->ext.iterator->var->symtree->n.sym, expr))
+ break;
+ code = code->block->next;
+}
+ if (i >= loop_n)
+return false;
+
+ /* Canonic format: TREE_VEC with [var, multiplier, offset]. */
+ gfc_symbol *var = code->ext.iterator->var->symtree->n.sym;
+
+ tree tree_var = NULL_TREE;
+ tree a1 = integer_one_node;
+ tree a2 = integer_zero_node;
+
+ if (!simple)
+{
+ /* FIXME: Handle non-unit iter steps, cf. PR fortran/107424. */
+ sorry_at (gfc_get_location (&curr_loop_var->where),
+ "non-rectangular loop nest with step other than constant 1 "
+ "or -1 for %qs", curr_loop_var->symtree->n.sym->name);
+ return false;
+}
+
+ dovar_init *di;
+ unsigned ix;
+ FOR_EACH_VEC_ELT (*inits, ix, di)
+if (di->sym == var && !di->non_unit_iter)
+ {
+ tree_var = di->init;
+ gcc_assert (DECL_P (tree_var));
+ break;
+ }
+else if (di->sym == var)
+ {
+ /* FIXME:
[PATCH, committed] Fortran: ICE in gfc_compare_array_spec [PR108528]
Dear all,
I've committed the attached simple and obvious patch by
Steve after regtesting on x86_64-pc-linux-gnu.
Instead of generating an internal error when wrong types
are passed to a bounds comparison, simply return false.
Committed:
https://gcc.gnu.org/g:9fb9da3d38513d320bfea72050f7a59688595e0b
Thanks,
Harald
From 9fb9da3d38513d320bfea72050f7a59688595e0b Mon Sep 17 00:00:00 2001
From: Steve Kargl
Date: Wed, 25 Jan 2023 20:38:43 +0100
Subject: [PATCH] Fortran: ICE in gfc_compare_array_spec [PR108528]
gcc/fortran/ChangeLog:
PR fortran/108528
* array.cc (compare_bounds): Return false instead of generating an
internal error on an invalid argument type.
gcc/testsuite/ChangeLog:
PR fortran/108528
* gfortran.dg/pr108528.f90: New test.
---
gcc/fortran/array.cc | 4 ++--
gcc/testsuite/gfortran.dg/pr108528.f90 | 9 +
2 files changed, 11 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr108528.f90
diff --git a/gcc/fortran/array.cc b/gcc/fortran/array.cc
index e8a2c32a627..be5eb8b6a0f 100644
--- a/gcc/fortran/array.cc
+++ b/gcc/fortran/array.cc
@@ -967,7 +967,7 @@ gfc_copy_array_spec (gfc_array_spec *src)
/* Returns nonzero if the two expressions are equal.
- We should not need to support more than constant values, as thatâs what is
+ We should not need to support more than constant values, as that's what is
allowed in derived type component array spec. However, we may create types
with non-constant array spec for dummy variable class container types, for
which the _data component holds the array spec of the variable declaration.
@@ -979,7 +979,7 @@ compare_bounds (gfc_expr *bound1, gfc_expr *bound2)
if (bound1 == NULL || bound2 == NULL
|| bound1->ts.type != BT_INTEGER
|| bound2->ts.type != BT_INTEGER)
-gfc_internal_error ("gfc_compare_array_spec(): Array spec clobbered");
+return false;
/* What qualifies as identical bounds? We could probably just check that the
expressions are exact clones. We avoid rewriting a specific comparison
diff --git a/gcc/testsuite/gfortran.dg/pr108528.f90 b/gcc/testsuite/gfortran.dg/pr108528.f90
new file mode 100644
index 000..7a353cb7eab
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108528.f90
@@ -0,0 +1,9 @@
+! { dg-do compile }
+! PR fortran/108528 -
+! Contributed by G.Steinmetz
+
+function f() ! { dg-error "mismatched array specifications" }
+ integer :: f((2.)) ! { dg-error "must be of INTEGER type" }
+ integer :: g((2))
+entry g()
+end
--
2.35.3
[PATCH v5 0/5] P1689R5 support
Hi, This patch series adds initial support for ISO C++'s [P1689R5][], a format for describing C++ module requirements and provisions based on the source code. This is required because compiling C++ with modules is not embarrassingly parallel and need to be ordered to ensure that `import some_module;` can be satisfied in time by making sure that any TU with `export import some_module;` is compiled first. [P1689R5]: https://isocpp.org/files/papers/P1689R5.html I've also added patches to include imported module CMI files and the module mapper file as dependencies of the compilation. I briefly looked into adding dependencies on response files as well, but that appeared to need some code contortions to have a `class mkdeps` available before parsing the command line or to keep the information around until one was made. I'd like feedback on the approach taken here with respect to the user-visible flags. I'll also note that header units are not supported at this time because the current `-E` behavior with respect to `import ;` is to search for an appropriate `.gcm` file which is not something such a "scan" can support. A new mode will likely need to be created (e.g., replacing `-E` with `-fc++-module-scanning` or something) where headers are looked up "normally" and processed only as much as scanning requires. FWIW, Clang as taken an alternate approach with its `clang-scan-deps` tool rather than using the compiler directly. Thanks, --Ben --- v4 -> v5: - add dependency tracking for imported modules to `-MF` - add dependency tracking for static module mapper files given to `-fmodule-mapper=` v3 -> v4: - add missing spaces between function names and arguments v2 -> v3: - changelog entries moved to commit messages - documentation updated/added in the UTF-8 routine editing v1 -> v2: - removal of the `deps_write(extra)` parameter to option-checking where ndeeded - default parameter of `cpp_finish(fdeps_stream = NULL)` - unification of libcpp UTF-8 validity functions from v1 - test cases for flag parsing states (depflags-*) and p1689 output (p1689-*) Ben Boeckel (5): libcpp: reject codepoints above 0x10 libcpp: add a function to determine UTF-8 validity of a C string p1689r5: initial support c++modules: report imported CMI files as dependencies c++modules: report module mapper files as a dependency gcc/c-family/c-opts.cc| 40 +++- gcc/c-family/c.opt| 12 + gcc/cp/mapper-client.cc | 4 + gcc/cp/mapper-client.h| 1 + gcc/cp/module.cc | 23 +- gcc/doc/invoke.texi | 15 ++ gcc/testsuite/g++.dg/modules/depflags-f-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-f.C | 1 + gcc/testsuite/g++.dg/modules/depflags-fi.C| 3 + gcc/testsuite/g++.dg/modules/depflags-fj-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-fj.C| 4 + .../g++.dg/modules/depflags-fjo-MD.C | 4 + gcc/testsuite/g++.dg/modules/depflags-fjo.C | 5 + gcc/testsuite/g++.dg/modules/depflags-fo-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-fo.C| 4 + gcc/testsuite/g++.dg/modules/depflags-j-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-j.C | 3 + gcc/testsuite/g++.dg/modules/depflags-jo-MD.C | 3 + gcc/testsuite/g++.dg/modules/depflags-jo.C| 4 + gcc/testsuite/g++.dg/modules/depflags-o-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-o.C | 3 + gcc/testsuite/g++.dg/modules/modules.exp | 1 + gcc/testsuite/g++.dg/modules/p1689-1.C| 18 ++ gcc/testsuite/g++.dg/modules/p1689-1.exp.json | 27 +++ gcc/testsuite/g++.dg/modules/p1689-2.C| 16 ++ gcc/testsuite/g++.dg/modules/p1689-2.exp.json | 16 ++ gcc/testsuite/g++.dg/modules/p1689-3.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-3.exp.json | 16 ++ gcc/testsuite/g++.dg/modules/p1689-4.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-4.exp.json | 14 ++ gcc/testsuite/g++.dg/modules/p1689-5.C| 14 ++ gcc/testsuite/g++.dg/modules/p1689-5.exp.json | 14 ++ gcc/testsuite/g++.dg/modules/test-p1689.py| 222 ++ gcc/testsuite/lib/modules.exp | 71 ++ libcpp/charset.cc | 28 ++- libcpp/include/cpplib.h | 12 +- libcpp/include/mkdeps.h | 17 +- libcpp/init.cc| 13 +- libcpp/internal.h | 2 + libcpp/mkdeps.cc | 149 +++- 40 files changed, 789 insertions(+), 30 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f-MD.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-f.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fi.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj-MD.C create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fj.C create mo
[PATCH v5 1/5] libcpp: reject codepoints above 0x10FFFF
Unicode does not support such values because they are unrepresentable in UTF-16. libcpp/ * charset.cc: Reject encodings of codepoints above 0x10. UTF-16 does not support such codepoints and therefore all Unicode rejects such values. Signed-off-by: Ben Boeckel --- libcpp/charset.cc | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/libcpp/charset.cc b/libcpp/charset.cc index 3c47d4f868b..f7ae12ea5a2 100644 --- a/libcpp/charset.cc +++ b/libcpp/charset.cc @@ -158,6 +158,10 @@ struct _cpp_strbuf encoded as any of DF 80, E0 9F 80, F0 80 9F 80, F8 80 80 9F 80, or FC 80 80 80 9F 80. Only the first is valid. + Additionally, Unicode declares that all codepoints above 0010 are + invalid because they cannot be represented in UTF-16. As such, all 5- and + 6-byte encodings are invalid. + An implementation note: the transformation from UTF-16 to UTF-8, or vice versa, is easiest done by using UTF-32 as an intermediary. */ @@ -216,7 +220,7 @@ one_utf8_to_cppchar (const uchar **inbufp, size_t *inbytesleftp, if (c <= 0x3FF && nbytes > 5) return EILSEQ; /* Make sure the character is valid. */ - if (c > 0x7FFF || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; + if (c > 0x10 || (c >= 0xD800 && c <= 0xDFFF)) return EILSEQ; *cp = c; *inbufp = inbuf; @@ -320,7 +324,7 @@ one_utf32_to_utf8 (iconv_t bigend, const uchar **inbufp, size_t *inbytesleftp, s += inbuf[bigend ? 2 : 1] << 8; s += inbuf[bigend ? 3 : 0]; - if (s >= 0x7FFF || (s >= 0xD800 && s <= 0xDFFF)) + if (s > 0x10 || (s >= 0xD800 && s <= 0xDFFF)) return EILSEQ; rval = one_cppchar_to_utf8 (s, outbufp, outbytesleftp); -- 2.39.0
[PATCH v5 2/5] libcpp: add a function to determine UTF-8 validity of a C string
This simplifies the interface for other UTF-8 validity detections when a
simple "yes" or "no" answer is sufficient.
libcpp/
* charset.cc: Add `_cpp_valid_utf8_str` which determines whether
a C string is valid UTF-8 or not.
* internal.h: Add prototype for `_cpp_valid_utf8_str`.
Signed-off-by: Ben Boeckel
---
libcpp/charset.cc | 20
libcpp/internal.h | 2 ++
2 files changed, 22 insertions(+)
diff --git a/libcpp/charset.cc b/libcpp/charset.cc
index f7ae12ea5a2..616be9d02ee 100644
--- a/libcpp/charset.cc
+++ b/libcpp/charset.cc
@@ -1868,6 +1868,26 @@ _cpp_valid_utf8 (cpp_reader *pfile,
return true;
}
+/* Detect whether a C-string is a valid UTF-8-encoded set of bytes. Returns
+`false` if any contained byte sequence encodes an invalid Unicode codepoint
+or is not a valid UTF-8 sequence. Returns `true` otherwise. */
+
+extern bool
+_cpp_valid_utf8_str (const char *name)
+{
+ const uchar* in = (const uchar*)name;
+ size_t len = strlen (name);
+ cppchar_t cp;
+
+ while (*in)
+{
+ if (one_utf8_to_cppchar (&in, &len, &cp))
+ return false;
+}
+
+ return true;
+}
+
/* Subroutine of convert_hex and convert_oct. N is the representation
in the execution character set of a numeric escape; write it into the
string buffer TBUF and update the end-of-string pointer therein. WIDE
diff --git a/libcpp/internal.h b/libcpp/internal.h
index 9724676a8cd..48520901b2d 100644
--- a/libcpp/internal.h
+++ b/libcpp/internal.h
@@ -834,6 +834,8 @@ extern bool _cpp_valid_utf8 (cpp_reader *pfile,
struct normalize_state *nst,
cppchar_t *cp);
+extern bool _cpp_valid_utf8_str (const char *str);
+
extern void _cpp_destroy_iconv (cpp_reader *);
extern unsigned char *_cpp_convert_input (cpp_reader *, const char *,
unsigned char *, size_t, size_t,
--
2.39.0
[PATCH v5 3/5] p1689r5: initial support
This patch implements support for [P1689R5][] to communicate to a build system the C++20 module dependencies to build systems so that they may build `.gcm` files in the proper order. Support is communicated through the following three new flags: - `-fdeps-format=` specifies the format for the output. Currently named `p1689r5`. - `-fdeps-file=` specifies the path to the file to write the format to. - `-fdep-output=` specifies the `.o` that will be written for the TU that is scanned. This is required so that the build system can correlate the dependency output with the actual compilation that will occur. CMake supports this format as of 17 Jun 2022 (to be part of 3.25.0) using an experimental feature selection (to allow for future usage evolution without committing to how it works today). While it remains experimental, docs may be found in CMake's documentation for experimental features. Future work may include using this format for Fortran module dependencies as well, however this is still pending work. [P1689R5]: https://isocpp.org/files/papers/P1689R5.html [cmake-experimental]: https://gitlab.kitware.com/cmake/cmake/-/blob/master/Help/dev/experimental.rst TODO: - header-unit information fields Header units (including the standard library headers) are 100% unsupported right now because the `-E` mechanism wants to import their BMIs. A new mode (i.e., something more workable than existing `-E` behavior) that mocks up header units as if they were imported purely from their path and content would be required. - non-utf8 paths The current standard says that paths that are not unambiguously represented using UTF-8 are not supported (because these cases are rare and the extra complication is not worth it at this time). Future versions of the format might have ways of encoding non-UTF-8 paths. For now, this patch just doesn't support non-UTF-8 paths (ignoring the "unambiguously represetable in UTF-8" case). - figure out why junk gets placed at the end of the file Sometimes it seems like the file gets a lot of `NUL` bytes appended to it. It happens rarely and seems to be the result of some `ftruncate`-style call which results in extra padding in the contents. Noting it here as an observation at least. libcpp/ * include/cpplib.h: Add cpp_deps_format enum. (cpp_options): Add format field (cpp_finish): Add dependency stream parameter. * include/mkdeps.h (deps_add_module_target): Add new preprocessor parameter used for C++ module tracking. * init.cc (cpp_finish): Add new preprocessor parameter used for C++ module tracking. * mkdeps.cc (mkdeps): Implement P1689R5 output. gcc/ * doc/invoke.texi: Document -fdeps-format=, -fdep-file=, and -fdep-output= flags. gcc/c-family/ * c-opts.cc (c_common_handle_option): Add fdeps_file variable and -fdeps-format=, -fdep-file=, and -fdep-output= parsing. * c.opt: Add -fdeps-format=, -fdep-file=, and -fdep-output= flags. gcc/cp/ * module.cc (preprocessed_module): Pass whether the module is exported to dependency tracking. gcc/testsuite/ * g++.dg/modules/depflags-f-MD.C: New test. * g++.dg/modules/depflags-f.C: New test. * g++.dg/modules/depflags-fi.C: New test. * g++.dg/modules/depflags-fj-MD.C: New test. * g++.dg/modules/depflags-fj.C: New test. * g++.dg/modules/depflags-fjo-MD.C: New test. * g++.dg/modules/depflags-fjo.C: New test. * g++.dg/modules/depflags-fo-MD.C: New test. * g++.dg/modules/depflags-fo.C: New test. * g++.dg/modules/depflags-j-MD.C: New test. * g++.dg/modules/depflags-j.C: New test. * g++.dg/modules/depflags-jo-MD.C: New test. * g++.dg/modules/depflags-jo.C: New test. * g++.dg/modules/depflags-o-MD.C: New test. * g++.dg/modules/depflags-o.C: New test. * g++.dg/modules/p1689-1.C: New test. * g++.dg/modules/p1689-1.exp.json: New test expectation. * g++.dg/modules/p1689-2.C: New test. * g++.dg/modules/p1689-2.exp.json: New test expectation. * g++.dg/modules/p1689-3.C: New test. * g++.dg/modules/p1689-3.exp.json: New test expectation. * g++.dg/modules/p1689-4.C: New test. * g++.dg/modules/p1689-4.exp.json: New test expectation. * g++.dg/modules/p1689-5.C: New test. * g++.dg/modules/p1689-5.exp.json: New test expectation. * g++.dg/modules/modules.exp: Load new P1689 library routines. * g++.dg/modules/test-p1689.py: New tool for validating P1689 output. * lib/modules.exp: Support for validating P1689 outputs. Signed-off-by: Ben Boeckel --- gcc/c-family/c-opts.cc| 40 +++- gcc/c-family/c.opt| 12 + gcc/cp/module.cc | 3 +- gcc/doc/invoke.texi | 15 ++ gcc/testsuite/g++.dg/modules/depflags-f
[PATCH v5 4/5] c++modules: report imported CMI files as dependencies
They affect the build, so report them via `-MF` mechanisms.
gcc/cp/
* module.cc (do_import): Report imported CMI files as
dependencies.
Signed-off-by: Ben Boeckel
---
gcc/cp/module.cc | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index ebd30f63d81..dbd1b721616 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18966,6 +18966,8 @@ module_state::do_import (cpp_reader *reader, bool
outermost)
dump () && dump ("CMI is %s", file);
if (note_module_cmi_yes || inform_cmi_p)
inform (loc, "reading CMI %qs", file);
+ /* Add the CMI file to the dependency tracking. */
+ deps_add_dep (cpp_get_deps (reader), file);
fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY);
e = errno;
}
--
2.39.0
[PATCH v5 5/5] c++modules: report module mapper files as a dependency
It affects the build, and if used as a static file, can reliably be
tracked using the `-MF` mechanism.
gcc/cp/:
* mapper-client.cc, mapper-client.h (open_module_client): Accept
dependency tracking and track module mapper files as
dependencies.
* module.cc (make_mapper, get_mapper): Pass the dependency
tracking class down.
Signed-off-by: Ben Boeckel
---
gcc/cp/mapper-client.cc | 4
gcc/cp/mapper-client.h | 1 +
gcc/cp/module.cc| 18 +-
3 files changed, 14 insertions(+), 9 deletions(-)
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
index 39e80df2d25..0ce5679d659 100644
--- a/gcc/cp/mapper-client.cc
+++ b/gcc/cp/mapper-client.cc
@@ -34,6 +34,7 @@ along with GCC; see the file COPYING3. If not see
#include "diagnostic-core.h"
#include "mapper-client.h"
#include "intl.h"
+#include "mkdeps.h"
#include "../../c++tools/resolver.h"
@@ -132,6 +133,7 @@ spawn_mapper_program (char const **errmsg, std::string
&name,
module_client *
module_client::open_module_client (location_t loc, const char *o,
+ class mkdeps *deps,
void (*set_repo) (const char *),
char const *full_program_name)
{
@@ -285,6 +287,8 @@ module_client::open_module_client (location_t loc, const
char *o,
errmsg = "opening";
else
{
+ /* Add the mapper file to the dependency tracking. */
+ deps_add_dep (deps, name.c_str ());
if (int l = r->read_tuple_file (fd, ident, false))
{
if (l > 0)
diff --git a/gcc/cp/mapper-client.h b/gcc/cp/mapper-client.h
index b32723ce296..a3b0b8adc51 100644
--- a/gcc/cp/mapper-client.h
+++ b/gcc/cp/mapper-client.h
@@ -55,6 +55,7 @@ public:
public:
static module_client *open_module_client (location_t loc, const char *option,
+ class mkdeps *,
void (*set_repo) (const char *),
char const *);
static void close_module_client (location_t loc, module_client *);
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index dbd1b721616..37066bf072b 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3969,12 +3969,12 @@ static GTY(()) vec
*partial_specializations;
/* Our module mapper (created lazily). */
module_client *mapper;
-static module_client *make_mapper (location_t loc);
-inline module_client *get_mapper (location_t loc)
+static module_client *make_mapper (location_t loc, class mkdeps *deps);
+inline module_client *get_mapper (location_t loc, class mkdeps *deps)
{
auto *res = mapper;
if (!res)
-res = make_mapper (loc);
+res = make_mapper (loc, deps);
return res;
}
@@ -14031,7 +14031,7 @@ get_module (const char *ptr)
/* Create a new mapper connecting to OPTION. */
module_client *
-make_mapper (location_t loc)
+make_mapper (location_t loc, class mkdeps *deps)
{
timevar_start (TV_MODULE_MAPPER);
const char *option = module_mapper_name;
@@ -14039,7 +14039,7 @@ make_mapper (location_t loc)
option = getenv ("CXX_MODULE_MAPPER");
mapper = module_client::open_module_client
-(loc, option, &set_cmi_repo,
+(loc, option, deps, &set_cmi_repo,
(save_decoded_options[0].opt_index == OPT_SPECIAL_program_name)
&& save_decoded_options[0].arg != progname
? save_decoded_options[0].arg : nullptr);
@@ -19503,7 +19503,7 @@ maybe_translate_include (cpp_reader *reader, line_maps
*lmaps, location_t loc,
dump.push (NULL);
dump () && dump ("Checking include translation '%s'", path);
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
size_t len = strlen (path);
path = canonicalize_header_name (NULL, loc, true, path, len);
@@ -19619,7 +19619,7 @@ module_begin_main_file (cpp_reader *reader, line_maps
*lmaps,
static void
name_pending_imports (cpp_reader *reader)
{
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
if (!vec_safe_length (pending_imports))
/* Not doing anything. */
@@ -20089,7 +20089,7 @@ init_modules (cpp_reader *reader)
if (!flag_module_lazy)
/* Get the mapper now, if we're not being lazy. */
-get_mapper (cpp_main_loc (reader));
+get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
if (!flag_preprocess_only)
{
@@ -20299,7 +20299,7 @@ late_finish_module (cpp_reader *reader,
module_processing_cookie *cookie,
if (!errorcount)
{
- auto *mapper = get_mapper (cpp_main_loc (reader));
+ auto *mapper = get_mapper (cpp_main_loc (reader), cpp_get_deps (reader));
mapper->ModuleCompiled (state->get_flatname ());
}
else if (cookie->cmi_name)
--
2.39.0
[PATCH] Fortran: fix ICE in check_host_association [PR108544]
Dear all,
the attached patch fixes two issues: first it addresses a NULL pointer
dereference on invalid input, triggered by the provided testcase.
Second, while analyzing the context of the affected code, I looked into
the testcase for PR96102, and by varying it slightly, i.e. replacing
functions by subroutines I found that we accept invalid code that is
rejected by several other brands tested. To fix this, I removed one
line of a condition that did not seem to make sense to me.
Regtested on x86_64-pc-linux-gnu. OK for mainline?
Thanks,
Harald
From 59034b3b938a2f5e3391208fca56fcf54d5b6d18 Mon Sep 17 00:00:00 2001
From: Harald Anlauf
Date: Wed, 25 Jan 2023 22:47:26 +0100
Subject: [PATCH] Fortran: fix ICE in check_host_association [PR108544]
gcc/fortran/ChangeLog:
PR fortran/108544
* resolve.cc (check_host_association): Extend host association check
so that it is not restricted to functions. Also prevent NULL pointer
dereference.
gcc/testsuite/ChangeLog:
PR fortran/108544
* gfortran.dg/pr108544.f90: New test.
* gfortran.dg/pr96102b.f90: New test.
---
gcc/fortran/resolve.cc | 4 +++-
gcc/testsuite/gfortran.dg/pr108544.f90 | 11 +++
gcc/testsuite/gfortran.dg/pr96102b.f90 | 24
3 files changed, 38 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gfortran.dg/pr108544.f90
create mode 100644 gcc/testsuite/gfortran.dg/pr96102b.f90
diff --git a/gcc/fortran/resolve.cc b/gcc/fortran/resolve.cc
index 94213cd3cd4..9e2edf7be71 100644
--- a/gcc/fortran/resolve.cc
+++ b/gcc/fortran/resolve.cc
@@ -6087,7 +6087,6 @@ check_host_association (gfc_expr *e)
gfc_find_symbol (e->symtree->name, gfc_current_ns, 1, &sym);
if (sym && old_sym != sym
- && sym->ts.type == old_sym->ts.type
&& sym->attr.flavor == FL_PROCEDURE
&& sym->attr.contained)
{
@@ -6132,6 +6131,9 @@ check_host_association (gfc_expr *e)
return false;
}
+ if (ref == NULL)
+ return false;
+
gcc_assert (ref->type == REF_ARRAY);
/* Grab the start expressions from the array ref and
diff --git a/gcc/testsuite/gfortran.dg/pr108544.f90 b/gcc/testsuite/gfortran.dg/pr108544.f90
new file mode 100644
index 000..783cb7aaf7b
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr108544.f90
@@ -0,0 +1,11 @@
+! { dg-do compile }
+! PR fortran/108544 - ICE in check_host_association
+! Contributed by G.Steinmetz
+
+module m
+contains
+ subroutine s
+select type (s => 1) ! { dg-error "Selector shall be polymorphic" }
+end select
+ end
+end
diff --git a/gcc/testsuite/gfortran.dg/pr96102b.f90 b/gcc/testsuite/gfortran.dg/pr96102b.f90
new file mode 100644
index 000..82147da3893
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr96102b.f90
@@ -0,0 +1,24 @@
+! { dg-do compile }
+!
+! PR fortran/108544 - host association
+! Variation of testcase pr96102.f90 using subroutines instead of functions
+
+module m
+ type mytype
+integer :: i
+ end type
+ type(mytype) :: d = mytype (42) ! { dg-error "is host associated" }
+ integer :: n = 2 ! { dg-error "is host associated" }
+contains
+ subroutine s
+if ( n /= 0 ) stop 1 ! { dg-error "internal procedure of the same name" }
+if ( d%i /= 0 ) stop 2 ! { dg-error "internal procedure of the same name" }
+ contains
+subroutine n()
+end
+subroutine d()
+end
+ end
+end
+
+! { dg-prune-output "Operands of comparison operator" }
--
2.35.3
Re: [PATCH] Fortran: fix ICE in check_host_association [PR108544]
On Wed, Jan 25, 2023 at 10:59:22PM +0100, Harald Anlauf via Fortran wrote: > Dear all, > > the attached patch fixes two issues: first it addresses a NULL pointer > dereference on invalid input, triggered by the provided testcase. > > Second, while analyzing the context of the affected code, I looked into > the testcase for PR96102, and by varying it slightly, i.e. replacing > functions by subroutines I found that we accept invalid code that is > rejected by several other brands tested. To fix this, I removed one > line of a condition that did not seem to make sense to me. > > Regtested on x86_64-pc-linux-gnu. OK for mainline? > Yes. I briefly looked at this by simply commenting out the assert, which gives too many odd error messages. Returning 'false' seems to be best. -- Steve
