[Bug ipa/98744] New: [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[Bug ipa/98744] New: [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[marxin at gcc dot gnu.org]

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

            Bug ID: 98744
           Summary: [11 Regression] ICE in gimple_call_arg, at
                    gimple.h:3246 since r11-6735-g424deca72b63e644
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: marxin at gcc dot gnu.org
                CC: jamborm at gcc dot gnu.org, jason at gcc dot gnu.org,
                    marxin at gcc dot gnu.org
  Target Milestone: ---

I see the following ICE for:

$ cat elide.C
struct A {};
struct B : virtual A {};
struct C : B {
  C() : B(B()) {}
};
int main() { C c; return 0; }

$ g++ elide.C -c -Wmaybe-uninitialized -fno-inline -O2 -c
during GIMPLE pass: uninit
elide.C: In constructor ‘C::C()’:
elide.C:4:3: internal compiler error: in gimple_call_arg, at gimple.h:3246
    4 |   C() : B(B()) {}
      |   ^
0x85ddd8 gimple_call_arg
        /home/marxin/Programming/gcc/gcc/gimple.h:3246
0x85ddd8 gimple_call_arg
        /home/marxin/Programming/gcc/gcc/gimple.h:3244
0x85ddd8 maybe_warn_pass_by_reference
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:515
0x85ddd8 warn_uninitialized_vars
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:648
0x1382507 execute
        /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:3019
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.

What happens is that:
#2  0x0000000001383cef in maybe_warn_pass_by_reference (stmt=0x7ffff7552630,
wlims=...) at /home/marxin/Programming/gcc/gcc/tree-ssa-uninit.c:531

(gdb) p debug_gimple_stmt(stmt)
B::__ct_base .isra (_1);

which is an ISRA clone, but the gimple_call_fntype function declaration has 2
arguments:

(gdb) pct(fntype)

void B::<T3e0> (struct B *, const void *)

@Martin: Is it expected or should be the decl adjusted after IPA SRA is done?

[Bug ipa/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[marxin at gcc dot gnu.org]

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

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
           Priority|P3                          |P1
      Known to work|                            |10.2.0
   Last reconfirmed|                            |2021-01-19
      Known to fail|                            |11.0
     Ever confirmed|0                           |1
   Target Milestone|---                         |11.0

[Bug ipa/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
You have to adjust gimple_call_fntype when you adjust the call signature.
Mind that gimple_call_fntype specifies the ABI so when you for example
change a 'double *' to a 'double' you'll generate wrong code as well.

[Bug ipa/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jamborm at gcc dot gnu.org]

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

--- Comment #2 from Martin Jambor <jamborm at gcc dot gnu.org> ---
That cannot be the problem.  IPA-SRA re-creates the call statements
and builds them with gimple_build_call_vec (callee_decl, vargs); where
calle_decl is the new function which has had its type adjusted and
everything.  Indeed, IPA-SRA does not remove an argument here, it just
tries to push one dereference to the caller and the printed type is
correct.

Even in the release_ssa dump, pre-IPA, the constructor has two
parameters but is called with just one.  At the moment it looks to me
that IPA-SRA makes an already invalid IL somehow a bit worse.  Indeed,
in the revision before 424deca72b6 the constructor in the release_ssa
dump only has one parameter.

Jason, is it possible a wrong constructor is emitted to the IL?

(And note that tree-ssa-uninit.c should not ICE either, because with
K&R C input or malformed LTO we can have weird mismatches too.)

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jamborm at gcc dot gnu.org]

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

Martin Jambor <jamborm at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
          Component|ipa                         |c++

--- Comment #3 from Martin Jambor <jamborm at gcc dot gnu.org> ---
This is the gimple dump (omitting only function main), note the mismatch
between formal parameters of B::B and the actual arguments of its call:

void C::C (struct C * const this)
{
  *this = {CLOBBER};
  {
    this->D.2385._vptr.B = 0B;
    _1 = &this->D.2385;
    B::B (_1);
    _2 = &_ZTV1C + 24;
    this->D.2385._vptr.B = _2;
  }
}


void B::B (struct B * const this, const void * * __vtt_parm)
{
  _1 = *__vtt_parm;
  this->_vptr.B = _1;
}

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[msebor at gcc dot gnu.org]

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

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |msebor at gcc dot gnu.org

--- Comment #4 from Martin Sebor <msebor at gcc dot gnu.org> ---
It's easy enough to change tree-ssa-uninit.c to avoid assuming that a call has
as many arguments as the called function type indicates by calling
gimple_call_arg() with an index in excess of gimple_call_num_args().  The last
time we had something like this happen the decision was to make the type match:
pr95581.  If this case is different I can adjust the uninit pass.

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[marxin at gcc dot gnu.org]

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

--- Comment #5 from Martin Liška <marxin at gcc dot gnu.org> ---
> 
> Jason, is it possible a wrong constructor is emitted to the IL?
>

Thank you Martin for the analysis. That explains why the bisection point to
Jason's revision.

@Jason: Can you please take a look?

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jakub at gcc dot gnu.org]

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

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jakub at gcc dot gnu.org

--- Comment #6 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
If the testcase is compiled with -O0, then it looks clearly like a wrong-code:
  B::B (_1);
is the call in C::C, while the ctor it is calling is:
void B::B (struct B * const this, const void * * __vtt_parm)
{
  const void * _1;

  <bb 2> :
  _1 = *__vtt_parm_3(D);
  this_4(D)->_vptr.B = _1;
  return;

}
        pushq   %rbp
        movq    %rsp, %rbp
        movq    %rdi, -8(%rbp)
        movq    %rsi, -16(%rbp)
        movq    -16(%rbp), %rax
        movq    (%rax), %rdx
        movq    -8(%rbp), %rax
        movq    %rdx, (%rax)
        nop
        popq    %rbp
and actually uses the second argument which is not passed.

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
The
+  if (!fn || DECL_HAS_IN_CHARGE_PARM_P (fn))                                   
+    /* The base constructor has more parameters, so we can't just change the   
+       call target.  It would be possible to splice in the appropriate         
+       arguments, but probably not worth the complexity.  */                   
+    return false;                                                              
looks strange, isn't DECL_HAS_IN_CHARGE_PARM_P (fn) false on all
base constructors (as those are the abstract ctors with the in_charge parameter
removed and implied to be false)?
I would have expected
  if (!fn || DECL_HAS_VTT_PARM_P (fn))
instead but then that is true even on the elide4.C testcase.
But even elide4.C seems to suffer from this bug, in *.gimple I see:
    _3 = &this->D.2458;
    B::B (_3);
    goto <D.2555>;
    <D.2554>:
    _4 = &this->D.2458;
    B::B (_4);
in void C::C (struct C * const this), but the ctor they are actually calling
is:
void B::B (struct B * const this, const void * * __vtt_parm)
{
  _1 = *__vtt_parm;
  this->_vptr.B = _1;
}

Therefore, I'd say either it should be that DECL_HAS_VTT_PARM_P (fn) but then
the question is whether the change does something useful, or
for DECL_HAS_VTT_PARM_P we need to adjust the AGGR_INIT_EXPR too, to pass some
virtual table pointer (no idea what).
Anyway, that's all from me I'm afraid.

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
           Assignee|unassigned at gcc dot gnu.org      |jason at gcc dot gnu.org

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jason at gcc dot gnu.org]

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

