Re: [hatari-devel] undefined behaviour fixes

[Thomas Huth <th.huth@xxxxxxxxx>]

Am Tue, 7 Jan 2025 18:23:30 +0100
schrieb Andreas Grabher <andreas_g86@xxxxxxxxxx>:
....
> 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
> 
> 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

Yeah, it likely makes sense to always enable it at least for the CPU core
which has clearly been written with twos-complement arithmetics in mind.

> 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.

It's not. There is at least one more spot in the sound code:

 src/sound.c:389:18: runtime error: left shift of negative value -893

Maybe Nicolas could have a look at it? ... those filters
are not really my turf.

> Furthermore I think the CPU code should be cleaned from those cases on the long term.

That's a question for Toni to answer, for sure.

 Thomas