Skip to content

Commit

Permalink
w4_abc_{aiger,verilog}: Handle variable-less properties correctly
Browse files Browse the repository at this point in the history
Previously, any proofs involving the `w4_abc_aiger` (a.k.a., `abc`) or
`w4_abc_verilog` proof scripts would succeed if they did not involve any
variables, even false properties (e.g., `False`). This happened for a very
silly reason: the counterexamples that the `abc` would generate contained a
blank output (since there are no variables to describe), and SAW was
misinterpreting this as a successful proof. Oops!

With this patch, SAW now properly distinguishes between an successful proof (in
which case no counterexample file will be generated) and a unsuccessful proof
involving no variables (in which case a blank counterexample file will be
generated). This is admittedly a bit fiddly, as it requires making some
assumptions about the format of the counterexample files that `abc` produces.
Nevertheless, this does work on all the examples that I have tried.

Fixes #1938.
  • Loading branch information
RyanGlScott committed Sep 19, 2023
1 parent 9b71965 commit 8f73198
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 19 deletions.
11 changes: 11 additions & 0 deletions intTests/test1938/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
CC = clang
CFLAGS = -g -frecord-command-line -O0

all: test.bc

test.bc: test.c
$(CC) $(CFLAGS) -c -emit-llvm $< -o $@

.PHONY: clean
clean:
rm -f test.bc
Binary file added intTests/test1938/test.bc
Binary file not shown.
3 changes: 3 additions & 0 deletions intTests/test1938/test.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
void test(int *x) {
*x = 2;
}
18 changes: 18 additions & 0 deletions intTests/test1938/test.saw
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
// Test that the `abc` and `w4_abc_verilog` proof scripts will report invalid
// goals that do not contain any variables. We test this both directly (with the
// trivial `False` property) as well as via an LLVM verification involving a
// proof goal that evaluates to `False`.

fails (prove_print abc {{ False }});
fails (prove_print w4_abc_verilog {{ False }});

m <- llvm_load_module "test.bc";

// This will generate a failing proof goal about `x` being read-only.
let setup = do {
x <- llvm_alloc_readonly (llvm_int 32);
llvm_execute_func [x];
};

fails (llvm_verify m "test" [] true setup abc);
fails (llvm_verify m "test" [] true setup w4_abc_verilog);
3 changes: 3 additions & 0 deletions intTests/test1938/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
set -e

$SAW test.saw
45 changes: 26 additions & 19 deletions src/SAWScript/Prover/ABC.hs
Original file line number Diff line number Diff line change
Expand Up @@ -8,16 +8,16 @@ module SAWScript.Prover.ABC
import Control.Monad (unless)
import Control.Monad.IO.Class
import qualified Data.ByteString.Char8 as C8
import Data.Char (isSpace)
import Data.List (isPrefixOf)
import qualified Data.Map as Map
import Data.Maybe
import qualified Data.Text as Text
import Text.Read (readMaybe)

import System.Directory
import System.FilePath ((</>))
import System.IO
import System.IO.Temp (emptySystemTempFile)
import System.IO.Temp (createTempDirectory, getCanonicalTemporaryDirectory)
import System.Process (readProcessWithExitCode)

import qualified Data.AIG as AIG
Expand Down Expand Up @@ -105,30 +105,33 @@ w4AbcExternal ::
SATQuery ->
TopLevel (Maybe CEX, String)
w4AbcExternal exporter argFn _hashcons satq =
-- Create temporary files
do let tpl = "abc_verilog.v"
tplCex = "abc_verilog.cex"
tmp <- liftIO $ emptySystemTempFile tpl
tmpCex <- liftIO $ emptySystemTempFile tplCex
-- Create a temporary directory to put input and output files
do canonTmpDir <- liftIO getCanonicalTemporaryDirectory
tmpDir <- liftIO $ createTempDirectory canonTmpDir "saw_abc_external"
let tmp = tmpDir </> "abc_verilog.v"
tmpCex = tmpDir </> "abc_verilog.cex"

(argNames, argTys) <- unzip <$> exporter tmp satq

-- Run ABC and remove temporaries
let execName = "abc"
args = ["-q", argFn tmp tmpCex]
(_out, _err) <- liftIO $ readProcessExitIfFailure execName args
cexText <- liftIO $ C8.unpack <$> C8.readFile tmpCex
liftIO $ removeFile tmp
liftIO $ removeFile tmpCex

-- Parse and report results
res <- if all isSpace cexText
then return Nothing
else do cex <- liftIO $ parseAigerCex cexText argTys
case cex of
Left parseErr -> fail parseErr
Right vs -> return $ Just model
where model = zip argNames (map toFirstOrderValue vs)
cexExists <- liftIO $ doesFileExist tmpCex
res <-
if cexExists
then do
-- If a counterexample file exists, parse and report the results.
cexText <- liftIO $ C8.unpack <$> C8.readFile tmpCex
cex <- liftIO $ parseAigerCex cexText argTys
case cex of
Left parseErr -> fail parseErr
Right vs -> return $ Just model
where model = zip argNames (map toFirstOrderValue vs)
else
-- Otherwise, there is no counterexample.
return Nothing
liftIO $ removeDirectoryRecursive tmpDir
return (res, "abc_verilog")

parseAigerCex :: String -> [FiniteType] -> IO (Either String [FiniteValue])
Expand All @@ -137,11 +140,15 @@ parseAigerCex text tys =
-- Output from `write_cex`
[cex] ->
case words cex of
-- Queries with no variables will result in a blank line of output.
[] -> pure $ liftCexBB tys []
[bits] -> liftCexBB tys <$> mapM bitToBool bits
_ -> fail $ "invalid counterexample line: " ++ cex
-- Output from `write_aiger_cex`
[_, cex] ->
case words cex of
-- Queries with no variables will result in an empty vector of output.
[_] -> pure $ liftCexBB tys []
[bits, _] -> liftLECexBB tys <$> mapM bitToBool bits
_ -> fail $ "invalid counterexample line: " ++ cex
_ -> fail $ "invalid counterexample text: " ++ text
Expand Down

0 comments on commit 8f73198

Please sign in to comment.