Zubin pushed to branch wip/25924 at Glasgow Haskell Compiler / GHC
Commits:
-
56d39f72
by Zubin Duggal at 2026-06-30T18:02:12+05:30
5 changed files:
- changelog.d/fix-absent-dict-projection
- compiler/GHC/Core/Make.hs
- compiler/GHC/Core/Opt/Specialise.hs
- compiler/GHC/Core/Opt/WorkWrap/Utils.hs
- compiler/GHC/Types/Literal.hs
Changes:
| 1 | 1 | section: compiler
|
| 2 | -synopsis: Fix a CorePrep miscompilation that could project a field out of an absent dictionary, resulting in a segfault.
|
|
| 2 | +synopsis: Fix a miscompilation that could project a field out of an absent dictionary, resulting in a segfault.
|
|
| 3 | 3 | issues: #25924
|
| 4 | 4 | mrs: !16219
|
| 5 | -description: We no longer speculatively evaluate bindings that we have already discovered are absent. |
|
| 5 | +description:
|
|
| 6 | + We no longer make an absent filler (a rubbish literal or error thunk) for an
|
|
| 7 | + absent dictionary or other predicate. We also no longer speculatively evaluate
|
|
| 8 | + a binding once we have discovered that it is absent. |
| ... | ... | @@ -66,7 +66,7 @@ import GHC.Types.Unique.Supply |
| 66 | 66 | import GHC.Core
|
| 67 | 67 | import GHC.Core.Utils ( exprType, mkSingleAltCase, bindNonRec, mkCast, mkTick )
|
| 68 | 68 | import GHC.Core.Type
|
| 69 | -import GHC.Core.Predicate ( scopedSort, isEqPred )
|
|
| 69 | +import GHC.Core.Predicate ( scopedSort, isPredTy )
|
|
| 70 | 70 | import GHC.Core.TyCo.Compare ( eqType )
|
| 71 | 71 | import GHC.Core.Coercion ( isCoVar, mkRepReflCo, mkForAllVisCos )
|
| 72 | 72 | import GHC.Core.DataCon ( DataCon, dataConWorkId, dataConWrapId )
|
| ... | ... | @@ -226,13 +226,15 @@ mkLitRubbish :: Type -> Maybe CoreExpr |
| 226 | 226 | -- Make a rubbish-literal CoreExpr of the given type.
|
| 227 | 227 | -- Fail (returning Nothing) if
|
| 228 | 228 | -- * the RuntimeRep of the Type is not monomorphic;
|
| 229 | --- * the type is (a ~# b), the type of coercion
|
|
| 229 | +-- * the type is a predicate (isPredTy). This used to apply only to
|
|
| 230 | +-- equalities (a ~# b) but was expanded to all predicates. See
|
|
| 231 | +-- Note [Don't make fillers for predicates] in GHC.Core.Opt.WorkWrap.Utils
|
|
| 230 | 232 | -- See INVARIANT 1 and 2 of item (2) in Note [Rubbish literals]
|
| 231 | 233 | -- in GHC.Types.Literal
|
| 232 | 234 | mkLitRubbish ty
|
| 233 | 235 | | not (noFreeVarsOfType rep)
|
| 234 | 236 | = Nothing -- Satisfy INVARIANT 1
|
| 235 | - | isEqPred ty
|
|
| 237 | + | isPredTy ty
|
|
| 236 | 238 | = Nothing -- Satisfy INVARIANT 2
|
| 237 | 239 | | otherwise
|
| 238 | 240 | = Just (Lit (LitRubbish torc rep) `mkTyApps` [ty])
|
| ... | ... | @@ -2639,6 +2639,10 @@ specHeader subst (bndr:bndrs) (_ : args) |
| 2639 | 2639 | | isDeadBinder bndr
|
| 2640 | 2640 | , let (subst1, bndr') = Core.substBndr subst (zapIdOccInfo bndr)
|
| 2641 | 2641 | , Just rubbish_lit <- mkLitRubbish (idType bndr')
|
| 2642 | + -- NB: mkLitRubbish returns Nothing for a predicate (e.g. a dictionary), so
|
|
| 2643 | + -- this guard fails and we fall through, keeping the argument instead of
|
|
| 2644 | + -- dropping it. See Note [Don't make fillers for predicates] in
|
|
| 2645 | + -- GHC.Core.Opt.WorkWrap.Utils
|
|
| 2642 | 2646 | = -- See Note [Drop dead args from specialisations]
|
| 2643 | 2647 | do { (useful, subst2, rule_bs, rule_es, spec_bs, dx, spec_args) <- specHeader subst1 bndrs args
|
| 2644 | 2648 | ; pure ( useful, subst2
|
| ... | ... | @@ -30,7 +30,7 @@ import GHC.Core.Subst |
| 30 | 30 | import GHC.Core.Type
|
| 31 | 31 | import GHC.Core.Multiplicity
|
| 32 | 32 | import GHC.Core.Coercion
|
| 33 | -import GHC.Core.Predicate( isDictTy )
|
|
| 33 | +import GHC.Core.Predicate( isPredTy )
|
|
| 34 | 34 | import GHC.Core.Reduction
|
| 35 | 35 | import GHC.Core.FamInstEnv
|
| 36 | 36 | import GHC.Core.Predicate( isEqualityClass )
|
| ... | ... | @@ -1074,7 +1074,7 @@ mkAbsentFiller opts arg str |
| 1074 | 1074 | -- if we are wrong (like we were in #11126). Otherwise we fall through to the
|
| 1075 | 1075 | -- less-desirable mkLitRubbish case.
|
| 1076 | 1076 | | mightBeLiftedType arg_ty
|
| 1077 | - , not (isDictTy arg_ty) -- See (AF4) in Note [Absent fillers]
|
|
| 1077 | + , not (isPredTy arg_ty) -- See (AF4) in Note [Absent fillers]
|
|
| 1078 | 1078 | , not (isStrictDmd (idDemandInfo arg)) -- See (AF2)
|
| 1079 | 1079 | , not (isMarkedStrict str) -- in Note [Absent fillers]
|
| 1080 | 1080 | = Just (mkAbsentErrorApp arg_ty msg)
|
| ... | ... | @@ -1311,9 +1311,14 @@ Needless to say, there are some wrinkles: |
| 1311 | 1311 | dictionary is non-bottom. (If -XDictsStrict is on, the invariant is even
|
| 1312 | 1312 | more important.)
|
| 1313 | 1313 | |
| 1314 | - Simple solution: never use an error thunk for a dictionary; instead fall
|
|
| 1315 | - through to mkRubbishLit. (The only downside is that we lose the compiler
|
|
| 1316 | - debugging advantages of (AF1).)
|
|
| 1314 | + We used to fix this by never using an error thunk for a dictionary, and
|
|
| 1315 | + falling through to mkLitRubbish instead. But that is not enough either.
|
|
| 1316 | + #25924 shows that selecting a superclass out of a rubbish dictionary also
|
|
| 1317 | + segfaults. So now we make no filler at all for a predicate. mkLitRubbish
|
|
| 1318 | + returns Nothing for predicates, and the error thunk branch (AF1) is guarded
|
|
| 1319 | + by `not (isPredTy arg_ty)`, so mkAbsentFiller returns Nothing and
|
|
| 1320 | + worker/wrapper keeps the real argument.
|
|
| 1321 | + See Note [Don't make fillers for predicates].
|
|
| 1317 | 1322 | |
| 1318 | 1323 | This is quite delicate.
|
| 1319 | 1324 | |
| ... | ... | @@ -1337,6 +1342,43 @@ fragile |
| 1337 | 1342 | because `MkT` is strict in its Int# argument, so we get an absentError
|
| 1338 | 1343 | exception when we shouldn't. Very annoying!
|
| 1339 | 1344 | |
| 1345 | +Note [Don't make fillers for predicates]
|
|
| 1346 | +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
| 1347 | +We never make an absent filler, error thunk or rubbish literal, for a predicate
|
|
| 1348 | +(isPredTy). That includes dictionaries, equalities, quantified constraints and
|
|
| 1349 | +implicit parameters.
|
|
| 1350 | + |
|
| 1351 | +GHC relies on a dictionary value never being bottom (see Note [NON-BOTTOM-DICTS
|
|
| 1352 | +invariant] in GHC.Core). Because of that, speculative evaluation is happy to
|
|
| 1353 | +evaluate a dictionary, and a superclass selection is happy to project a field
|
|
| 1354 | +out of it.
|
|
| 1355 | + |
|
| 1356 | +But an absent filler is the wrong thing to find in such a position. An error
|
|
| 1357 | +thunk is bottom, so if it is ever evaluated we get "Entered absent arg"
|
|
| 1358 | +(#24934). A rubbish literal is not a real dictionary. It is compiled to the unit
|
|
| 1359 | +value (), so if we project a superclass out of it we read a field that is not
|
|
| 1360 | +there and get a segfault (see #25924).
|
|
| 1361 | + |
|
| 1362 | +We used to paper over this. !13233 replaced the error thunk for an absent
|
|
| 1363 | +dictionary with a rubbish literal, so that it could at least be evaluated
|
|
| 1364 | +without complaint. But #25924 showed that this is not enough, because we do not
|
|
| 1365 | +only evaluate the absent dictionary, we also select a superclass from it.
|
|
| 1366 | + |
|
| 1367 | +We could instead teach speculation to leave absent bindings alone, and we do
|
|
| 1368 | +that too (see Note [Speculative evaluation] in GHC.CoreToStg.Prep). But that is
|
|
| 1369 | +not a guarantee. After optimisation a binding that holds an absent filler may no
|
|
| 1370 | +longer be marked absent, so we cannot rely on the demand to protect us.
|
|
| 1371 | + |
|
| 1372 | +So the safe thing is to make no filler at all for a predicate. Then there is no
|
|
| 1373 | +bogus dictionary to evaluate or project from. mkLitRubbish returns Nothing for
|
|
| 1374 | +any predicate (INVARIANT 2 of Note [Rubbish literals] in GHC.Types.Literal), and
|
|
| 1375 | +both of its callers cope with the Nothing. mkAbsentFiller returns Nothing, so
|
|
| 1376 | +worker/wrapper keeps the real argument. Specialise.specHeader likewise keeps the
|
|
| 1377 | +dead dictionary argument rather than dropping it for a rubbish literal.
|
|
| 1378 | + |
|
| 1379 | +We use isPredTy rather than isDictTy so that this also covers equalities,
|
|
| 1380 | +quantified constraints and implicit parameters, not just ordinary dictionaries.
|
|
| 1381 | + |
|
| 1340 | 1382 | Note [Unboxing through unboxed tuples]
|
| 1341 | 1383 | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
| 1342 | 1384 | We should not to a worker/wrapper split just for unboxing the components of
|
| ... | ... | @@ -994,15 +994,21 @@ data type. Here are the moving parts: |
| 994 | 994 | substitutions and free variable finders over Literal. The rules around
|
| 995 | 995 | levity/runtime-rep polymorphism naturally uphold this invariant.
|
| 996 | 996 | |
| 997 | - INVARIANT 2: we never make a rubbish literal of type (a ~# b). Reason:
|
|
| 998 | - see Note [Core type and coercion invariant] in GHC.Core. We can't substitute
|
|
| 999 | - a LitRubbish inside a coercion, so it's best not to make one. They are zero
|
|
| 1000 | - width anyway, so passing absent ones around costs nothing. If we wanted
|
|
| 1001 | - an absent filler of type (a ~# b) we should use (Coercion (UnivCo ...)),
|
|
| 1002 | - but it doesn't seem worth making a new UnivCoProvenance for this purpose.
|
|
| 997 | + INVARIANT 2: we never make a rubbish literal of a predicate type (isPredTy).
|
|
| 998 | + |
|
| 999 | + For an equality (a ~# b): see Note [Core type and coercion invariant] in
|
|
| 1000 | + GHC.Core. We can't substitute a LitRubbish inside a coercion, so it's best
|
|
| 1001 | + not to make one. They are zero width anyway, so passing absent ones around
|
|
| 1002 | + costs nothing. If we wanted an absent filler of type (a ~# b) we should use
|
|
| 1003 | + (Coercion (UnivCo ...)), but it doesn't seem worth making a new
|
|
| 1004 | + UnivCoProvenance for this purpose.
|
|
| 1003 | 1005 | |
| 1004 | 1006 | This is sad, though: see #18983.
|
| 1005 | 1007 | |
| 1008 | + For a dictionary: a rubbish literal is not a real dictionary, so evaluating
|
|
| 1009 | + it or selecting a field from it crashes. See Note [Don't make fillers for
|
|
| 1010 | + predicates] in GHC.Core.Opt.WorkWrap.Utils.
|
|
| 1011 | + |
|
| 1006 | 1012 | 3. STG: The type app in `RUBBISH[IntRep] @Int# :: Int#` is erased and we get
|
| 1007 | 1013 | the (untyped) 'StgLit' `RUBBISH[IntRep] :: Int#` in STG.
|
| 1008 | 1014 |