[Bug ipa/109886] New: UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[Bug ipa/109886] New: UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[jamborm at gcc dot gnu.org]

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

            Bug ID: 109886
           Summary: UBSAN error: shift exponent 64 is too large for 64-bit
                    type when compiling gcc.c-torture/compile/pr96796.c
           Product: gcc
           Version: 14.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: ipa
          Assignee: unassigned at gcc dot gnu.org
          Reporter: jamborm at gcc dot gnu.org
                CC: aldyh at gcc dot gnu.org, marxin at gcc dot gnu.org
  Target Milestone: ---
              Host: x86_64-linux-gnu
            Target: x86_64-linux-gnu

Bootstrap with undefined behavior sanitizer and subsequent run of
the testsuite (on revision ac3a5bbc629, so check for PR 109788 is
included) reports a new error when compiling C torture testcase
gcc/testsuite/gcc.c-torture/compile/pr96796.c:

$ UBSAN_OPTIONS="halt_on_error=1 print_stacktrace=1"
/home/mjambor/gcc/mine/b-obj/gcc/xgcc -B/home/mjambor/gcc/mine/b-obj/gcc/
-fdiagnostics-plain-output -O1 -w -fcommon -c -o pr96796.o
/home/mjambor/gcc/mine/src/gcc/testsuite/gcc.c-torture/compile/pr96796.c

/home/mjambor/gcc/mine/src/gcc/hwint.h:293:61: runtime error: shift exponent 64
is too large for 64-bit type 'long unsigned int'

    #0 0xbf8117 in sext_hwi(long, unsigned int)
/home/mjambor/gcc/mine/src/gcc/hwint.h:293
    #1 0xbf8117 in wi::hwi_with_prec::hwi_with_prec(long, unsigned int, signop)
/home/mjambor/gcc/mine/src/gcc/wide-int.h:1622
    #2 0xbf8117 in wi::shwi(long, unsigned int)
/home/mjambor/gcc/mine/src/gcc/wide-int.h:1631
    #3 0xbf8117 in wi::minus_one(unsigned int)
/home/mjambor/gcc/mine/src/gcc/wide-int.h:1645
    #4 0xbf8117 in irange::set_varying(tree_node*)
/home/mjambor/gcc/mine/src/gcc/value-range.h:871
    #5 0x2257e45 in range_cast(vrange&, tree_node*)
/home/mjambor/gcc/mine/src/gcc/range-op.cc:4860
    #6 0x1b119a6 in ipa_compute_jump_functions_for_edge
/home/mjambor/gcc/mine/src/gcc/ipa-prop.cc:2325
    #7 0x1b14f66 in ipa_compute_jump_functions_for_bb
/home/mjambor/gcc/mine/src/gcc/ipa-prop.cc:2449
    #8 0x1b14f66 in analysis_dom_walker::before_dom_children(basic_block_def*)
/home/mjambor/gcc/mine/src/gcc/ipa-prop.cc:3035
    #9 0x65a5ff3 in dom_walker::walk(basic_block_def*)
/home/mjambor/gcc/mine/src/gcc/domwalk.cc:311
    #10 0x1b0e601 in ipa_analyze_node(cgraph_node*)
/home/mjambor/gcc/mine/src/gcc/ipa-prop.cc:3103
    #11 0x1991487 in inline_indirect_intraprocedural_analysis
/home/mjambor/gcc/mine/src/gcc/ipa-fnsummary.cc:4315
    #12 0x1991487 in inline_analyze_function(cgraph_node*)
/home/mjambor/gcc/mine/src/gcc/ipa-fnsummary.cc:4334
    #13 0x1991afc in ipa_fn_summary_generate
/home/mjambor/gcc/mine/src/gcc/ipa-fnsummary.cc:4378
    #14 0x21351c1 in execute_ipa_summary_passes(ipa_opt_pass_d*)
/home/mjambor/gcc/mine/src/gcc/passes.cc:2304
    #15 0x10a2163 in ipa_passes
/home/mjambor/gcc/mine/src/gcc/cgraphunit.cc:2235
    [...]

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[pinskia at gcc dot gnu.org]

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

Andrew Pinski <pinskia at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2023-05-17
             Blocks|                            |63426
           Keywords|                            |ice-on-valid-code
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Breakpoint 6, range_cast (r=..., type=<record_type 0x7ffff73eda80 S0>) at
/home/apinski/src/upstream-gcc/gcc/gcc/range-op.cc:4853
4853      Value_Range tmp (r);


Confirmed.
The code looks like:
```
int g_5, func_1_l_32, func_50___trans_tmp_31;
...
int func_1() { func_50(g_3_4, g_5, func_1_l_32, 8, 3); }
...
struct S0 *func_50(int p_51, struct S0 p_52, struct S1 p_53, int p_54,
                   int p_55) {
...
}
```

