Re: [hatari-devel] undefined behaviour fixes

[Andreas Grabher <andreas_g86@xxxxxxxxxx>]

> Am 11.01.2025 um 08:30 schrieb 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.
> 

Great! I now also applied it to Previous. As before there are only these two shifting greater than type width warnings left. I recognised, that offset is never greater than 31. So I simplified my patch a bit (appended). Maybe Toni can have a look.

diff -ru a/src/cpu/gencpu.c b/src/cpu/gencpu.c
--- a/src/cpu/gencpu.c	2024-05-04 20:10:03
+++ b/src/cpu/gencpu.c	2025-01-11 09:51:09
@@ -9224,7 +9224,7 @@
 			if (curi->dmode == Dreg) {
 				out("uae_u32 tmp = m68k_dreg(regs, dstreg);\n");
 				out("offset &= 0x1f;\n");
-				out("tmp = (tmp << offset) | (tmp >> (32 - offset));\n");
+				out("if(offset) tmp = (tmp << offset) | (tmp >> (32 - offset));\n");
 				out("bdata[0] = tmp & ((1 << (32 - width)) - 1);\n");
 			} else {
 				out("uae_u32 tmp;\n");
@@ -9273,7 +9273,7 @@
 				|| curi->mnemo == i_BFINS) {
 					if (curi->dmode == Dreg) {
 						out("tmp = bdata[0] | (tmp << (32 - width));\n");
-						out("m68k_dreg(regs, dstreg) = (tmp >> offset) | (tmp << (32 - offset));\n");
+						out("m68k_dreg(regs, dstreg) = offset ? ((tmp >> offset) | (tmp << (32 - offset))) : tmp;\n");
 					} else {
 						out("%s(dsta, bdata, tmp, offset, width);\n", putb);
 					}
diff -ru a/src/cpu/readcpu.c b/src/cpu/readcpu.c
--- a/src/cpu/readcpu.c	2022-12-29 16:51:59
+++ b/src/cpu/readcpu.c	2025-01-06 14:04:13
@@ -868,9 +868,11 @@
 	for (srcreg=0; srcreg < sbitdst; srcreg++) {
 		for (dstreg=0; dstreg < dstend; dstreg++) {
 			uae_u16 code = (uae_u16)opcode;
+			uae_u8 spos = (table68k[opcode].spos < 0) ? 0 : table68k[opcode].spos;
+			uae_u8 dpos = (table68k[opcode].dpos < 0) ? 0 : table68k[opcode].dpos;
 
-			code = (code & ~smsk) | (srcreg << table68k[opcode].spos);
-			code = (code & ~dmsk) | (dstreg << table68k[opcode].dpos);
+			code = (code & ~smsk) | (srcreg << spos);
+			code = (code & ~dmsk) | (dstreg << dpos);
 
 			/* Check whether this is in fact the same instruction.
 			* The instructions should never differ, except for the