[Bug c++/111710] New: [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[Bug c++/111710] New: [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[nicolas.werner at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111710

            Bug ID: 111710
           Summary: [modules] ICE when compiling module where a lambda is
                    assigned to a field in an exported class
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c++
          Assignee: unassigned at gcc dot gnu.org
          Reporter: nicolas.werner at hotmail dot de
  Target Milestone: ---

Created attachment 56063
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=56063&action=edit
Patch which prevents the ICE when assigning a lambda to a field inside an
exported entity

Reduced minimal example:

export module argparse;

export {

  struct Argument {
    int (*i)(int) = 
      [](int value) { return value; };
  };

}


When compiling this example with "g++ -std=c++23  -fmodules-ts  -x c++ -o
argparse.ixx.o -c argparse.ixx" it produces the following crash:

0x5583c3e758bb crash_signal
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/toplev.cc:314
0x7f2a9658041f ???
       
/usr/src/debug/sys-libs/glibc-2.38-r5/glibc-2.38/signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0
0x5583c3627ea5 trees_out::key_mergeable(int, merge_kind, tree_node*,
tree_node*, tree_node*, depset*)
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/module.cc:10651
0x5583c36220e8 trees_out::decl_value(tree_node*, depset*)
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/module.cc:7786
0x5583c362abd2 depset::hash::find_dependencies(module_state*)
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/module.cc:13328
0x5583c362ba29 module_state::write_begin(elf_out*, cpp_reader*,
module_state_config&, unsigned int&)
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/module.cc:17895
0x5583c362d0b4 finish_module_processing(cpp_reader*)
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/module.cc:20241
0x5583c35af85d c_parse_final_cleanups()
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/cp/decl2.cc:5255
0x5583c381d2fd c_common_parse_file()
       
/usr/src/debug/sys-devel/gcc-14.0.0_pre20231001/gcc-14-20231001/gcc/c-family/c-opts.cc:1296

This is because the lambda is treated as a field by trees_out::get_merge_kind,
but the corresponding case in trees_out::key_mergeable can't find such a field
and then runs over the end of the linked list and dereferences a nullptr.

I am not sure, what the proper mergeable kind is for such a lambda. I tried
changing it to be MK_unique, which seems to fix the crash, but I don't know
what the consequences of that would be. I would assume MK_keyed to be the right
value, however I couldn't make that work. Alternatively possibly the
key_mergeable needs to be adapted to handle such fields properly, but since
this is my first time touching the gcc codebase, I find that part of the code
to be a bit hard to wrap my head around.

I have attached the patch, which changes the mergekind  to demonstrate the
problem area as well as included a test case in that patch. Maybe that can help
solving that issue properly.

I tested this with 13.2.1_p20230826 and 14.0.0_pre20231001.

[Bug c++/111710] [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111710

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Patches should be sent to gcc-patches@ after reading
https://gcc.gnu.org/contribute.html .

[Bug c++/111710] [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[nicolas.werner at hotmail dot de]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111710

--- Comment #2 from Nicolas Werner <nicolas.werner at hotmail dot de> ---
I don't really have sufficient knowledge to push this patch forward, since that
currently exceeds my skillset. As such I have no confidence this patch is
actually doing something beneficial, which is why I didn't submit it to the
mailing list, so that I could gather either more knowledge about that issue or
someone else might know how to fix it. Should I still submit it to the mailing
list in this case?

[Bug c++/111710] [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111710

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Nathaniel Shead <nshead@gcc.gnu.org>:

https://gcc.gnu.org/g:3685fae23bb00898749dfc155212c9c5cd3a0980

commit r14-9232-g3685fae23bb00898749dfc155212c9c5cd3a0980
Author: Nathaniel Shead <nathanieloshead@gmail.com>
Date:   Fri Feb 16 15:52:48 2024 +1100

    c++: Support lambdas attached to more places in modules [PR111710]

    The fix for PR107398 weakened the restrictions that lambdas must belong
    to namespace scope. However this was not sufficient: we also need to
    allow lambdas attached to FIELD_DECLs, PARM_DECLs, and TYPE_DECLs.

    For field decls we key the lambda to its class rather than the field
    itself. Otherwise we can run into issues when deduplicating the lambda's
    TYPE_DECL, because when loading its context we load the containing field
    before we've deduplicated the keyed lambda, causing mismatches; by
    keying to the class instead we defer checking keyed declarations until
    deduplication has completed.

    Additionally, by [basic.link] p15.2 a lambda defined anywhere in a
    class-specifier should not be TU-local, which includes base-class
    declarations, so ensure that lambdas declared there are keyed
    appropriately as well.

    Because this now requires 'DECL_MODULE_KEYED_DECLS_P' to be checked on a
    fairly large number of different kinds of DECLs, and that in general
    it's safe to just get 'false' as a result of a check on an unexpected
    DECL type, this patch also removes the tree checking from the accessor.

    Finally, to handle deduplicating templated lambda fields, we need to
    ensure that we can determine that two lambdas from different field decls
    match, so we ensure that we also deduplicate LAMBDA_EXPRs on stream in.

            PR c++/111710

    gcc/cp/ChangeLog:

            * cp-tree.h (DECL_MODULE_KEYED_DECLS_P): Remove tree checking.
            (struct lang_decl_base): Update comments and fix whitespace.
            * module.cc (trees_out::lang_decl_bools): Always write
            module_keyed_decls_p flag...
            (trees_in::lang_decl_bools): ...and always read it.
            (trees_out::decl_value): Handle all kinds of keyed decls.
            (trees_in::decl_value): Likewise.
            (trees_in::tree_value): Deduplicate LAMBDA_EXPRs.
            (maybe_key_decl): Also support lambdas attached to fields,
            parameters, and types. Key lambdas attached to fields to their
            class.
            (trees_out::get_merge_kind): Likewise.
            (trees_out::key_mergeable): Likewise.
            (trees_in::key_mergeable): Support keyed decls in a TYPE_DECL
            container.
            * parser.cc (cp_parser_class_head): Start a lambda scope when
            parsing base classes.

    gcc/testsuite/ChangeLog:

            * g++.dg/modules/lambda-7.h: New test.
            * g++.dg/modules/lambda-7_a.H: New test.
            * g++.dg/modules/lambda-7_b.C: New test.
            * g++.dg/modules/lambda-7_c.C: New test.

    Signed-off-by: Nathaniel Shead <nathanieloshead@gmail.com>
    Reviewed-by: Patrick Palka <ppalka@redhat.com>
    Reviewed-by: Jason Merrill <jason@redhat.com>

[Bug c++/111710] [modules] ICE when compiling module where a lambda is assigned to a field in an exported class

[nshead at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=111710

Nathaniel Shead <nshead at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
         Resolution|---                         |FIXED
                 CC|                            |nshead at gcc dot gnu.org
           Assignee|unassigned at gcc dot gnu.org      |nshead at gcc dot gnu.org
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #4 from Nathaniel Shead <nshead at gcc dot gnu.org> ---
Fixed.