[Bug target/106097] New: undefined behaviors regarding integer shifts in loongarch_build_integer

[Bug target/106097] New: undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

            Bug ID: 106097
           Summary: undefined behaviors regarding integer shifts in
                    loongarch_build_integer
           Product: gcc
           Version: 13.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: xry111 at mengyan1223 dot wang
  Target Milestone: ---

UBSan reports these undefined behaviors building a cross compiler for
loongarch64-linux-gnu on x86_64-linux-gnu:

/home/xry111/git-repos/gcc-la-build/./gcc/xgcc
-B/home/xry111/git-repos/gcc-la-build/./gcc/ -xc -nostdinc /dev/null -S -o
/dev/null -fself-test=../../gcc/gcc/testsuite/selftests
../../gcc/gcc/config/loongarch/loongarch.cc:1471:49: runtime error: left shift
of 4294967296 by 32 places cannot be represented in type 'long int'
../../gcc/gcc/config/loongarch/loongarch.cc:1520:49: runtime error: left shift
of negative value -524288
../../gcc/gcc/config/loongarch/loongarch.cc:1515:38: runtime error: left shift
of negative value -2048

/home/xry111/git-repos/gcc-la-build/./gcc/xgcc
-B/home/xry111/git-repos/gcc-la-build/./gcc/ -xc++ -nostdinc /dev/null -S -o
/dev/null -fself-test=../../gcc/gcc/testsuite/selftests
../../gcc/gcc/config/loongarch/loongarch.cc:1471:49: runtime error: left shift
of 4294967296 by 32 places cannot be represented in type 'long int'
../../gcc/gcc/config/loongarch/loongarch.cc:1520:49: runtime error: left shift
of negative value -524288
../../gcc/gcc/config/loongarch/loongarch.cc:1515:38: runtime error: left shift
of negative value -2048

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[chenglulu at loongson dot cn]

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

--- Comment #1 from chenglulu <chenglulu at loongson dot cn> ---

How can I reproduce the problem?
Thanks!
Lulu Cheng

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[pinskia at gcc dot gnu.org]

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

--- Comment #2 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
by using the --with-build-config=bootstrap-ubsan option at configure time or
BUILD_CONFIG variable to build time.

See https://gcc.gnu.org/install/build.html

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

--- Comment #3 from Xi Ruoyao <xry111 at mengyan1223 dot wang> ---
(In reply to Andrew Pinski from comment #2)
> by using the --with-build-config=bootstrap-ubsan option at configure time or
> BUILD_CONFIG variable to build time.
> 
> See https://gcc.gnu.org/install/build.html

The problem is -fsanitize=undefined does not work on LoongArch for now.  But it
can be reproduced by building a cross compile on x86_64:

/path/to/gcc/configure --target=loongarch64-linux-gnu
make {C,CXX}FLAGS="-O2 -g -fsanitize=undefined" all-gcc

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

--- Comment #4 from Xi Ruoyao <xry111 at mengyan1223 dot wang> ---
BTW I found this issue trying to triage PR106096, but I think it's not related
to this one.

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

--- Comment #5 from Xi Ruoyao <xry111 at mengyan1223 dot wang> ---
And it actually does not need a reproducer: "x << 32 >> 32" for sign-extension
is undefined by C++ standard if x is negative:

> The value of E1 << E2 is E1 left-shifted E2 bit positions; vacated bits are 
> zero-filled. If E1 has an unsigned type, the value of the result is
> E1 × 2^{E2}, reduced modulo one more than the maximum value representable in
> the result type. Otherwise, if E1 has a signed type and non-negative value,
> and E1 × 2^{E2} is representable in the corresponding unsigned type of the
> result type, then that value, converted to the result type, is the resulting
> value; otherwise, the behavior is undefined.

And the result of right shifting negative numbers is implementation-defined
(zext or sext), though GCC always uses sext.

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[chenglulu at loongson dot cn]

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

--- Comment #6 from chenglulu <chenglulu at loongson dot cn> ---
Created attachment 53205
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53205&action=edit
0001-Fix-bug-for-PR16097.patch

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

--- Comment #7 from Xi Ruoyao <xry111 at mengyan1223 dot wang> ---
(In reply to chenglulu from comment #6)
> Created attachment 53205 [details]
> 0001-Fix-bug-for-PR16097.patch

You can reuse LU32I_B and LU52I_B instead of hard coding those long constants
:).

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[chenglulu at loongson dot cn]

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

--- Comment #8 from chenglulu <chenglulu at loongson dot cn> ---
> You can reuse LU32I_B and LU52I_B instead of hard coding those long
> constants :).

I have fixed it, thanks！:)

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[chenglulu at loongson dot cn]

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

--- Comment #9 from chenglulu <chenglulu at loongson dot cn> ---
Created attachment 53206
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=53206&action=edit
use LU52I_B and LU32I_B instead of hard coding those long

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at mengyan1223 dot wang]

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

--- Comment #10 from Xi Ruoyao <xry111 at mengyan1223 dot wang> ---
(In reply to chenglulu from comment #9)
> Created attachment 53206 [details]
> use LU52I_B and LU32I_B instead of hard coding those long

> +      codes[cost].value = (value & LU32I_B)
> +	| (sign51 ? LU52I_B : 0);

nit： I think this can fit in one line.

Otherwise LGTM.  As the port maintainer you can push it directly into master. 
Normally we should bootstrap and regtest before applying a patch, but currently
the bootstrap is blocked by PR106096 :(.

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[chenglulu at loongson dot cn]

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

--- Comment #11 from chenglulu <chenglulu at loongson dot cn> ---

> Otherwise LGTM.  As the port maintainer you can push it directly into
> master.  Normally we should bootstrap and regtest before applying a patch,
> but currently the bootstrap is blocked by PR106096 :(.
Ok, I will submit patches in order.

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at gcc dot gnu.org]

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

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

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

--- Comment #12 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
Fixed at r13-1337 and r12-8252.

Interesting: why aren't we receiving a cvs-commit notification here?

[Bug target/106097] undefined behaviors regarding integer shifts in loongarch_build_integer

[xry111 at gcc dot gnu.org]

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

Xi Ruoyao <xry111 at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |12.2

--- Comment #13 from Xi Ruoyao <xry111 at gcc dot gnu.org> ---
(In reply to Xi Ruoyao from comment #12)
> Fixed at r13-1337 and r12-8252.
> 
> Interesting: why aren't we receiving a cvs-commit notification here?

It seems the cvs-commit script cannot handle non-ASCII characters ("×") in
commit message :(.

Setting target milestone as it's already fixed for 12.2.