From df9fd9f7bd0500dc0ff010cb845a024046203ada Mon Sep 17 00:00:00 2001
From: Sylvain Henry <sylvain@haskus.fr>
Date: Thu, 30 Nov 2023 11:15:12 +0100
Subject: [PATCH] JS: handle stored null StablePtr

Some Haskell codes unsafely cast StablePtr into ptr to compare against
NULL. E.g. in direct-sqlite:

  if castStablePtrToPtr aggStPtr /= nullPtr then

where `aggStPtr` is read (`peek`) from zeroed memory initially.

We fix this by giving these StablePtr the same representation as other
null pointers. It's safe because StablePtr at offset 0 is unused (for
this exact reason).
---
 compiler/GHC/StgToJS/Prim.hs | 16 ++++++++++------
 rts/js/stableptr.js          |  7 +++++++
 2 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/compiler/GHC/StgToJS/Prim.hs b/compiler/GHC/StgToJS/Prim.hs
index dcd360b0d498..414b7251acf3 100644
--- a/compiler/GHC/StgToJS/Prim.hs
+++ b/compiler/GHC/StgToJS/Prim.hs
@@ -1348,23 +1348,27 @@ write_boff_addr a i r o = mconcat
 
 read_stableptr :: JStgExpr -> JStgExpr -> JStgExpr -> JStgExpr -> JStgStat
 read_stableptr a i r o = mconcat
-  [ r |= var "h$stablePtrBuf" -- stable pointers are always in this array
-  , o |= read_i32 a i
+  [ o |= read_i32 a i
+  , ifS (o .===. zero_)
+      (r |= null_)
+      (r |= var "h$stablePtrBuf")
   ]
 
 read_boff_stableptr :: JStgExpr -> JStgExpr -> JStgExpr -> JStgExpr -> JStgStat
 read_boff_stableptr a i r o = mconcat
-  [ r |= var "h$stablePtrBuf" -- stable pointers are always in this array
-  , o |= read_boff_i32 a i
+  [ o |= read_boff_i32 a i
+  , ifS (o .===. zero_)
+      (r |= null_)
+      (r |= var "h$stablePtrBuf")
   ]
 
 write_stableptr :: JStgExpr -> JStgExpr -> JStgExpr -> JStgExpr -> JStgStat
 write_stableptr a i _r o = write_i32 a i o
-  -- don't store "r" as it must be h$stablePtrBuf
+  -- don't store "r" as it must be h$stablePtrBuf or null
 
 write_boff_stableptr :: JStgExpr -> JStgExpr -> JStgExpr -> JStgExpr -> JStgStat
 write_boff_stableptr a i _r o = write_boff_i32 a i o
-  -- don't store "r" as it must be h$stablePtrBuf
+  -- don't store "r" as it must be h$stablePtrBuf or null
 
 write_u8 :: JStgExpr -> JStgExpr -> JStgExpr -> JStgStat
 write_u8 a i v = idx_u8 a i |= v
diff --git a/rts/js/stableptr.js b/rts/js/stableptr.js
index 82fc2d336c8b..7912137ef0ad 100644
--- a/rts/js/stableptr.js
+++ b/rts/js/stableptr.js
@@ -18,6 +18,13 @@ var h$stablePtrData = [null];
 var h$stablePtrBuf  = h$newByteArray(8);
 var h$stablePtrN    = 1;
 var h$stablePtrFree = [];
+// Slot 0 isn't used as offset 0 is reserved for the null pointer. In
+// particular, when we store a StablePtr in an array, we don't store the array
+// part. When we read it back, we only have the offset. Some codes initialize
+// these stored StablePtr with NULL (hence offset 0) and if we were creating a
+// StablePtr from it (i.e. [$stablePtrBuf,0]) then we can't compare them to
+// nullPtr (castStablePtrToPtr [$stablePtrBuf,0] /= [null,0]).
+// This happens in direct-sqlite package for example.
 
 function h$makeStablePtr(v) {
   TRACE_STABLEPTR("makeStablePtr")
-- 
GitLab