Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Le 31/08/2023 à 22:42, Harald Anlauf via Fortran a écrit : Dear all, gfortran's array bounds-checking code does a mostly reasonable job for array sections in expressions and assignments, but forgot the case that (rank-1) expressions can involve array constructors, which have a shape ;-) The attached patch walks over the loops generated by the scalarizer, checks for the presence of a constructor, and takes the first shape found as reference. (If several constructors are present, discrepancies in their shape seems to be already detected at compile time). For more details on what will be caught now see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is OK. May I suggest to handle functions the same way? Thanks. Thanks, Harald
[PATCH v8 1/4] spec: add a spec function to join arguments
When passing `-o` flags to other options, the typical `-o foo` spelling
leaves a leading whitespace when replacing elsewhere. This ends up
creating flags spelled as `-some-option-with-arg= foo.ext` which doesn't
parse properly. When attempting to make a spec function to just remove
the leading whitespace, the argument splitting ends up masking the
whitespace. However, the intended extension *also* ends up being its own
argument. To perform the desired behavior, the arguments need to be
concatenated together.
gcc/:
* gcc.cc (join_spec_func): Add a spec function to join all
arguments.
Signed-off-by: Ben Boeckel
---
gcc/gcc.cc | 23 +++
1 file changed, 23 insertions(+)
diff --git a/gcc/gcc.cc b/gcc/gcc.cc
index fdfac0b4fe4..4c4e81dee50 100644
--- a/gcc/gcc.cc
+++ b/gcc/gcc.cc
@@ -447,6 +447,7 @@ static const char *greater_than_spec_func (int, const char
**);
static const char *debug_level_greater_than_spec_func (int, const char **);
static const char *dwarf_version_greater_than_spec_func (int, const char **);
static const char *find_fortran_preinclude_file (int, const char **);
+static const char *join_spec_func (int, const char **);
static char *convert_white_space (char *);
static char *quote_spec (char *);
static char *quote_spec_arg (char *);
@@ -1772,6 +1773,7 @@ static const struct spec_function static_spec_functions[]
=
{ "debug-level-gt", debug_level_greater_than_spec_func },
{ "dwarf-version-gt",dwarf_version_greater_than_spec_func },
{ "fortran-preinclude-file", find_fortran_preinclude_file},
+ { "join",join_spec_func},
#ifdef EXTRA_SPEC_FUNCTIONS
EXTRA_SPEC_FUNCTIONS
#endif
@@ -10975,6 +10977,27 @@ find_fortran_preinclude_file (int argc, const char
**argv)
return result;
}
+/* The function takes any number of arguments and joins them together.
+
+ This seems to be necessary to build "-fjoined=foo.b" from "-fseparate foo.a"
+ with a %{fseparate*:-fjoined=%.b$*} rule without adding undesired spaces:
+ when doing $* replacement we first replace $* with the rest of the switch
+ (in this case ""), and then add any arguments as arguments after the result,
+ resulting in "-fjoined= foo.b". Using this function with e.g.
+ %{fseparate*:-fjoined=%:join(%.b$*)} gets multiple words as separate argv
+ elements instead of separated by spaces, and we paste them together. */
+
+static const char *
+join_spec_func (int argc, const char **argv)
+{
+ if (argc == 1)
+return argv[0];
+ for (int i = 0; i < argc; ++i)
+obstack_grow (&obstack, argv[i], strlen (argv[i]));
+ obstack_1grow (&obstack, '\0');
+ return XOBFINISH (&obstack, const char *);
+}
+
/* If any character in ORIG fits QUOTE_P (_, P), reallocate the string
so as to precede every one of them with a backslash. Return the
original string or the reallocated one. */
--
2.41.0
[PATCH v8 0/4] 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 --- v7 -> v8: - rename `DEPS_FMT_` enum variants to `FDEPS_FMT_` to match the associated flag - memory leak fix in the `join` specfunc implementation (also better comments), both from Jason - formatting fix in `mkdeps.cc` for `write_make_modules_deps` assignment - comments on new functions for P1689R5 implementation v6 -> v7: - rebase onto `master` (80ae426a195 (d: Fix core.volatile.volatileLoad discarded if result is unused, 2023-07-02)) - add test cases for patches 3 and 4 (new dependency reporting in `-MF`) - add a Python script to test aspects of generated dependency files - a new `join` spec function to support `-fdeps-*` defaults based on the `-o` flag (needed to strip the leading space that appears otherwise) - note that JSON writing support should be factored out for use by `libcpp` and `gcc` (libiberty?) - use `.ddi` for the extension of `-fdeps-*` output files by default - support defaults for `-fdeps-file=` and `-fdeps-target=` when only `-fdeps-format=` is provided (with tests) - error if `-MF` and `-fdeps-file=` are both the same (non-`stdout`) file as their formats are incompatible - expand the documentation on how the `-fdeps-*` flags should be used v5 -> v6: - rebase onto `master` (585c660f041 (reload1: Change return type of predicate function from int to bool, 2023-06-06)) - fix crash related to reporting imported CMI files as dependencies - rework utf-8 validity to patch the new `cpp_valid_utf8_p` function instead of the core utf-8 decoding routine to reject invalid codepoints (preserves higher-level error detection of invalid utf-8) - harmonize of `fdeps` spelling in flags, variables, comments, etc. - rename `-fdeps-output=` to `-fdeps-target=` 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 (4): spec: add a spec function to join arguments 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| 44 +++- gcc/c-family/c.opt| 12 + gcc/cp/mapper-client.cc | 5 + gcc/cp/mapper-client.h| 1 + gcc/cp/module.cc | 24 +- gcc/doc/invoke.texi | 27 +++ gcc/gcc.cc| 27 ++- gcc/json.h| 3 + gcc/testsuite/g++.dg/modules/depflags-f-MD.C | 2 + gcc/testsuite/g++.dg/modules/depflags-f.C | 3 + gcc/testsuite/g++.dg/modules/depflags-fi.C| 4 + gcc/testsuite/g++.dg/modules/depflags-fj-MD.C | 3 + .../g++.dg/modules/depflags-fj-MF-share.C | 6 + 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
[PATCH v8 3/4] 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.
gcc/testsuite/
* g++.dg/modules/depreport-1_a.C: New test.
* g++.dg/modules/depreport-1_b.C: New test.
* g++.dg/modules/test-depfile.py: New tool for validating depfile
information.
* lib/modules.exp: Support for validating depfile contents.
Signed-off-by: Ben Boeckel
---
gcc/cp/module.cc | 3 +
gcc/testsuite/g++.dg/modules/depreport-1_a.C | 10 +
gcc/testsuite/g++.dg/modules/depreport-1_b.C | 12 ++
gcc/testsuite/g++.dg/modules/test-depfile.py | 187 +++
gcc/testsuite/lib/modules.exp| 29 +++
5 files changed, 241 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/depreport-1_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/depreport-1_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/test-depfile.py
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9df60d695b1..f3acc4e02fe 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18968,6 +18968,9 @@ 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. */
+ if (cpp_get_deps (reader))
+ deps_add_dep (cpp_get_deps (reader), file);
fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY);
e = errno;
}
diff --git a/gcc/testsuite/g++.dg/modules/depreport-1_a.C
b/gcc/testsuite/g++.dg/modules/depreport-1_a.C
new file mode 100644
index 000..241701728a2
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depreport-1_a.C
@@ -0,0 +1,10 @@
+// { dg-additional-options -fmodules-ts }
+
+export module Foo;
+// { dg-module-cmi Foo }
+
+export class Base
+{
+public:
+ int m;
+};
diff --git a/gcc/testsuite/g++.dg/modules/depreport-1_b.C
b/gcc/testsuite/g++.dg/modules/depreport-1_b.C
new file mode 100644
index 000..b6e317c6703
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/depreport-1_b.C
@@ -0,0 +1,12 @@
+// { dg-additional-options -fmodules-ts }
+// { dg-additional-options -MD }
+// { dg-additional-options "-MF depreport-1.d" }
+
+import Foo;
+
+void foo ()
+{
+ Base b;
+}
+
+// { dg-final { run-check-module-dep-expect-input "depreport-1.d"
"gcm.cache/Foo.gcm" } }
diff --git a/gcc/testsuite/g++.dg/modules/test-depfile.py
b/gcc/testsuite/g++.dg/modules/test-depfile.py
new file mode 100644
index 000..ea4edb61434
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/test-depfile.py
@@ -0,0 +1,187 @@
+import json
+
+
+# Parameters.
+ALL_ERRORS = False
+
+
+def _report_error(msg):
+'''Report an error.'''
+full_msg = 'ERROR: ' + msg
+if ALL_ERRORS:
+print(full_msg)
+else:
+raise RuntimeError(full_msg)
+
+
+class Token(object):
+pass
+
+
+class Output(Token):
+def __init__(self, path):
+self.path = path
+
+
+class Input(Token):
+def __init__(self, path):
+self.path = path
+
+
+class Colon(Token):
+pass
+
+
+class Append(Token):
+pass
+
+
+class Variable(Token):
+def __init__(self, name):
+self.name = name
+
+
+class Word(Token):
+def __init__(self, name):
+self.name = name
+
+
+def validate_depfile(depfile, expect_input=None):
+'''Validate a depfile contains some information
+
+Returns `False` if the information is not found.
+'''
+with open(depfile, 'r') as fin:
+depfile_content = fin.read()
+
+real_lines = []
+join_line = False
+for line in depfile_content.split('\n'):
+# Join the line if needed.
+if join_line:
+line = real_lines.pop() + line
+
+# Detect line continuations.
+join_line = line.endswith('\\')
+# Strip line continuation characters.
+if join_line:
+line = line[:-1]
+
+# Add to the real line set.
+real_lines.append(line)
+
+# Perform tokenization.
+tokenized_lines = []
+for line in real_lines:
+tokenized = []
+join_word = False
+for word in line.split(' '):
+if join_word:
+word = tokenized.pop() + ' ' + word
+
+# Detect word joins.
+join_word = word.endswith('\\')
+# Strip escape character.
+if join_word:
+word = word[:-1]
+
+# Detect `:` at the end of a word.
+if word.endswith(':'):
+tokenized.append(word[:-1])
+word = word[-1]
+
+# Add word to the tokenized set.
+tokenized.append(word)
+
+tokenized_lines.append(tokenized)
+
+# Parse.
+ast = []
+for line in tokenized_lines:
+kind = None
+for token in line:
+if token == ':':
+kind = 'dependency'
+el
[PATCH v8 4/4] 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.
gcc/testsuite/:
* g++.dg/modules/depreport-2.modmap: New test.
* g++.dg/modules/depreport-2_a.C: New test.
* g++.dg/modules/depreport-2_b.C: New test.
* g++.dg/modules/test-depfile.py: Support `:|` syntax output
when generating modules.
Signed-off-by: Ben Boeckel
---
gcc/cp/mapper-client.cc | 5 +
gcc/cp/mapper-client.h| 1 +
gcc/cp/module.cc | 18 -
.../g++.dg/modules/depreport-2.modmap | 2 ++
gcc/testsuite/g++.dg/modules/depreport-2_a.C | 15 ++
gcc/testsuite/g++.dg/modules/depreport-2_b.C | 14 +
gcc/testsuite/g++.dg/modules/test-depfile.py | 20 +++
7 files changed, 66 insertions(+), 9 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2.modmap
create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/depreport-2_b.C
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
index 39e80df2d25..92727195246 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,9 @@ module_client::open_module_client (location_t loc, const
char *o,
errmsg = "opening";
else
{
+ /* Add the mapper file to the dependency tracking. */
+ if (deps)
+ 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 f3acc4e02fe..77c9edcbc04 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;
}
@@ -14033,7 +14033,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;
@@ -14041,7 +14041,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);
@@ -19506,7 +19506,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);
@@ -19622,7 +19622,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
[PATCH v8 2/4] 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. - `-fdeps-target=` 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 representable 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_fdeps_format enum. (cpp_options): Add fdeps_format field (cpp_finish): Add structured dependency fdeps_stream parameter. * include/mkdeps.h (deps_add_module_target): Add flag for whether a module is exported or not. (fdeps_add_target): Add function. (deps_write_p1689r5): Add function. * 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=, -fdeps-file=, and -fdeps-target= flags. * gcc.cc: add defaults for -fdeps-target= and -fdeps-file= when only -fdeps-format= is specified. * json.h: Add a TODO item to refactor out to share with `libcpp/mkdeps.cc`. gcc/c-family/ * c-opts.cc (c_common_handle_option): Add fdeps_file variable and -fdeps-format=, -fdeps-file=, and -fdeps-target= parsing. * c.opt: Add -fdeps-format=, -fdeps-file=, and -fdeps-target= 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.ddi: New test expectation. * g++.dg/modules/p1689-2.C: New test. * g++.dg/modules/p1689-2.exp.ddi: New test expectation. * g++.dg/modules/p1689-3.C: New test. * g++.dg/modules/p1689-3.exp.ddi: New test expectation. * g++.dg/modules/p1689-4.C: New test. * g++.dg/modules/p1689-4.exp.ddi: New test expectation. * g++.dg/modules/p1689-5.C: New test. * g++.dg/modules/p1689-5.exp.ddi: 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 validati
Re: [PATCH] Fortran: runtime bounds-checking in presence of array constructors [PR31059]
Hi Mikael, On 9/1/23 10:41, Mikael Morin via Gcc-patches wrote: Le 31/08/2023 à 22:42, Harald Anlauf via Fortran a écrit : Dear all, gfortran's array bounds-checking code does a mostly reasonable job for array sections in expressions and assignments, but forgot the case that (rank-1) expressions can involve array constructors, which have a shape ;-) The attached patch walks over the loops generated by the scalarizer, checks for the presence of a constructor, and takes the first shape found as reference. (If several constructors are present, discrepancies in their shape seems to be already detected at compile time). For more details on what will be caught now see testcase. Regtested on x86_64-pc-linux-gnu. OK for mainline? This is OK. I've pushed this is the first step. May I suggest to handle functions the same way? I'll have a look at them, but will need to gather a few suitable testcases first. Thanks for the review! Harald Thanks. Thanks, Harald
