Simon Jakobi pushed to branch wip/sjakobi/T26964 at Glasgow Haskell Compiler / GHC Commits: 64b4252e by Simon Jakobi at 2026-06-04T01:37:58+02:00 Improve -fcheck-prim-bounds runtime error messages When an array access instrumented by -fcheck-prim-bounds fails at runtime, report the failing primop, the offending index and the array size, and exit with a normal failure status. Previously the program aborted via barf with an "internal error" framed as a GHC bug, even though such failures are almost always caused by incorrect use of unsafe primops in user or library code. The primop name is threaded to the failure handlers through a new fcs_prim_op field in FCodeState, set once in cmmPrimOpApp, so the bounds-check sites themselves are unchanged. Fixes #26964, #24617. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> - - - - - 6bf57c99 by Simon Jakobi at 2026-06-04T01:37:58+02:00 rts: name the primop in DEBUG ASSERT_IN_BOUNDS failures ASSERT_IN_BOUNDS passed the literal "array access" as the op name, which collided with rtsOutOfBoundsAccess's own message text ("<op>: array access out of bounds"), producing the redundant "array access: array access out of bounds". Thread the real primop name (e.g. "casIntArray#") through the macro instead, matching the native code generator's messages. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> - - - - - 12 changed files: - + changelog.d/improve-check-prim-bounds-messages - compiler/GHC/StgToCmm/Monad.hs - compiler/GHC/StgToCmm/Prim.hs - docs/users_guide/debugging.rst - rts/PrimOps.cmm - rts/RtsMessages.c - rts/include/rts/Messages.h - + testsuite/tests/codeGen/should_fail/T26964.hs - + testsuite/tests/codeGen/should_fail/T26964.stderr - + testsuite/tests/codeGen/should_fail/T26964b.hs - + testsuite/tests/codeGen/should_fail/T26964b.stderr - testsuite/tests/codeGen/should_fail/all.T Changes: ===================================== changelog.d/improve-check-prim-bounds-messages ===================================== @@ -0,0 +1,13 @@ +section: codegen +synopsis: Improve ``-fcheck-prim-bounds`` runtime error messages +issues: #26964 #24617 +mrs: !16130 + +description: { + When an array access instrumented by ``-fcheck-prim-bounds`` fails at + runtime, the program now reports the failing primop, the offending index and + the array size, and exits with a normal non-zero status. Previously it + aborted with an "internal error" framed as a GHC bug, even though such + failures are almost always caused by incorrect use of unsafe primops in user + or library code. +} ===================================== compiler/GHC/StgToCmm/Monad.hs ===================================== @@ -42,6 +42,7 @@ module GHC.StgToCmm.Monad ( SelfLoopInfo(..), setTickyCtrLabel, getTickyCtrLabel, + withCurrentPrimOpName, getCurrentPrimOpName, tickScope, getTickScope, withUpdFrameOff, getUpdFrameOff, @@ -304,6 +305,9 @@ data FCodeState = -- See Note [Self-recursive tail calls] in GHC.StgToCmm.Expr , fcs_ticky :: !CLabel -- ^ Destination for ticky counts , fcs_tickscope :: !CmmTickScope -- ^ Tick scope for new blocks & ticks + , fcs_prim_op :: Maybe String -- ^ Source name of the primop currently being + -- compiled, used by -fcheck-prim-bounds error + -- messages. } data HeapUsage -- See Note [Virtual and real heap pointers] @@ -462,6 +466,7 @@ initFCodeState p = , fcs_selfloop = Nothing , fcs_ticky = mkTopTickyCtrLabel , fcs_tickscope = GlobalScope + , fcs_prim_op = Nothing } getFCodeState :: FCode FCodeState @@ -520,6 +525,20 @@ setTickyCtrLabel ticky code = do fstate <- getFCodeState withFCodeState code (fstate {fcs_ticky = ticky}) +-- ---------------------------------------------------------------------------- +-- Track the primop currently being compiled + +-- | The source name of the primop currently being compiled (e.g. +-- @"writeArray#"@), if any. Used to produce informative @-fcheck-prim-bounds@ +-- error messages. +getCurrentPrimOpName :: FCode (Maybe String) +getCurrentPrimOpName = fcs_prim_op <$> getFCodeState + +withCurrentPrimOpName :: String -> FCode a -> FCode a +withCurrentPrimOpName name code = do + fstate <- getFCodeState + withFCodeState code (fstate {fcs_prim_op = Just name}) + -- ---------------------------------------------------------------------------- -- Manage tick scopes ===================================== compiler/GHC/StgToCmm/Prim.hs ===================================== @@ -25,11 +25,13 @@ import GHC.StgToCmm.Layout import GHC.StgToCmm.Foreign import GHC.StgToCmm.Monad import GHC.StgToCmm.Utils +import GHC.StgToCmm.Lit ( newStringCLit ) import GHC.StgToCmm.Ticky import GHC.StgToCmm.Heap import GHC.StgToCmm.Prof ( costCentreFrom ) import GHC.Types.Basic +import GHC.Types.Name.Occurrence ( occNameString ) import GHC.Types.Literal.Floating import GHC.Cmm.BlockId import GHC.Cmm.Graph @@ -94,7 +96,13 @@ cmmPrimOpApp cfg primop cmm_args mres_ty = do -- if the result type isn't explicitly given, we directly use the -- result type of the primop. res_ty = fromMaybe (primOpResultType primop) mres_ty - f res_ty + -- When -fcheck-prim-bounds is on, record the primop name so that any + -- bounds-check failure handler emitted while compiling it can name it in the + -- error message. Guarded by the flag so that the common (unchecked) case pays + -- nothing: no name thunk, no FCodeState update. + if stgToCmmDoBoundsCheck cfg + then withCurrentPrimOpName (occNameString (primOpOcc primop)) (f res_ty) + else f res_ty externalPrimop :: PrimOp -> [CmmExpr] -> PrimopCmmEmit externalPrimop primop args = outOfLinePrimop (callExternalPrimop primop args) @@ -3615,8 +3623,12 @@ emitCheckedMemcpyCall dst src n align = do emitMemcpyCall dst src n align where doCheck platform = do + name <- fromMaybe "<unknown primop>" <$> getCurrentPrimOpName + nameLbl <- newStringCLit name overlapCheckFailed <- getCode $ - emitCCallNeverReturns [] (mkLblExpr mkMemcpyRangeOverlapLabel) [] + emitCCallNeverReturns [] + (mkLblExpr mkMemcpyRangeOverlapLabel) + [ (CmmLit nameLbl, AddrHint) ] emit =<< mkCmmIfThen' rangesOverlap overlapCheckFailed (Just False) where rangesOverlap = (checkDiff dst src `or` checkDiff src dst) `ne` zero @@ -3728,6 +3740,25 @@ whenCheckBounds a = do False -> pure () True -> a +-- | Emit a call to the RTS @rtsOutOfBoundsAccess@ bounds-check failure +-- handler, passing the offending index, the number of accessed elements and +-- the array size, so that the runtime can produce an informative error +-- message. The name of the offending primop is read from the code generator +-- environment (see 'withCurrentPrimOpName'). +emitBoundsCheckFailed :: CmmExpr -- ^ accessed index + -> CmmExpr -- ^ number of accessed elements + -> CmmExpr -- ^ array size (in elements) + -> FCode () +emitBoundsCheckFailed idx count sz = do + name <- fromMaybe "<unknown primop>" <$> getCurrentPrimOpName + nameLbl <- newStringCLit name + emitCCallNeverReturns [] + (mkLblExpr mkOutOfBoundsAccessLabel) + [ (idx, NoHint) + , (count, NoHint) + , (sz, NoHint) + , (CmmLit nameLbl, AddrHint) ] + emitBoundsCheck :: CmmExpr -- ^ accessed index -> CmmExpr -- ^ array size (in elements) -> FCode () @@ -3735,7 +3766,7 @@ emitBoundsCheck idx sz = do assertM (stgToCmmDoBoundsCheck <$> getStgToCmmConfig) platform <- getPlatform boundsCheckFailed <- getCode $ - emitCCallNeverReturns [] (mkLblExpr mkOutOfBoundsAccessLabel) [] + emitBoundsCheckFailed idx (mkIntExpr platform 1) sz let isOutOfBounds = cmmUGeWord platform idx sz emit =<< mkCmmIfThen' isOutOfBounds boundsCheckFailed (Just False) @@ -3754,7 +3785,7 @@ emitRangeBoundsCheck idx len arrSizeExpr = do _ <- withSequel (AssignTo [lastSafeIndexReg, rangeTooLargeReg] False) $ cmmPrimOpApp config WordSubCOp [arrSize, len] Nothing boundsCheckFailed <- getCode $ - emitCCallNeverReturns [] (mkLblExpr mkOutOfBoundsAccessLabel) [] + emitBoundsCheckFailed idx len arrSize let rangeTooLarge = CmmReg (CmmLocal rangeTooLargeReg) lastSafeIndex = CmmReg (CmmLocal lastSafeIndexReg) ===================================== docs/users_guide/debugging.rst ===================================== @@ -1167,10 +1167,25 @@ Checking for consistency Typically primops operations like ``writeArray#`` exhibit unsafe behavior, relying on the user to perform any bounds checking. This flag instructs the code generator to instrument such operations with bound checking logic - which aborts the program when an out-of-bounds access is detected. + which terminates the program when an out-of-bounds access is detected. + + When a check fails, the program prints a message naming the offending + primop together with the offending index and the array size, and exits + with a non-zero status, for example:: + + myprog: writeArray#: array access out of bounds: + index 5 is not within [0, 3). + This is usually caused by incorrect use of unsafe primops in user or library code. + + The message does not identify the enclosing function or module. To obtain a + backtrace pinpointing the failing call, build with profiling and run with + ``+RTS -xc`` (see :ref:`prof-time-options`), or use :ghc-flag:`-finfo-table-map` + together with ``+RTS -xc``. Note that this is only intended to be used as a debugging measure, not as - the primary means of catching out-of-bounds accesses. + the primary means of catching out-of-bounds accesses. Currently only the + native code generator is instrumented; the JavaScript backend is unaffected + by this flag. .. ghc-flag:: -fcmm-thread-sanitizer :shortdesc: Enable ThreadSanitizer instrumentation of memory accesses. ===================================== rts/PrimOps.cmm ===================================== @@ -88,10 +88,14 @@ import CLOSURE ghc_hs_iface; #endif #if defined(DEBUG) -#define ASSERT_IN_BOUNDS(ind, sz) \ - if (ind >= sz) { ccall rtsOutOfBoundsAccess(); } +// `op` is the source name of the primop being checked (e.g. "casIntArray#"). +// NB: in some callers `ind` is a byte offset rather than an element index, so +// the index reported here may be in bytes. count is 1 since these checks cover +// a single access. +#define ASSERT_IN_BOUNDS(op, ind, sz) \ + if (ind >= sz) { ccall rtsOutOfBoundsAccess(ind, 1, sz, op); } #else -#define ASSERT_IN_BOUNDS(ind, sz) +#define ASSERT_IN_BOUNDS(op, ind, sz) #endif /*----------------------------------------------------------------------------- @@ -336,7 +340,7 @@ stg_casIntArrayzh( gcptr arr, W_ ind, W_ old, W_ new ) { W_ p, h; - ASSERT_IN_BOUNDS(ind + WDS(1) - 1, StgArrBytes_bytes(arr)); + ASSERT_IN_BOUNDS("casIntArray#", ind + WDS(1) - 1, StgArrBytes_bytes(arr)); p = arr + SIZEOF_StgArrBytes + WDS(ind); (h) = prim %cmpxchgW(p, old, new); @@ -350,7 +354,7 @@ stg_casInt8Arrayzh( gcptr arr, W_ ind, I8 old, I8 new ) W_ p; I8 h; - ASSERT_IN_BOUNDS(ind, StgArrBytes_bytes(arr)); + ASSERT_IN_BOUNDS("casInt8Array#", ind, StgArrBytes_bytes(arr)); p = arr + SIZEOF_StgArrBytes + ind; (h) = prim %cmpxchg8(p, old, new); @@ -364,7 +368,7 @@ stg_casInt16Arrayzh( gcptr arr, W_ ind, I16 old, I16 new ) W_ p; I16 h; - ASSERT_IN_BOUNDS(ind + 1, StgArrBytes_bytes(arr)); + ASSERT_IN_BOUNDS("casInt16Array#", ind + 1, StgArrBytes_bytes(arr)); p = arr + SIZEOF_StgArrBytes + ind*2; (h) = prim %cmpxchg16(p, old, new); @@ -378,7 +382,7 @@ stg_casInt32Arrayzh( gcptr arr, W_ ind, I32 old, I32 new ) W_ p; I32 h; - ASSERT_IN_BOUNDS(ind + 3, StgArrBytes_bytes(arr)); + ASSERT_IN_BOUNDS("casInt32Array#", ind + 3, StgArrBytes_bytes(arr)); p = arr + SIZEOF_StgArrBytes + ind*4; (h) = prim %cmpxchg32(p, old, new); @@ -392,7 +396,7 @@ stg_casInt64Arrayzh( gcptr arr, W_ ind, I64 old, I64 new ) W_ p; I64 h; - ASSERT_IN_BOUNDS(ind + 7, StgArrBytes_bytes(arr)); + ASSERT_IN_BOUNDS("casInt64Array#", ind + 7, StgArrBytes_bytes(arr)); p = arr + SIZEOF_StgArrBytes + ind*8; (h) = prim %cmpxchg64(p, old, new); @@ -470,7 +474,7 @@ stg_casArrayzh ( gcptr arr, W_ ind, gcptr old, gcptr new ) gcptr h; W_ p, len; - ASSERT_IN_BOUNDS(ind, StgMutArrPtrs_ptrs(arr)); + ASSERT_IN_BOUNDS("casArray#", ind, StgMutArrPtrs_ptrs(arr)); p = arr + SIZEOF_StgMutArrPtrs + WDS(ind); (h) = prim %cmpxchgW(p, old, new); @@ -578,8 +582,8 @@ stg_copySmallArrayzh ( gcptr src, W_ src_off, gcptr dst, W_ dst_off, W_ n) SET_INFO(dst, stg_SMALL_MUT_ARR_PTRS_DIRTY_info); - ASSERT_IN_BOUNDS(dst_off + n - 1, StgSmallMutArrPtrs_ptrs(dst)); - ASSERT_IN_BOUNDS(src_off + n - 1, StgSmallMutArrPtrs_ptrs(src)); + ASSERT_IN_BOUNDS("copySmallArray#", dst_off + n - 1, StgSmallMutArrPtrs_ptrs(dst)); + ASSERT_IN_BOUNDS("copySmallArray#", src_off + n - 1, StgSmallMutArrPtrs_ptrs(src)); dst_p = dst + SIZEOF_StgSmallMutArrPtrs + WDS(dst_off); src_p = src + SIZEOF_StgSmallMutArrPtrs + WDS(src_off); bytes = WDS(n); @@ -601,8 +605,8 @@ stg_copySmallMutableArrayzh ( gcptr src, W_ src_off, gcptr dst, W_ dst_off, W_ n SET_INFO(dst, stg_SMALL_MUT_ARR_PTRS_DIRTY_info); - ASSERT_IN_BOUNDS(dst_off + n - 1, StgSmallMutArrPtrs_ptrs(dst)); - ASSERT_IN_BOUNDS(src_off + n - 1, StgSmallMutArrPtrs_ptrs(src)); + ASSERT_IN_BOUNDS("copySmallMutableArray#", dst_off + n - 1, StgSmallMutArrPtrs_ptrs(dst)); + ASSERT_IN_BOUNDS("copySmallMutableArray#", src_off + n - 1, StgSmallMutArrPtrs_ptrs(src)); dst_p = dst + SIZEOF_StgSmallMutArrPtrs + WDS(dst_off); src_p = src + SIZEOF_StgSmallMutArrPtrs + WDS(src_off); bytes = WDS(n); @@ -623,7 +627,7 @@ stg_casSmallArrayzh ( gcptr arr, W_ ind, gcptr old, gcptr new ) gcptr h; W_ p, len; - ASSERT_IN_BOUNDS(ind, StgSmallMutArrPtrs_ptrs(arr)); + ASSERT_IN_BOUNDS("casSmallArray#", ind, StgSmallMutArrPtrs_ptrs(arr)); p = arr + SIZEOF_StgSmallMutArrPtrs + WDS(ind); (h) = prim %cmpxchgW(p, old, new); ===================================== rts/RtsMessages.c ===================================== @@ -352,13 +352,32 @@ rtsBadAlignmentBarf(void) } void -rtsOutOfBoundsAccess(void) +rtsOutOfBoundsAccess(StgInt index, StgWord count, StgWord size, const char *op) { - barf("Encountered out of bounds array access."); + if (count <= 1) { + errorBelch("%s: array access out of bounds:\n" + " index %" FMT_Int " is not within [0, %" FMT_Word ").\n" + "This is usually caused by incorrect use of unsafe primops " + "in user or library code.", + op, index, size); + } else { + errorBelch("%s: array access out of bounds:\n" + " range of %" FMT_Word " elements starting at index %" FMT_Int + " is not within [0, %" FMT_Word ").\n" + "This is usually caused by incorrect use of unsafe primops " + "in user or library code.", + op, count, index, size); + } + stg_exit(EXIT_FAILURE); } void -rtsMemcpyRangeOverlap(void) +rtsMemcpyRangeOverlap(const char *op) { - barf("Encountered overlapping source/destination ranges in a memcpy-using op."); + errorBelch("%s: overlapping source and destination ranges in a " + "memcpy-using operation.\n" + "This is usually caused by incorrect use of unsafe primops " + "in user or library code.", + op); + stg_exit(EXIT_FAILURE); } ===================================== rts/include/rts/Messages.h ===================================== @@ -108,5 +108,5 @@ extern RtsMsgFunction rtsSysErrorMsgFn; /* Used by code generator */ void rtsBadAlignmentBarf(void) STG_NORETURN; -void rtsOutOfBoundsAccess(void) STG_NORETURN; -void rtsMemcpyRangeOverlap(void) STG_NORETURN; +void rtsOutOfBoundsAccess(StgInt index, StgWord count, StgWord size, const char *op) STG_NORETURN; +void rtsMemcpyRangeOverlap(const char *op) STG_NORETURN; ===================================== testsuite/tests/codeGen/should_fail/T26964.hs ===================================== @@ -0,0 +1,17 @@ +{-# LANGUAGE UnboxedTuples #-} +{-# LANGUAGE MagicHash #-} + +-- Test that -fcheck-prim-bounds reports the failing primop, the offending +-- index and the array size. The negative index also checks that it is reported +-- as a signed number (e.g. -1, not a huge unsigned word). + +module Main where + +import GHC.Exts +import GHC.IO + +main :: IO () +main = do + IO $ \s0 -> + case newSmallArray# 5# () s0 of + (# s1, marr #) -> readSmallArray# marr (-1#) s1 ===================================== testsuite/tests/codeGen/should_fail/T26964.stderr ===================================== @@ -0,0 +1,3 @@ +readSmallArray#: array access out of bounds: + index -1 is not within [0, 5). +This is usually caused by incorrect use of unsafe primops in user or library code. ===================================== testsuite/tests/codeGen/should_fail/T26964b.hs ===================================== @@ -0,0 +1,20 @@ +{-# LANGUAGE UnboxedTuples #-} +{-# LANGUAGE MagicHash #-} + +-- Test that -fcheck-prim-bounds reports a failing *range* access (count > 1) +-- with the "range of N elements starting at index" wording. The source range +-- is in bounds, but the destination range [6, 10) overruns the 8-byte array. + +module Main where + +import GHC.Exts +import GHC.IO + +main :: IO () +main = IO $ \s0 -> + case newByteArray# 8# s0 of + (# s1, src #) -> + case newByteArray# 8# s1 of + (# s2, dst #) -> + case copyMutableByteArray# src 0# dst 6# 4# s2 of + s3 -> (# s3, () #) ===================================== testsuite/tests/codeGen/should_fail/T26964b.stderr ===================================== @@ -0,0 +1,3 @@ +copyMutableByteArray#: array access out of bounds: + range of 4 elements starting at index 6 is not within [0, 8). +This is usually caused by incorrect use of unsafe primops in user or library code. ===================================== testsuite/tests/codeGen/should_fail/all.T ===================================== @@ -6,8 +6,10 @@ test('T8131', [cmm_src, only_ways(llvm_ways)], compile_fail, ['-no-hs-main']) def check_bounds_test(name): """ A -fcheck-prim-bounds test that is expected to fail. """ + # The native backend exits with EXIT_FAILURE (1); the JS backend still + # aborts with h$exitProcess(134) (the improved messages are native-only). test(name, - [ignore_stderr, omit_ghci, exit_code(127 if opsys('mingw32') else 134)], + [ignore_stderr, omit_ghci, exit_code(134 if js_arch() else 1)], compile_and_run, ['-fcheck-prim-bounds']) check_bounds_test('CheckBoundsWriteArray') # Check past end @@ -25,3 +27,18 @@ check_bounds_test('CheckBoundsCompareByteArray3') # Check negative length check_bounds_test('CheckOverlapCopyByteArray') check_bounds_test('CheckOverlapCopyAddrToByteArray') check_bounds_test('T26958') + +# Unlike the check_bounds_test cases above, these tests pin down the exact +# -fcheck-prim-bounds error message. Drop the leading "<prog>: " prefix so the +# tests don't depend on the binary's name/path (incl. Windows drive letters). +def strip_prog_prefix(s): + return re.sub(r'(?m)^.*?(\w+#: array access)', r'\1', s) + +# T26964 checks the single-element message. T26964b checks the range variant. +# Skipped on the JS backend, which emits no message and aborts with code 134. +test('T26964', + [omit_ghci, js_skip, exit_code(1), normalise_errmsg_fun(strip_prog_prefix)], + compile_and_run, ['-fcheck-prim-bounds']) +test('T26964b', + [omit_ghci, js_skip, exit_code(1), normalise_errmsg_fun(strip_prog_prefix)], + compile_and_run, ['-fcheck-prim-bounds']) View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/faeb0540a61b6f4e3fa1210ae364f48... -- View it on GitLab: https://gitlab.haskell.org/ghc/ghc/-/compare/faeb0540a61b6f4e3fa1210ae364f48... You're receiving this email because of your account on gitlab.haskell.org.