Code in gcc:
          if (TREE_CODE (arg) == SSA_NAME
              && param_type
              /* Limit the ranger query to integral types as the rest
                 of this file uses value_range's, which only hold
                 integers and pointers.  */
              && irange::supports_p (TREE_TYPE (arg))
              && get_range_query (cfun)->range_of_expr (vr, arg)
              && !vr.undefined_p ())
            {
              value_range resvr = vr;
              range_cast (resvr, param_type);
              if (!resvr.undefined_p () && !resvr.varying_p ())
                ipa_set_jfunc_vr (jfunc, &resvr);
              else
                gcc_assert (!jfunc->m_vr);
            }
          else
            gcc_assert (!jfunc->m_vr);


Maybe there should be an extra check for `irange::supports_p (param_type)` too
to catch the case where param_type type is not supported at all.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=63426
[Bug 63426] [meta-bug] Issues found with -fsanitize=undefined

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[aldyh at gcc dot gnu.org]

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

--- Comment #2 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #1)
> Breakpoint 6, range_cast (r=..., type=<record_type 0x7ffff73eda80 S0>) at
> /home/apinski/src/upstream-gcc/gcc/gcc/range-op.cc:4853
> 4853      Value_Range tmp (r);
> 
> 
> Confirmed.
> The code looks like:
> ```
> int g_5, func_1_l_32, func_50___trans_tmp_31;
> ...
> int func_1() { func_50(g_3_4, g_5, func_1_l_32, 8, 3); }
> ...
> struct S0 *func_50(int p_51, struct S0 p_52, struct S1 p_53, int p_54,
>                    int p_55) {
> ...
> }
> ```
> 
> Code in gcc:
>           if (TREE_CODE (arg) == SSA_NAME
>               && param_type
>               /* Limit the ranger query to integral types as the rest
>                  of this file uses value_range's, which only hold
>                  integers and pointers.  */
>               && irange::supports_p (TREE_TYPE (arg))
>               && get_range_query (cfun)->range_of_expr (vr, arg)
>               && !vr.undefined_p ())
>             {
>               value_range resvr = vr;
>               range_cast (resvr, param_type);
>               if (!resvr.undefined_p () && !resvr.varying_p ())
>                 ipa_set_jfunc_vr (jfunc, &resvr);
>               else
>                 gcc_assert (!jfunc->m_vr);
>             }
>           else
>             gcc_assert (!jfunc->m_vr);
> 
> 
> Maybe there should be an extra check for `irange::supports_p (param_type)`
> too to catch the case where param_type type is not supported at all.

If irange::supports_p (TREE_TYPE (arg)) is true, we're talking about an
integer/pointer, but if range_cast is being called on a parm_type of
RECORD_TYPE, someone's trying to cast a structure to an integer.  Is that the
intent here, because that will not work with ranges??

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[pinskia at gcc dot gnu.org]

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

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #2)
> If irange::supports_p (TREE_TYPE (arg)) is true, we're talking about an
> integer/pointer, but if range_cast is being called on a parm_type of
> RECORD_TYPE, someone's trying to cast a structure to an integer.  Is that
> the intent here, because that will not work with ranges??

That is correct. The generated code has a VIEW_CONVERT_EXR from an integer type
to a RECORD_TYPE.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[aldyh at gcc dot gnu.org]

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