--- Comment #8 from Jason Merrill <jason at gcc dot gnu.org> ---
(In reply to Jakub Jelinek from comment #7)
> looks strange, isn't DECL_HAS_IN_CHARGE_PARM_P (fn) false on all
> base constructors (as those are the abstract ctors with the in_charge
> parameter removed and implied to be false)?
> I would have expected
>   if (!fn || DECL_HAS_VTT_PARM_P (fn))
> instead

Yes, that's the bug, thanks.

> But even elide4.C seems to suffer from this bug

With the bug fixed, elide4.C doesn't elide, and that's OK.

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[cvs-commit at gcc dot gnu.org]

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

--- Comment #9 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:

https://gcc.gnu.org/g:90cbc769006a43ed17d2384b3a0a4634f315d3fd

commit r11-6866-g90cbc769006a43ed17d2384b3a0a4634f315d3fd
Author: Jason Merrill <jason@redhat.com>
Date:   Fri Jan 22 11:57:27 2021 -0500

    c++: Fix base copy elision thinko [PR98744]

    As Jakub points out in the PR, I was mixing up
    DECL_HAS_IN_CHARGE_PARM_P (which is true for the abstract maybe-in-charge
    constructor) and DECL_HAS_VTT_PARM_P (which is true for a base constructor
    that needs to handle virtual bases).

    gcc/cp/ChangeLog:

            PR c++/98744
            * call.c (make_base_init_ok): Use DECL_HAS_VTT_PARM_P.

    gcc/testsuite/ChangeLog:

            PR c++/98744
            * g++.dg/init/elide7.C: New test.

[Bug c++/98744] [11 Regression] ICE in gimple_call_arg, at gimple.h:3246 since r11-6735-g424deca72b63e644

[jason at gcc dot gnu.org]

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

Jason Merrill <jason at gcc dot gnu.org> changed:

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

--- Comment #10 from Jason Merrill <jason at gcc dot gnu.org> ---
Fixed.