Skip to content

Commit 3fc349f

Browse files
committed
Fix a problem with pre-evaluation in the sandboxed runtime
Pre-evaluation tries to evaluate all top level values ahead of time. There are a few such values, though, that come from operations we have marked as sandboxed, like the standard handles. Documents and such would never actually use these handles, but they might be transitively referred to by the documents. Pre-evaluation was then eagerly evaluating the disallowed functions, even though the values aren't actually needed to calculate the document value. This commit just ignores sandboxing failures during pre-evaluation. They will cause there to be no stored result for the value, but as long as the document (or whatever else is being evaluated) doesn't _actually_ depend on the sandboxed value, it will evaluate fine.
1 parent c4149a9 commit 3fc349f

File tree

2 files changed

+82
-0
lines changed

2 files changed

+82
-0
lines changed

unison-runtime/src/Unison/Runtime/Machine.hs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import Control.Lens
3333
import Data.Atomics qualified as Atomic
3434
import Data.Bits
3535
import Data.Functor.Classes (Eq1 (..), Ord1 (..))
36+
import Data.List qualified as List
3637
import Data.IORef (IORef)
3738
import Data.IORef qualified as IORef
3839
import Data.Map.Strict qualified as M
@@ -2361,13 +2362,32 @@ preEvalTopLevelConstants cacheableCombs newCombs cc = do
23612362
atomically $ do
23622363
modifyTVar evaluatedCacheableCombsVar $ EC.mapInsert w (EC.mapSingleton 0 $ CachedVal w val)
23632364
apply0 (Just hook) cc activeThreads w
2365+
`catch` \e ->
2366+
-- ignore sandboxing exceptions during pre-eval, in case they
2367+
-- don't matter for the final result.
2368+
if isSandboxingException e
2369+
then pure ()
2370+
else throwIO e
23642371

23652372
evaluatedCacheableCombs <- readTVarIO evaluatedCacheableCombsVar
23662373
let allNew = evaluatedCacheableCombs <> newCombs
23672374
-- Rewrite all the inlined combinator references to point to the
23682375
-- new cached versions.
23692376
atomically $ modifyTVar (combs cc) (\existingCombs -> (resolveCombs (Just $ EC.mapDifference existingCombs allNew) allNew) <> existingCombs)
23702377

2378+
-- Checks if a runtime exception is due to sandboxing.
2379+
--
2380+
-- This is used above during pre-evaluation, to ignore sandboxing
2381+
-- exceptions for top-level constant dependencies of docs and such, in
2382+
-- case the docs don't actually evaluate them.
2383+
isSandboxingException :: RuntimeExn -> Bool
2384+
isSandboxingException (PE _ (P.toPlainUnbroken -> msg)) =
2385+
List.isPrefixOf sdbx1 msg || List.isPrefixOf sdbx2 msg
2386+
where
2387+
sdbx1 = "attempted to use sandboxed operation"
2388+
sdbx2 = "Attempted to use disallowed builtin in sandboxed"
2389+
isSandboxingException _ = False
2390+
23712391
expandSandbox ::
23722392
Map Reference (Set Reference) ->
23732393
[(Reference, SuperGroup Symbol)] ->
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
``` ucm :hide
2+
scratch/main> builtins.mergeio
3+
```
4+
5+
``` unison
6+
7+
stdOut = stdHandle StdOut
8+
9+
putText h t = match putBytes.impl h (Text.toUtf8 t) with
10+
Left e -> raise e
11+
_ -> ()
12+
13+
printLine t =
14+
putText stdOut t
15+
putText stdOut "\n"
16+
```
17+
18+
``` ucm :added-by-ucm
19+
Loading changes detected in scratch.u.
20+
21+
I found and typechecked these definitions in scratch.u. If you
22+
do an `add` or `update`, here's how your codebase would
23+
change:
24+
25+
⍟ These new definitions are ok to `add`:
26+
27+
printLine : Text ->{IO, Exception} ()
28+
putText : Handle -> Text ->{IO, Exception} ()
29+
stdOut : Handle
30+
```
31+
32+
``` ucm
33+
scratch/main> update
34+
35+
Okay, I'm searching the branch for code that needs to be
36+
updated...
37+
38+
Done.
39+
```
40+
41+
``` unison
42+
43+
hmmm = {{ I'll try {printLine}. That's a good trick. }}
44+
```
45+
46+
``` ucm :added-by-ucm
47+
Loading changes detected in scratch.u.
48+
49+
I found and typechecked these definitions in scratch.u. If you
50+
do an `add` or `update`, here's how your codebase would
51+
change:
52+
53+
⍟ These new definitions are ok to `add`:
54+
55+
hmmm : Doc2
56+
```
57+
58+
``` ucm
59+
scratch/main> display hmmm
60+
61+
I'll try printLine. That's a good trick.
62+
```

0 commit comments

Comments
 (0)