--- Comment #4 from Aldy Hernandez <aldyh at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #3)
> (In reply to Aldy Hernandez from comment #2)
> > If irange::supports_p (TREE_TYPE (arg)) is true, we're talking about an
> > integer/pointer, but if range_cast is being called on a parm_type of
> > RECORD_TYPE, someone's trying to cast a structure to an integer.  Is that
> > the intent here, because that will not work with ranges??
> 
> That is correct. The generated code has a VIEW_CONVERT_EXR from an integer
> type to a RECORD_TYPE.

Eeeech.  In that case, then what you suggest is reasonable.  Bail if param_type
is not supported by the underlying range.  Maybe the IPA experts could opine?

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[jamborm at gcc dot gnu.org]

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

--- Comment #5 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Aldy Hernandez from comment #4)
> (In reply to Andrew Pinski from comment #3)
> > (In reply to Aldy Hernandez from comment #2)
> > > If irange::supports_p (TREE_TYPE (arg)) is true, we're talking about an
> > > integer/pointer, but if range_cast is being called on a parm_type of
> > > RECORD_TYPE, someone's trying to cast a structure to an integer.  Is that
> > > the intent here, because that will not work with ranges??
> > 
> > That is correct. The generated code has a VIEW_CONVERT_EXR from an integer
> > type to a RECORD_TYPE.
> 
> Eeeech.  In that case, then what you suggest is reasonable.  Bail if
> param_type is not supported by the underlying range.  Maybe the IPA experts
> could opine?

With LTOed type mismateches or with K&R style code, IPA has to be prepared to
deal with such cases, unfortunately.  So a check like that indeed looks
reasonable.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[amacleod at redhat dot com]

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

Andrew Macleod <amacleod at redhat dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |amacleod at redhat dot com

--- Comment #6 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Martin Jambor from comment #5)
> (In reply to Aldy Hernandez from comment #4)
> > (In reply to Andrew Pinski from comment #3)

> > > That is correct. The generated code has a VIEW_CONVERT_EXR from an integer
> > > type to a RECORD_TYPE.
> > 
> > Eeeech.  In that case, then what you suggest is reasonable.  Bail if
> > param_type is not supported by the underlying range.  Maybe the IPA experts
> > could opine?
> 
> With LTOed type mismateches or with K&R style code, IPA has to be prepared
> to deal with such cases, unfortunately.  So a check like that indeed looks
> reasonable.

The new range-op dispatch code is coming shortly.. when an unsupported type is
passed in to any ranger routine, we'll simply return false instead of trapping
like we do now.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[jamborm at gcc dot gnu.org]

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

--- Comment #7 from Martin Jambor <jamborm at gcc dot gnu.org> ---
I see, thanks!  But I wonder whether it would make sense to commit the simple
fix in the meantime so that the test pass.  It is easy to miss new regressions
now when I expect the overall result not to be clean.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[amacleod at redhat dot com]

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

--- Comment #8 from Andrew Macleod <amacleod at redhat dot com> ---
I just checked in:

commit cd9c7f898d6e6313465f78227e566f27dce5e5a3 
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Wed May 31 13:10:31 2023 -0400

    Provide a new dispatch mechanism for range-ops.

As a side effect it should make range_cast simply return varying if the RHS is
an unsupported type or unimplemented cast.

Check if this resolves this PR.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[jamborm at gcc dot gnu.org]

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

--- Comment #9 from Martin Jambor <jamborm at gcc dot gnu.org> ---
A buildbot run which checked out this revision unfortunately still reports this
problem with UBSAN-bootstrapped compiler.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[amacleod at redhat dot com]

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

--- Comment #10 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Martin Jambor from comment #9)
> A buildbot run which checked out this revision unfortunately still reports
> this problem with UBSAN-bootstrapped compiler.

Oh, I see..  there's a second problem, and that is that range_cast doesn't
check if the type is supported before it blindly sets it to varying :-P

One more patcxh coming today to deal with that.. I was going to move cast to
the header file anyway, I will tweak the definition at the same time.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[amacleod at redhat dot com]

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

--- Comment #11 from Andrew Macleod <amacleod at redhat dot com> ---
(In reply to Martin Jambor from comment #9)
> A buildbot run which checked out this revision unfortunately still reports
> this problem with UBSAN-bootstrapped compiler.

Actually, I do not think that range_cast is the root problem upon further
review... Discussing it with Aldy, I think there is a typo in the code:

  value_range resvr = vr;
  range_cast (resvr, param_type);

That should be

  Value_Range resvr = vr;
  range_cast (resvr, param_type);

value_range is the legacy int_range (which should be going away this release)
and Value_Range is the new range type that supports multiple ranges, including
unsupported ranges for UNDEFINED and VARYING.

So the set_varying call in range_cast is trying to cast the integer to a
different class which isnt supported by int_range.

BUt then, i see that ipa_set_jfunc_vr (jfunc, &resvr); is expecting a
value_range *, so perhapsd this is really a can of worms.  we're checking that
arg is an irange supported, we should also perhaps just check that param_type
is too...

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[cvs-commit at gcc dot gnu.org]

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

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

https://gcc.gnu.org/g:5612aa4d06594166c0ee848dc733bb9458c1bdbf

commit r14-1662-g5612aa4d06594166c0ee848dc733bb9458c1bdbf
Author: Andrew MacLeod <amacleod@redhat.com>
Date:   Fri Jun 9 10:17:59 2023 -0400

    Also check type being cast to

    before casting into an irange, make sure the type being cast into
    is also supported.

            PR ipa/109886
            * ipa-prop.cc (ipa_compute_jump_functions_for_edge): Check param
            type as well.

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[amacleod at redhat dot com]

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

--- Comment #13 from Andrew Macleod <amacleod at redhat dot com> ---
Let me know if the buildbot likes that change :-)

[Bug ipa/109886] UBSAN error: shift exponent 64 is too large for 64-bit type when compiling gcc.c-torture/compile/pr96796.c

[jamborm at gcc dot gnu.org]

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

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

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

--- Comment #14 from Martin Jambor <jamborm at gcc dot gnu.org> ---
(In reply to Andrew Macleod from comment #13)
> Let me know if the buildbot likes that change :-)

The buildbot seems happy now, thanks!