[Bug target/100212] New: Shift by-1 in aarch64_classify_index

[Bug target/100212] New: Shift by-1 in aarch64_classify_index

[acoplan at gcc dot gnu.org]

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

            Bug ID: 100212
           Summary: Shift by-1 in aarch64_classify_index
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: acoplan at gcc dot gnu.org
  Target Milestone: ---

Bootstrapping on aarch64 --with-build-config=bootstrap-ubsan shows the
following issue (reduced from gcc.target/aarch64/sve/mask_struct_store_2.c):

$ cat sve_test.c
int *__restrict a;
void b() {
  for (int c; c; ++c)
    a[c] = a[c * 3];
}
$ gcc/xgcc -B gcc sve_test.c -c -march=armv8.2-a+sve -O -ftree-vectorize
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64.c:9556:10: runtime
error: shift exponent -1 is negative
    #0 0x2de7d7c in aarch64_classify_index
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64.c:9556
    #1 0x2deca68 in aarch64_classify_address(aarch64_address_info*, rtx_def*,
machine_mode, bool, aarch64_addr_query_type)
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64.c:9781
    #2 0x2df2bb8 in aarch64_legitimate_address_hook_p
/home/alecop01/toolchain/src/gcc/gcc/config/aarch64/aarch64.c:9946
    #3 0x1c673fc in memory_address_addr_space_p(machine_mode, rtx_def*,
unsigned char) /home/alecop01/toolchain/src/gcc/gcc/recog.c:1793
    #4 0x2389da4 in valid_mem_ref_p(machine_mode, unsigned char, mem_address*)
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-address.c:355
    #5 0x2500e84 in get_address_cost
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:4675
    #6 0x2500e84 in get_computation_cost
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:4881
    #7 0x2505f44 in determine_group_iv_cost_address
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:4990
    #8 0x2505f44 in determine_group_iv_cost
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:5575
    #9 0x2506b40 in determine_group_iv_costs
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:5769
    #10 0x251287c in tree_ssa_iv_optimize_loop
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:8034
    #11 0x251287c in tree_ssa_iv_optimize()
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop-ivopts.c:8083
    #12 0x258c574 in execute
/home/alecop01/toolchain/src/gcc/gcc/tree-ssa-loop.c:513
    #13 0x1b5ec1c in execute_one_pass(opt_pass*)
/home/alecop01/toolchain/src/gcc/gcc/passes.c:2567
    #14 0x1b623b8 in execute_pass_list_1
/home/alecop01/toolchain/src/gcc/gcc/passes.c:2656
    #15 0x1b623f0 in execute_pass_list_1
/home/alecop01/toolchain/src/gcc/gcc/passes.c:2657
    #16 0x1b623f0 in execute_pass_list_1
/home/alecop01/toolchain/src/gcc/gcc/passes.c:2657
    #17 0x1b624d4 in execute_pass_list(function*, opt_pass*)
/home/alecop01/toolchain/src/gcc/gcc/passes.c:2667
    #18 0xc7b2a4 in cgraph_node::expand()
/home/alecop01/toolchain/src/gcc/gcc/cgraphunit.c:1830
    #19 0xc7ff24 in expand_all_functions
/home/alecop01/toolchain/src/gcc/gcc/cgraphunit.c:1994
    #20 0xc7ff24 in symbol_table::compile()
/home/alecop01/toolchain/src/gcc/gcc/cgraphunit.c:2358
    #21 0xc8b81c in symbol_table::compile()
/home/alecop01/toolchain/src/gcc/gcc/cgraphunit.c:2271
    #22 0xc8b81c in symbol_table::finalize_compilation_unit()
/home/alecop01/toolchain/src/gcc/gcc/cgraphunit.c:2539
    #23 0x1f97478 in compile_file
/home/alecop01/toolchain/src/gcc/gcc/toplev.c:482
    #24 0x68d9b0 in do_compile
/home/alecop01/toolchain/src/gcc/gcc/toplev.c:2201
    #25 0x68d9b0 in toplev::main(int, char**)
/home/alecop01/toolchain/src/gcc/gcc/toplev.c:2340
    #26 0x6910d0 in main /home/alecop01/toolchain/src/gcc/gcc/main.c:39
    #27 0xffffb95236dc in __libc_start_main
(/lib/aarch64-linux-gnu/libc.so.6+0x206dc)
    #28 0x691b2c  (/data/alecop01/builds/bstrap-ubsan/gcc/cc1+0x691b2c)

[Bug target/100212] UB (shift by -1) in aarch64_classify_index

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2021-08-17
     Ever confirmed|0                           |1

--- Comment #1 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
      shift = exact_log2 (INTVAL (XEXP (XEXP (x, 0), 1)));
      if (INTVAL (XEXP (x, 1)) != (HOST_WIDE_INT)0xffffffff << shift)
        shift = -1;

Simple fix is check shift to be -1 before doing the other check.
That is:
      if (shift == -1
          || INTVAL (XEXP (x, 1)) != (HOST_WIDE_INT)0xffffffff << shift)
        shift = -1;
Or:
      if (shift != -1
          && INTVAL (XEXP (x, 1)) != (HOST_WIDE_INT)0xffffffff << shift)
        shift = -1;

Both are valid as shift will be -1 before or afterwards :).

[Bug target/100212] UB (shift by -1) in aarch64_classify_index

[pinskia at gcc dot gnu.org]

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

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

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
I am going to test a fix for this.

[Bug target/100212] UB (shift by -1) in aarch64_classify_index

[cvs-commit at gcc dot gnu.org]

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

--- Comment #3 from GCC Commits <cvs-commit at gcc dot gnu.org> ---
The trunk branch has been updated by Andrew Pinski <pinskia@gcc.gnu.org>:

https://gcc.gnu.org/g:0c2583dc2575f3f64e3d09e12c296eb56f01916d

commit r14-8441-g0c2583dc2575f3f64e3d09e12c296eb56f01916d
Author: Andrew Pinski <quic_apinski@quicinc.com>
Date:   Thu Jan 25 13:45:59 2024 -0800

    aarch64: Fix/avoid undefinedness in aarch64_classify_index [PR100212]

    The problem here is we don't check the return value of exact_log2
    and always use that result as shifter. This fixes the issue by avoiding
    the shift if the value was `-1` (which means the value was not exact a
power of 2);
    in this case we could either check if the values was equal to -1 or not
equal to because
    we then assign -1 to shift if the constant value was not equal. I chose
`!=` as
    it seemed to be more obvious of what the code is doing.

    Committed as obvious after a build/test for aarch64-linux-gnu.

    gcc/ChangeLog:

            PR target/100212
            * config/aarch64/aarch64.cc (aarch64_classify_index): Avoid
            undefined shift after the call to exact_log2.

    Signed-off-by: Andrew Pinski <quic_apinski@quicinc.com>

[Bug target/100212] UB (shift by -1) in aarch64_classify_index

[pinskia at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |14.0
         Resolution|---                         |FIXED
             Status|ASSIGNED                    |RESOLVED

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Fixed.