[v6,0/4] P1689R5 support

Message ID 20230606205025.3164738-1-ben.boeckel@kitware.com
Headers
Series P1689R5 support |

Message

Ben Boeckel June 6, 2023, 8:50 p.m. UTC
  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
<some_header>;` 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

---
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):
  libcpp: reject codepoints above 0x10FFFF
  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                              |  24 +-
 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                             |   7 +
 libcpp/include/cpplib.h                       |  12 +-
 libcpp/include/mkdeps.h                       |  17 +-
 libcpp/init.cc                                |  13 +-
 libcpp/mkdeps.cc                              | 149 +++++++++++-
 39 files changed, 769 insertions(+), 28 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 mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o-MD.C
 create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
 create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.json
 create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
 create mode 100644 gcc/testsuite/lib/modules.exp


base-commit: 585c660f041c57694a61b3e65085af5a54431be4
  

Comments

Maxim Kuvyrkov June 8, 2023, 5:59 p.m. UTC | #1
> On Jun 7, 2023, at 00:50, Ben Boeckel via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:
> 
> 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.

Hi Ben,

This patch series causes ICEs on arm-linux-gnueabihf.  Would you please investigate?  Please let me know if you need any in reproducing these.

=== g++ tests ===

Running g++:g++.dg/modules/modules.exp ...
FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/ben-1_a.C module-cmi =partitions/module-import.mod (partitions/module-import.mod)
FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/ben-1_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/ben-1_b.C module-cmi =module.mod (module.mod)
FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/gc-2_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/gc-2_a.C module-cmi =map-1_a.nms (map-1_a.nms)
UNRESOLVED: g++.dg/modules/map-1 -std=c++17 execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++17 link
UNRESOLVED: g++.dg/modules/map-1 -std=c++2a execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++2a link
UNRESOLVED: g++.dg/modules/map-1 -std=c++2b execute
UNRESOLVED: g++.dg/modules/map-1 -std=c++2b link
FAIL: g++.dg/modules/map-1_a.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_a.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/map-1_a.C module-cmi =map-1_a.nms (map-1_a.nms)
FAIL: g++.dg/modules/map-1_b.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-1_b.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-1_b.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-1_b.C -std=c++2b (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++17 at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++17 at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++17 (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++17 (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++2a at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++2a at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++2a (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++2a (test for excess errors)
FAIL: g++.dg/modules/map-2.C -std=c++2b at line 8 (test for errors, line 7)
FAIL: g++.dg/modules/map-2.C -std=c++2b at line 9 (test for errors, line )
FAIL: g++.dg/modules/map-2.C -std=c++2b (internal compiler error: Segmentation fault)
FAIL: g++.dg/modules/map-2.C -std=c++2b (test for excess errors)
===

Thanks,

--
Maxim Kuvyrkov
https://www.linaro.org





> 
> [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
> <some_header>;` 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
> 
> ---
> 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):
>  libcpp: reject codepoints above 0x10FFFF
>  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                              |  24 +-
> 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                             |   7 +
> libcpp/include/cpplib.h                       |  12 +-
> libcpp/include/mkdeps.h                       |  17 +-
> libcpp/init.cc                                |  13 +-
> libcpp/mkdeps.cc                              | 149 +++++++++++-
> 39 files changed, 769 insertions(+), 28 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 mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fjo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-fo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-j.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-jo.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o-MD.C
> create mode 100644 gcc/testsuite/g++.dg/modules/depflags-o.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-1.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-2.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-3.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-4.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.C
> create mode 100644 gcc/testsuite/g++.dg/modules/p1689-5.exp.json
> create mode 100644 gcc/testsuite/g++.dg/modules/test-p1689.py
> create mode 100644 gcc/testsuite/lib/modules.exp
> 
> 
> base-commit: 585c660f041c57694a61b3e65085af5a54431be4
> -- 
> 2.40.1
>
  
Ben Boeckel June 16, 2023, 11:15 p.m. UTC | #2
On Fri, Jun 16, 2023 at 15:48:59 -0400, Ben Boeckel wrote:
> On Thu, Jun 08, 2023 at 21:59:13 +0400, Maxim Kuvyrkov wrote:
> > This patch series causes ICEs on arm-linux-gnueabihf.  Would you
> > please investigate?  Please let me know if you need any in reproducing
> > these.
> 
> Finally back at it. I tried on aarch64, but wasn't able to reproduce the
> errors (alas, it is probably a 32bit thing…let me try with `-m32`). Is
> there hardware I can access to try this out on the same target triple?

Trying inside of an i386 container also came up with nothing…I'll try
qemu.

--Ben
  
Jason Merrill June 17, 2023, 3:55 a.m. UTC | #3
On Fri, Jun 16, 2023 at 3:49 PM Ben Boeckel <ben.boeckel@kitware.com> wrote:
>
> On Thu, Jun 08, 2023 at 21:59:13 +0400, Maxim Kuvyrkov wrote:
> > This patch series causes ICEs on arm-linux-gnueabihf.  Would you
> > please investigate?  Please let me know if you need any in reproducing
> > these.
>
> Finally back at it. I tried on aarch64, but wasn't able to reproduce the
> errors (alas, it is probably a 32bit thing…let me try with `-m32`). Is
> there hardware I can access to try this out on the same target triple?
>
> Alternatively, a backtrace may be able to help pinpoint it enough if you
> have the cycles.

I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.

FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
Excess errors:
/home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
compiler error: Segmentation fault
0x19e2f3c crash_signal
        /home/jason/gt/gcc/toplev.cc:314
0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
        /home/jason/gt/libcpp/mkdeps.cc:57
0x340dc1f apply_vpath
        /home/jason/gt/libcpp/mkdeps.cc:194
