Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't panic in AssumptionStack.popFrame #1168

Open
langston-barrett opened this issue Jan 24, 2024 · 1 comment
Open

Don't panic in AssumptionStack.popFrame #1168

langston-barrett opened this issue Jan 24, 2024 · 1 comment
Assignees
Labels

Comments

@langston-barrett
Copy link
Contributor

Consider the following program, written against Crucible's public API:

module Main where

import Prelude
import Data.Parameterized.Nonce
import Data.Parameterized.Some
import Lang.Crucible.Backend.AssumptionStack

main :: IO ()
main = do
  Some gen <- newIONonceGenerator
  stk <- initAssumptionStack gen
  frameId <- pushFrame stk
  _frameId <- pushFrame stk
  _result <- popFrame @() frameId stk
  putStrLn "No panic?"

This program panics:

%< ---------------------------------------------------
  Revision:  bb50c0d16922e13d6358ce9abc7042ed13feccab
  Branch:    master
  Location:  AssumptionStack.popFrame
  Message:   Push/pop mismatch in assumption stack!
             *** Current frame:  0
             *** Expected ident: 1
CallStack (from HasCallStack):
  panic, called at src/Lang/Crucible/Panic.hs:11:9 in crucible-0.6-inplace:Lang.Crucible.Panic
  panic, called at src/Lang/Crucible/Backend/AssumptionStack.hs:204:16 in crucible-0.6-inplace:Lang.Crucible.Backend.AssumptionStack
%< ---------------------------------------------------

I would argue that I have not found a bug in Crucible. More specifically, the only bug here is that this function calls panic when passed invalid arguments, but is exposed via a public API where such arguments may, in fact, be passed. This panic (and others like it) should either be reified in the return type (e.g., Either), or turned into a dedicated Exception instance (which should be noted in the Haddocks). Panics should be reserved for situations that are considered by Crucible to be literally impossible.

@RyanGlScott
Copy link
Contributor

I agree. It would nice to be able to recover more gracefully in situations like this, so this change would be welcome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants