Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#13977 closed bug (fixed)

ExnStr doesn't propagate "outwards"

Reported by: bgamari Owned by:
Priority: normal Milestone: 8.2.1
Component: Compiler Version: 8.0.1
Keywords: Exceptions Cc: jmct
Operating System: Unknown/Multiple Architecture: Unknown/Multiple
Type of failure: None/Unknown Test Case:
Blocked By: Blocking:
Related Tickets: Differential Rev(s): Phab:D3756
Wiki Page:

Description (last modified by bgamari)

While looking into #8091 (see ticket:8091#comment:9) and #13916, I noticed something that I found rather surprising. Consider this program,

hello :: STM Char -> STM Char
hello a = a `orElse` pure 'a'

Recall that catchRetry# (which orElse is defined in terms of) has the following demand signature,

<xC(S),1*C1(U)><L,1*C1(U)><L,U>

Note the x here: this means it is ExnStr, since it doesn't bottom even if the first argument does (despite the fact that it's strict).

So, the question is this: What demand should hello place on its first argument? I would have thought that it should be precisely the same as the demand that catchRetry# places on its first argument. However, the demand analyser seems to conclude this:

<C(S),1*C1(U)>

Note the lack of an x. I believe this may be what causes #13916.

Change History (9)

comment:1 Changed 2 years ago by bgamari

I think I might see why the demand analyser drops the ExnStr. Consider postProcessDmdEnv,

postProcessDmdEnv :: DmdShell -> DmdEnv -> DmdEnv
postProcessDmdEnv ds@(JD { sd = ss, ud = us }) env
  | Abs <- us       = emptyDmdEnv
  | Str _ _   <- ss
  , Use One _ <- us = env  -- Shell is a no-op
  | otherwise       = mapVarEnv (postProcessDmd ds) env

When we get here while demand-analysing the catchRetry# @Char a application in hello we will have,

  • the "shell" sd = JD {sd = Str ExnStr (), ud = Use One ()} (as computed as defer_and_use by dmdAnalStar)
  • The environment is {aQq-><C(S),1*C1(U)>}.

Note that the shell (ds) has an ExnStr which gets eaten by the Str _ _ <- ss case of postProcessDmdEnv, meaning we don't transform the environment. I haven't yet done enough reading to know whether this is correct but is seems suspicious.

Last edited 2 years ago by bgamari (previous) (diff)

comment:2 Changed 2 years ago by bgamari

Description: modified (diff)

comment:3 Changed 2 years ago by bgamari

Description: modified (diff)

comment:4 Changed 2 years ago by jmct

Cc: jmct added

comment:5 Changed 2 years ago by bgamari

  • compiler/basicTypes/Demand.hs

    diff --git a/compiler/basicTypes/Demand.hs b/compiler/basicTypes/Demand.hs
    index 95c7b79b4f..d777da63f0 100644
    a b postProcessDmdResult _ res = res 
    14421442postProcessDmdEnv :: DmdShell -> DmdEnv -> DmdEnv
    14431443postProcessDmdEnv ds@(JD { sd = ss, ud = us }) env
    14441444  | Abs <- us       = emptyDmdEnv
    1445   | Str _ _   <- ss
     1445  | Str VanStr _   <- ss   -- Make sure we don't throw about ExnStrs.
     1446                           -- See #13977.
    14461447  , Use One _ <- us = env  -- Shell is a no-op
    14471448  | otherwise       = mapVarEnv (postProcessDmd ds) env
    14481449  -- For the Absent case just discard all usage information

comment:6 Changed 2 years ago by bgamari

Differential Rev(s): Phab:D3756
Status: newpatch

comment:7 Changed 2 years ago by Ben Gamari <ben@…>

In c940e3b/ghc:

dmdAnal: Ensure that ExnStr flag isn't dropped inappropriately

This fixes #13977 and consequently #13615. Here an optimization in the
demand analyser was too liberal, causing us to drop the ExnStr flag and
consequently resulting in incorrect demand signatures. This manifested
as a segmentation fault in #13615 as we incorrectly concluded that an
application of catchRetry# would bottom.

Specifically, we had

    orElse' :: STM a -> STM a -> STM a
    orElse' x = catchRetry# x y
      where y = {- some action -}

Where the catchRetry# primop places a demand of <xC(S),1*C1(U)> on its
first argument. However, due to #13977 the demand analyser would assign
a demand of <C(S),1*C1(U)> on the first argument of orElse'. Note the
missing `x`.

    case orElse' bottomingAction anotherAction of { x -> Just x }

being transformed to,

    case orElse' bottomingAction anotherAction of {}

by the simplifier. This would naturally blow up when orElse' returned at
runtime, causing the segmentation fault described in #13615.

Test Plan: Validate, perhaps add a testcase

Reviewers: austin, simonpj

Reviewed By: simonpj

Subscribers: rwbarton, thomie

GHC Trac Issues: #13977, #13615

Differential Revision: https://phabricator.haskell.org/D3756

comment:8 Changed 2 years ago by bgamari

Milestone: 8.2.1
Resolution: fixed
Status: patchclosed

comment:9 Changed 2 years ago by bgamari

Keywords: Exceptions added

I believe this fix may bear on a number of tickets which have come up in the past year or so,

  • #13380: raiseIO# result looks wrong
  • #13330: forkIO has inconsistent behavior under optimization
  • #11555: catch _|_ breaks at -O1
  • #11222: Teach strictness analysis about catch-like operations
  • #13357: Check demand signatures for catchRetry# and catchSTM#
Note: See TracTickets for help on using tickets.