| Re: [hatari-devel] undefined behaviour fixes |
[ Thread Index |
Date Index
| More lists.tuxfamily.org/hatari-devel Archives
]
- To: hatari-devel@xxxxxxxxxxxxxxxxxxx
- Subject: Re: [hatari-devel] undefined behaviour fixes
- From: Andreas Grabher <andreas_g86@xxxxxxxxxx>
- Date: Thu, 23 Jan 2025 16:37:22 +0100
- Dkim-signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=icloud.com; s=1a1hai; bh=8C6X26mKSWGHbv3dvo243uisbN02cc9RobcYv43LeN8=; h=Content-Type:From:Mime-Version:Subject:Date:Message-Id:To:x-icloud-hme; b=B8qnfyr3mZvqkVf+2GT1MogCNt07j03XT/GDwz2tcrwrmQ0mdG6fsIl3Qo9YTS7Cj QNNzf0i8XJBrP/uxacuiplCYhitHJD2mbsZO5Yb1j00HO3x0aT3B5vU92w5CbVrHcB 1XmKSiaov7Mdzd0lbBlo4pFEdNeU2FH/3H4eGYWT1Zxc/Iv8rrb/asD1NZT4tRRPSZ 1E2IxIRXLIkczjuZicbx5I0t+2a4NFMZJca1jXs8Li4ZZwrSpuEicFM9gFdcL+IkwK ufKfzws8EfgNUtpjxDHUs4i33c7xr/as+gZrk8oxWUwVN4pkDttizkHAWkS0yodJ1g iBT7K+dBESrBg==
> Am 23.01.2025 um 08:14 schrieb Thomas Huth <th.huth@xxxxxxxxx>:
>
> On 07/01/2025 18.23, Andreas Grabher wrote:
> ...
>> I saw this commit and I am not sure if it should be done this way: https:// git.tuxfamily.org/hatari/hatari.git/commit/? id=108c56a1cb71e78326f142f0d892a6cadf5460dc <https://git.tuxfamily.org/ hatari/hatari.git/commit/?id=108c56a1cb71e78326f142f0d892a6cadf5460dc>
>> I think -fwrapv should always be set, because it produces different (safe from undefined behaviour) code. See here for an example: https:// stackoverflow.com/questions/47232954/what-does-fwrapv-do <https:// stackoverflow.com/questions/47232954/what-does-fwrapv-do>
>> Because I still think it is bad coding style to shift negative values and similar in my opinion we should only set -fwrapv where really necessary. So we should probably only do that in the CPU core, like in my previous patch. As I said previously, the parts of Hatari that are used in Previous including the DSP do not trigger any warning. So Hatari is aside from the CPU core likely to be clean and should stay like that.
>> Furthermore I think the CPU code should be cleaned from those cases on the long term. So probably do not set -fwrapv if the sanitiser is activated to keep the affected code visible.
>
> FWIW, I just spotted by accident that the new C23 standard will enforce two's complement for negative values:
>
> https://en.wikipedia.org/wiki/C23_(C_standard_revision)#Obsolete_features
>
> ... so I hope that undefined behavior with shifting negative values should be gone with future versions of the C compilers.
>
> Thomas
>
That does not necessarily mean that shifting negative values is going to be defined in the future, but i hope so.
Even with two‘s complement shifting 0b10000000 >> 1 might result in 0b01000000 or 0b11000000. Let‘s hope they also define how to handle the sign bit.