[Bug d/97644] New: FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[Bug d/97644] New: FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[msebor at gcc dot gnu.org]

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

            Bug ID: 97644
           Summary: FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk
           Product: gcc
           Version: unknown
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: d
          Assignee: ibuclaw at gdcproject dot org
          Reporter: msebor at gcc dot gnu.org
  Target Milestone: ---

While testing my patch for an unrelated bug I see the gdc.dg/gdc204.d test fail
with the following ICE:

Executing on host: /ssd/test/build/gcc-97556/gcc/testsuite/gdc/../../gdc
-B/ssd/test/build/gcc-97556/gcc/testsuite/gdc/../../
/ssd/test/src/gcc/97556/gcc/testsuite/gdc.dg/gdc204.d  
-fdiagnostics-plain-output 
-I/ssd/test/build/gcc-97556/x86_64-pc-linux-gnu/./libphobos/libdruntime
-I/ssd/test/src/gcc/97556/gcc/testsuite/../../libphobos/libdruntime
-I/ssd/test/src/gcc/97556/gcc/testsuite/../../libphobos/src     -S -o gdc204.s 
  (timeout = 300)
spawn -ignore SIGHUP /ssd/test/build/gcc-97556/gcc/testsuite/gdc/../../gdc
-B/ssd/test/build/gcc-97556/gcc/testsuite/gdc/../../
/ssd/test/src/gcc/97556/gcc/testsuite/gdc.dg/gdc204.d
-fdiagnostics-plain-output
-I/ssd/test/build/gcc-97556/x86_64-pc-linux-gnu/./libphobos/libdruntime
-I/ssd/test/src/gcc/97556/gcc/testsuite/../../libphobos/libdruntime
-I/ssd/test/src/gcc/97556/gcc/testsuite/../../libphobos/src -S -o gdc204.s
/ssd/test/src/gcc/97556/gcc/testsuite/gdc.dg/gdc204.d:9:1: internal compiler
error: Segmentation fault
0xe979da crash_signal
        /ssd/test/src/gcc/97556/gcc/toplev.c:330
0x66e57a expand_thunk(cgraph_node*, bool, bool)
        /ssd/test/src/gcc/97556/gcc/symtab-thunks.cc:334
0x9965c8 finish_thunk
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1708
0x9965c8 make_thunk(FuncDeclaration*, int)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1820
0x9b5026 TypeInfoVisitor::layout_base_vtable(ClassDeclaration*,
ClassDeclaration*, unsigned long)
        /ssd/test/src/gcc/97556/gcc/d/typeinfo.cc:509
0x9b5690 TypeInfoVisitor::visit(TypeInfoClassDeclaration*)
        /ssd/test/src/gcc/97556/gcc/d/typeinfo.cc:972
0x9ae5ef TypeInfoClassDeclaration::accept(Visitor*)
        /ssd/test/src/gcc/97556/gcc/d/dmd/declaration.h:343
0x9ae5ef layout_classinfo(ClassDeclaration*)
        /ssd/test/src/gcc/97556/gcc/d/typeinfo.cc:1172
