From 86ce5bc5d5018dcb62054d214b23f7a59b9c0d7d Mon Sep 17 00:00:00 2001 From: Cliff Click Date: Wed, 28 Oct 2026 13:39:05 -0700 Subject: Fix bugs x86 storing only 32bits of a large constant Missed field narrowing in a few cases --- .../simple/node/cpus/x86_64_v2/x86_64_v2.java | 2 +- .../com/seaofnodes/simple/X86StoreNativeTest.java | 50 ++++++++++++++++++++++ .../java/com/seaofnodes/simple/X86StoreTest.java | 35 +++++++++++++++ 3 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 chapter24b/src/test/java/com/seaofnodes/simple/X86StoreNativeTest.java create mode 100644 chapter24b/src/test/java/com/seaofnodes/simple/X86StoreTest.java (limited to 'chapter24b') diff --git a/chapter24b/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java b/chapter24b/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java index fcae17f8..1c452403 100644 --- a/chapter24b/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java +++ b/chapter24b/src/main/java/com/seaofnodes/simple/node/cpus/x86_64_v2/x86_64_v2.java @@ -551,7 +551,7 @@ public class x86_64_v2 extends Machine { private int imm( Node xval ) { assert val == null && imm == 0; - if( xval instanceof ConstantNode con && con._con instanceof TypeInteger ti) { + if( xval instanceof ConstantNode con && con._con instanceof TypeInteger ti && ti.isConstant() && imm32(ti.value()) ) { val = null; imm = (int) ti.value(); assert imm == ti.value(); // In 32-bit range diff --git a/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreNativeTest.java b/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreNativeTest.java new file mode 100644 index 00000000..f0c9d37f --- /dev/null +++ b/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreNativeTest.java @@ -0,0 +1,50 @@ +package com.seaofnodes.simple; + +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Path; +import org.junit.Test; + +public class X86StoreNativeTest { + @Test public void testConstantStoresAndAdds() throws IOException { + Path dir = Path.of("build/objs"); + Files.createDirectories(dir); + Files.writeString(dir.resolve("x86ConstantStore.smp"), """ + struct Narrow { u32 x; u32 guard; }; + struct Wide { int x; int guard; }; + val storeNarrow = { !Narrow s -> s.x=4294967295; return 0; }; + val storePositive = { !Wide s -> s.x=81985529216486895; return 0; }; + val storeNegative = { !Wide s -> s.x=-2147483649; return 0; }; + val addPositive = { !Wide s -> s.x+=4294967295; return 0; }; + val addNegative = { !Wide s -> s.x+=-2147483649; return 0; }; + """); + // Read from C so load forwarding cannot hide an incorrectly encoded Store. + Files.writeString(dir.resolve("x86ConstantStore.c"), """ + #include + struct Narrow { uint32_t x, guard; }; + struct Wide { int64_t x, guard; }; + extern long long storeNarrow(struct Narrow *); + extern long long storePositive(struct Wide *); + extern long long storeNegative(struct Wide *); + extern long long addPositive(struct Wide *); + extern long long addNegative(struct Wide *); + int main(void) { + struct Narrow n = {0, 0x12345678}; + struct Wide w = {0, 0x123456789abcdefLL}; + storeNarrow(&n); + if (n.x != UINT32_MAX || n.guard != 0x12345678) return 1; + storePositive(&w); + if (w.x != 81985529216486895LL || w.guard != 0x123456789abcdefLL) return 2; + storeNegative(&w); + if (w.x != -2147483649LL || w.guard != 0x123456789abcdefLL) return 3; + w.x = 7; + addPositive(&w); + if (w.x != 4294967302LL || w.guard != 0x123456789abcdefLL) return 4; + addNegative(&w); + if (w.x != 2147483653LL || w.guard != 0x123456789abcdefLL) return 5; + return 0; + } + """); + TestC.run("build/objs","x86ConstantStore","",-1); + } +} diff --git a/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreTest.java b/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreTest.java new file mode 100644 index 00000000..c07ac904 --- /dev/null +++ b/chapter24b/src/test/java/com/seaofnodes/simple/X86StoreTest.java @@ -0,0 +1,35 @@ +package com.seaofnodes.simple; + +import com.seaofnodes.simple.codegen.CodeGen; +import com.seaofnodes.simple.node.cpus.x86_64_v2.StoreX86; +import com.seaofnodes.simple.type.TypeInteger; +import org.junit.Test; +import static org.junit.Assert.*; + +public class X86StoreTest { + @Test public void testConstantStores() { + check("u8",255,false); + check("u16",65535,false); + check("u32",Integer.MAX_VALUE,false); + check("u32",2147483648L,true); + check("u32",4294967295L,true); + check("int",Integer.MIN_VALUE,false); + check("int",Integer.MAX_VALUE,false); + for( long value : new long[]{2147483648L,4294967295L,-2147483649L, + 0x123456789abcdefL,Long.MIN_VALUE,Long.MAX_VALUE} ) + check("int",value,true); + } + + private static void check(String type, long value, boolean register) { + // Returning the object keeps the Store; returning its field could fold. + String literal = value==Long.MIN_VALUE ? "(-9223372036854775807-1)" : Long.toString(value); + String src = "struct S { "+type+" x="+literal+"; }; return new S;"; + CodeGen code = new CodeGen(src).driver(CodeGen.Phase.Select,"x86_64_v2","SystemV"); + StoreX86 store = code._stop.walk(n -> n instanceof StoreX86 st && st._name.equals("x") ? st : null); + assertNotNull(src,store); + if( register ) { + assertNotNull(src,store.in(4)); + assertEquals(src,value,((TypeInteger)store.in(4)._type).value()); + } else assertNull(src,store.in(4)); + } +} -- cgit v1.2.3