0x340e08e deps_add_dep(mkdeps*, char const*)
        /home/jason/gt/libcpp/mkdeps.cc:318
0xea7b51 module_client::open_module_client(unsigned int, char const*,
mkdeps*, void (*)(char const*), char const*)
        /home/jason/gt/gcc/cp/mapper-client.cc:291
0xef2ba8 make_mapper
        /home/jason/gt/gcc/cp/module.cc:14042
0xf0896c get_mapper(unsigned int, mkdeps*)
        /home/jason/gt/gcc/cp/module.cc:3977
0xf032ac name_pending_imports
        /home/jason/gt/gcc/cp/module.cc:19623
0xf03a7d preprocessed_module(cpp_reader*)
        /home/jason/gt/gcc/cp/module.cc:19817
0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
        /home/jason/gt/gcc/cp/lex.cc:548
0xf467b2 cp_lexer_new_main
        /home/jason/gt/gcc/cp/parser.cc:756
0xfc1e3a c_parse_file()
        /home/jason/gt/gcc/cp/parser.cc:49725
0x11c5bf5 c_common_parse_file()
        /home/jason/gt/gcc/c-family/c-opts.cc:1268
  
Ben Boeckel June 17, 2023, 2:43 p.m. UTC | #4
On Fri, Jun 16, 2023 at 23:55:53 -0400, Jason Merrill wrote:
> I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.
> 
> FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
> Excess errors:
> /home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
> compiler error: Segmentation fault
> 0x19e2f3c crash_signal
>         /home/jason/gt/gcc/toplev.cc:314
> 0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
>         /home/jason/gt/libcpp/mkdeps.cc:57
> 0x340dc1f apply_vpath
>         /home/jason/gt/libcpp/mkdeps.cc:194
> 0x340e08e deps_add_dep(mkdeps*, char const*)
>         /home/jason/gt/libcpp/mkdeps.cc:318
> 0xea7b51 module_client::open_module_client(unsigned int, char const*,
> mkdeps*, void (*)(char const*), char const*)
>         /home/jason/gt/gcc/cp/mapper-client.cc:291
> 0xef2ba8 make_mapper
>         /home/jason/gt/gcc/cp/module.cc:14042
> 0xf0896c get_mapper(unsigned int, mkdeps*)
>         /home/jason/gt/gcc/cp/module.cc:3977
> 0xf032ac name_pending_imports
>         /home/jason/gt/gcc/cp/module.cc:19623
> 0xf03a7d preprocessed_module(cpp_reader*)
>         /home/jason/gt/gcc/cp/module.cc:19817
> 0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
>         /home/jason/gt/gcc/cp/lex.cc:548
> 0xf467b2 cp_lexer_new_main
>         /home/jason/gt/gcc/cp/parser.cc:756
> 0xfc1e3a c_parse_file()
>         /home/jason/gt/gcc/cp/parser.cc:49725
> 0x11c5bf5 c_common_parse_file()
>         /home/jason/gt/gcc/c-family/c-opts.cc:1268

Thanks. I missed a `nullptr` check before calling `deps_add_dep`. I
think I got misled by `make check` returning a zero exit code even if
there are failures.

Thanks,

--Ben
  
Jason Merrill June 20, 2023, 12:54 a.m. UTC | #5
On 6/17/23 10:43, Ben Boeckel wrote:
> On Fri, Jun 16, 2023 at 23:55:53 -0400, Jason Merrill wrote:
>> I see the same thing with patch 4 on x86_64-pc-linux-gnu, e.g.
>>
>> FAIL: g++.dg/modules/ben-1_a.C -std=c++17 (test for excess errors)
>> Excess errors:
>> /home/jason/gt/gcc/testsuite/g++.dg/modules/ben-1_a.C:9:1: internal
>> compiler error: Segmentation fault
>> 0x19e2f3c crash_signal
>>          /home/jason/gt/gcc/toplev.cc:314
>> 0x340f3f8 mkdeps::vec<mkdeps::velt>::size() const
>>          /home/jason/gt/libcpp/mkdeps.cc:57
>> 0x340dc1f apply_vpath
>>          /home/jason/gt/libcpp/mkdeps.cc:194
>> 0x340e08e deps_add_dep(mkdeps*, char const*)
>>          /home/jason/gt/libcpp/mkdeps.cc:318
>> 0xea7b51 module_client::open_module_client(unsigned int, char const*,
>> mkdeps*, void (*)(char const*), char const*)
>>          /home/jason/gt/gcc/cp/mapper-client.cc:291
>> 0xef2ba8 make_mapper
>>          /home/jason/gt/gcc/cp/module.cc:14042
>> 0xf0896c get_mapper(unsigned int, mkdeps*)
>>          /home/jason/gt/gcc/cp/module.cc:3977
>> 0xf032ac name_pending_imports
>>          /home/jason/gt/gcc/cp/module.cc:19623
>> 0xf03a7d preprocessed_module(cpp_reader*)
>>          /home/jason/gt/gcc/cp/module.cc:19817
>> 0xe85104 module_token_cdtor(cpp_reader*, unsigned long)
>>          /home/jason/gt/gcc/cp/lex.cc:548
>> 0xf467b2 cp_lexer_new_main
>>          /home/jason/gt/gcc/cp/parser.cc:756
>> 0xfc1e3a c_parse_file()
>>          /home/jason/gt/gcc/cp/parser.cc:49725
>> 0x11c5bf5 c_common_parse_file()
>>          /home/jason/gt/gcc/c-family/c-opts.cc:1268
> 
> Thanks. I missed a `nullptr` check before calling `deps_add_dep`. I
> think I got misled by `make check` returning a zero exit code even if
> there are failures.

Aha!

Patches 3 and 4 could also use testcases.

Jason