0x998a1b DeclVisitor::visit(ClassDeclaration*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:513
0x99397f DeclVisitor::build_dsymbol(Dsymbol*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:145
0x99397f build_decl_tree(Dsymbol*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1015
0x9a5f50 build_module_tree(Module*)
        /ssd/test/src/gcc/97556/gcc/d/modules.cc:729
0x996ecb DeclVisitor::visit(Module*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:162
0x99397f DeclVisitor::build_dsymbol(Dsymbol*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:145
0x99397f build_decl_tree(Dsymbol*)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1015
0x990952 d_parse_file
        /ssd/test/src/gcc/97556/gcc/d/d-lang.cc:1236
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <https://gcc.gnu.org/bugs/> for instructions.
compiler exited with status 1
FAIL: gdc.dg/gdc204.d   (internal compiler error)
FAIL: gdc.dg/gdc204.d   (test for excess errors)

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[msebor at gcc dot gnu.org]

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

--- Comment #1 from Martin Sebor <msebor at gcc dot gnu.org> ---
Ditto in gdc.dg/pr92216.d:

/ssd/test/src/gcc/97556/gcc/testsuite/gdc.dg/pr92216.d:10:1: internal compiler
error: Segmentation fault
0xe979da crash_signal
        /ssd/test/src/gcc/97556/gcc/toplev.c:330
0x66e57a expand_thunk(cgraph_node*, bool, bool)
        /ssd/test/src/gcc/97556/gcc/symtab-thunks.cc:334
0x9965c8 finish_thunk
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1708
0x9965c8 make_thunk(FuncDeclaration*, int)
        /ssd/test/src/gcc/97556/gcc/d/decl.cc:1820
0x9b5026 TypeInfoVisitor::layout_base_vtable(ClassDeclaration*,
ClassDeclaration*, unsigned long)

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[msebor at gcc dot gnu.org]

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

--- Comment #2 from Martin Sebor <msebor at gcc dot gnu.org> ---
Also same ICE in:
FAIL: gdc.test/runnable_cxx/cppa.d   (internal compiler error)

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #3 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
I haven't seen any failures as of r11-4466.  So a regression cropped up over
the last couple days maybe?

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #4 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Iain Buclaw from comment #3)
> I haven't seen any failures as of r11-4466.  So a regression cropped up over
> the last couple days maybe?

Actually, make that r11-4555 being the last commit tested, and I saw no
failures.  That narrows it down to today.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #5 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Without doing any bisecting, r11-4572 looks very suspect for the cause of the
segmentation fault.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #6 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Iain Buclaw from comment #5)
> Without doing any bisecting, r11-4572 looks very suspect for the cause of
> the segmentation fault.

Confirmed, that is the commit that caused the regression.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #7 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Created attachment 49485
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49485&action=edit
workaround pr97644

Current workaround I'm using locally for the time being is to call
thunk_info::process_early_thunks if the particular branch where this ICE
happens is hit.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[hubicka at ucw dot cz]

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

--- Comment #8 from Jan Hubicka <hubicka at ucw dot cz> ---
> Current workaround I'm using locally for the time being is to call
> thunk_info::process_early_thunks if the particular branch where this ICE
> happens is hit.

Why D frontend needs to expand the thunk early and does not rely on
backend to handle it same way as C++ does?  It would be most practical
if both was finalizing same way.

If there is good reason for not doing so and there is no PCH in D
frontend we could just add argument to expand_thunk to avoid the game o
putting things into a vector.

Honza

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #9 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Jan Hubicka from comment #8)
> > Current workaround I'm using locally for the time being is to call
> > thunk_info::process_early_thunks if the particular branch where this ICE
> > happens is hit.
> 
> Why D frontend needs to expand the thunk early and does not rely on
> backend to handle it same way as C++ does?  It would be most practical
> if both was finalizing same way.
> 
> If there is good reason for not doing so and there is no PCH in D
> frontend we could just add argument to expand_thunk to avoid the game o
> putting things into a vector.
> 

Not discounting that I may be doing it wrong.  The call to expand_thunk is
there so that thunks for externally defined methods are generated.

As the backend usually doesn't emit such thunks, a gimple generated thunk is
forced by the frontend.

I have seen linker errors in the past when this was not done, however there
could be some alignment with what C++ does to handle this though.

A simplistic example would be:

    interface I { void foo(); }
    class C : I { void foo(); }

Where a TU local thunk is generated for C.foo.

A more complex example would be when interfacing with C++, where different
methods are implemented half in C++, half in D.

    // C++
    class Base
    {
    public:
        virtual void base() { };
    };

    class Interface
    {
    public:
        virtual int MethodCPP() = 0;
        virtual int MethodD() = 0;
    };

    class Derived : public Base, public Interface
    {
    public:
        int MethodCPP();  // thunk _ZThn8_N7Derived9MethodCPPEv
        int MethodD();    // thunk _ZThn8_N7Derived7MethodDEv
    };

    int Derived::MethodCPP() { return 30; }

    // D
    extern (C++)
    {
        class Base
        {
            void based();
        }

        interface Interface
        {
            int MethodCPP();
            int MethodD();
        }

        class Derived : Base, Interface
        {
            int MethodCPP();  // thunk _DT8_ZN7Derived9MethodCPPEv
            int MethodD()     // thunk _DT8_ZN7Derived7MethodDEv
            {
               return 3;
            }
        }
    }

In the above, g++ and gdc give different global symbol names for the thunks, so
linker errors would occur if they are not emitted.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[hubicka at ucw dot cz]

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

--- Comment #10 from Jan Hubicka <hubicka at ucw dot cz> ---
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97644
> 
> --- Comment #9 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
> (In reply to Jan Hubicka from comment #8)
> > > Current workaround I'm using locally for the time being is to call
> > > thunk_info::process_early_thunks if the particular branch where this ICE
> > > happens is hit.
> > 
> > Why D frontend needs to expand the thunk early and does not rely on
> > backend to handle it same way as C++ does?  It would be most practical
> > if both was finalizing same way.
> > 
> > If there is good reason for not doing so and there is no PCH in D
> > frontend we could just add argument to expand_thunk to avoid the game o
> > putting things into a vector.
> > 
> 
> Not discounting that I may be doing it wrong.  The call to expand_thunk is
> there so that thunks for externally defined methods are generated.
> 
> As the backend usually doesn't emit such thunks, a gimple generated thunk is
> forced by the frontend.
> 
> I have seen linker errors in the past when this was not done, however there
> could be some alignment with what C++ does to handle this though.

Aha, thanks for explanation.
In C++ thunks was always connected to the actual function so for
external symbols we indeed do not produce the thunk.

How does the symbols look like? So you have external symbol and a thunk
associated to it with different visibility? What it is?
I see that if you expand it to gimple you turn it to normal function and
it will be output then. We could do that at the finalization time
avoiding the need for additional hacks in the frontend/middleend
interface.

Honza

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

--- Comment #11 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Jan Hubicka from comment #10)
> > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97644
> > 
> > --- Comment #9 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
> > (In reply to Jan Hubicka from comment #8)
> > > > Current workaround I'm using locally for the time being is to call
> > > > thunk_info::process_early_thunks if the particular branch where this ICE
> > > > happens is hit.
> > > 
> > > Why D frontend needs to expand the thunk early and does not rely on
> > > backend to handle it same way as C++ does?  It would be most practical
> > > if both was finalizing same way.
> > > 
> > > If there is good reason for not doing so and there is no PCH in D
> > > frontend we could just add argument to expand_thunk to avoid the game o
> > > putting things into a vector.
> > > 
> > 
> > Not discounting that I may be doing it wrong.  The call to expand_thunk is
> > there so that thunks for externally defined methods are generated.
> > 
> > As the backend usually doesn't emit such thunks, a gimple generated thunk is
> > forced by the frontend.
> > 
> > I have seen linker errors in the past when this was not done, however there
> > could be some alignment with what C++ does to handle this though.
> 
> Aha, thanks for explanation.
> In C++ thunks was always connected to the actual function so for
> external symbols we indeed do not produce the thunk.
> 
> How does the symbols look like? So you have external symbol and a thunk
> associated to it with different visibility? What it is?

Because this is what the reference D compiler does, thunks are static (in the C
sense) and connected with the class definition, where it is referenced by the
class vtable symbol.

However, I have for the past week been trialling out thunks that are global and
emitted only if the function has a definition as well as per what you described
about C++ thunks.

The only drawback I've found so far is that thunks for extern(C++) classes are
left undefined, however I can resolve this by implementing mangle support in
the D front-end so that the C++ generated thunks can be referenced directly.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[cvs-commit at gcc dot gnu.org]

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

--- Comment #12 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:5d4b824faf1e5846ec684a74f93912cf347928df

commit r11-4979-g5d4b824faf1e5846ec684a74f93912cf347928df
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Thu Nov 12 15:37:46 2020 +0100

    d: Fix ICE in finish_thunk (PR97644)

    Because this what the upstream reference compiler did, thunks for the D
    front-end were associated with the class definition, so were forced
    code-gen even if the target function was extern.  This has now been
    changed so there are now only generated if there is a function
    definition, fixing the ICE that occurred in PR 97644, which was caused
    by calling expand_thunk() early.

    gcc/d/ChangeLog:

            PR d/97644
            * dmd/MERGE: Merge upstream dmd 95044d8e4.
            * d-target.cc (TargetCPP::thunkMangle): New function.
            * decl.cc (finish_thunk): Don't force expand thunks for external
            functions.
            (make_thunk): Emit thunks only if the function has a definition.
            Generate correct mangling for thunks to C++ classes.

    gcc/testsuite/ChangeLog:

            * gdc.dg/pr92216.d: Update scan-assember.

[Bug d/97644] FAIL: gdc.dg/gdc204.d due to ICE in finish_thunk

[ibuclaw at gdcproject dot org]

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

Iain Buclaw <ibuclaw at gdcproject dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |FIXED

--- Comment #13 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Marking as fixed.