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

test0036_global is fragile due to undocumented assumptions #1155

Closed
RyanGlScott opened this issue Mar 26, 2021 · 4 comments · Fixed by #1287
Closed

test0036_global is fragile due to undocumented assumptions #1155

RyanGlScott opened this issue Mar 26, 2021 · 4 comments · Fixed by #1287
Labels
documentation Issues involving documentation subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm test assets Issues involving test programs or other test assets

Comments

@RyanGlScott
Copy link
Contributor

I was recently perplexed because I wrote a SAW spec that was almost identical to that of intTests/test0036_global, but while the spec in test0036_global verified successfully, mine was rejected. In fact, I discovered that test0036_global only seemed to work if used the test.bc file that was checked into the saw-script repo. If I recompiled test.bc like so:

clang-10 -g -c -emit-llvm test.bc

And then ran saw test.saw, it would fail thusly:

[13:50:32.161] Loading file "/home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw"
[13:50:32.182] Verifying f ...
[13:50:32.183] Simulating f ...
[13:50:32.184] Checking proof obligations f ...
[13:50:32.216] Subgoal failed: f safety assertion:
test.c:5:12: error: in f
Undefined behavior encountered
Details:
  Poison value created
  Signed addition caused wrapping even though the `nsw` flag was set

[13:50:32.216] SolverStats {solverStatsSolvers = fromList ["ABC"], solverStatsGoalSize = 66}
[13:50:32.216] ----------Counterexample----------
[13:50:32.216]   y: 2147483647
[13:50:32.216] ----------------------------------
[13:50:32.216] Stack trace:
"llvm_verify" (/home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:9:11-9:22):
Proof failed.

At first, I thought this was simply because the checked-in test.bc file was compiled with -fno-strict-overflow flag (this wasn't written down anywhere, so I couldn't be sure). To test this hypothesis, I again recompiled test.bc, this time like so:

clang-10 -g -c -emit-llvm -fno-strict-overflow test.bc

This time, however, saw test.saw failed for an entirely different reason:

[14:16:17.283] Loading file "/home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw"
[14:16:17.304] Verifying f ...
[14:16:17.304] Simulating f ...
[14:16:17.305] Checking proof obligations f ...
[14:16:17.331] Proof succeeded! f
[14:16:17.347] Verifying g ...
[14:16:17.348] Simulating g ...
[14:16:17.349] Checking proof obligations g ...
[14:16:17.350] Proof succeeded! g
[14:16:17.367] Verifying h ...
[14:16:17.367] Simulating h ...
[14:16:17.368] Registering overrides for `f`
[14:16:17.368]   variant `Symbol "f"`
[14:16:17.368] Registering overrides for `g`
[14:16:17.368]   variant `Symbol "g"`
[14:16:17.368] Matching 1 overrides of  f ...
[14:16:17.368] Branching on 1 override variants of f ...
[14:16:17.368] Applied override! f
[14:16:17.368] Matching 1 overrides of  g ...
[14:16:17.368] Stack trace:
"llvm_verify" (/home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:25:1-25:12):
Symbolic execution failed.
Abort due to false assumption:
  test.c:14:10: error: in h
  All overrides failed during structural matching:
  *  Name: g
     Location: /home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:16:11
     Argument types: 
     - i32
     Return type: i32
     Arguments:
     - symbolic integer:  width = 32
     at /home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:4:3
     error when loading through pointer that appeared in the override's points-to precondition(s):
     Precondition:
       Pointer: @x
       Pointee: global_initializer(x)

       Assertion made at: /home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:4:3
     Failure reason: 
       When reading through pointer: (5, 0x0:[64])
       in the  precondition of an override
       Tried to read something of size: 4
       And type: i32
       Found 1 possibly matching allocation(s):
       - GlobalAlloc 5 0x4:[64] Mutable 4-byte-aligned x
Stack frame h
  Allocations:
    StackAlloc 6 0x4:[64] Mutable 4-byte-aligned test.c:13:11
  Writes:
    Indexed chunk:
      5 |->
        invalidate (state of mutable global variable "x" (allocated at /home/rscott/Documents/Hacking/Haskell/signal-verification/saw-script/intTests/test0036_global/test.saw:4:3) not described in postcondition) (5, 0x0:[64]) 0x4:[64]
      6 |->   *(6, 0x0:[64]) := c@71:bv
Base memory
  Allocations:
    GlobalAlloc 5 0x4:[64] Mutable 4-byte-aligned x
    GlobalAlloc 4 0x0:[64] Immutable 1-byte-aligned [defined function ] h
    GlobalAlloc 3 0x0:[64] Immutable 1-byte-aligned [defined function ] g
    GlobalAlloc 2 0x0:[64] Immutable 1-byte-aligned [defined function ] f
    GlobalAlloc 1 0x0:[64] Immutable 1-byte-aligned [external function] llvm.dbg.declare
  Writes:
    Indexed chunk:
      5 |->   *(5, 0x0:[64]) := 0

And indeed, test.saw uses an llvm_points_to precondition on llvm_global "x" without a corresponding postcondition. So why does the checked-in version of test.bc not trigger this error? Some further debugging reveals that the optimization level is likely the culprit. If I compile test.bc using the -O2 flag like so:

clang-10 -g -c -emit-llvm -fno-strict-overflow -O2 test.bc

Then saw test.saw passes. But this is horribly delicate! If test.saw is relying on optimizations in order to succeed, then we should either:

  1. Repair test0036_global/test.saw so that it passes regardless of what combination of optimization flags and/or -fno-strict-overflow is used, or
  2. If the use of these flags are intended, we should document this assumption in test0036_global/README.

I'd personally favor (1) over (2), but I could be convinced of (2) with a persuasive argument. Regardless of which approach we go with, we should also update the SAW manual, which includes an example very similar to test0036_global/test.saw, to reflect our new understanding.

@RyanGlScott RyanGlScott added documentation Issues involving documentation subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm tooling: test infrastructure Issues involving test infrastructure or test execution, or making SAW more testable labels Mar 26, 2021
@langston-barrett
Copy link
Contributor

I'm in favor of repairing the test, probably by changing the ints to be unsigned (signed overflow is just a distraction here) and adding postconditions for the globals.

@kquick
Copy link
Member

kquick commented Mar 26, 2021

Perhaps both are useful to capture: fixing the test for the non-optimized version, and documenting that the optimized version does not fail, along with some more details on why it doesn't. The latter helps capture both (a) the behavior of optimization is documented and can be referred to as an example, and (b) if the optimization behavior changes, we will be alerted to that fact by this test now failing.

@RyanGlScott
Copy link
Contributor Author

I tried the following naïve fix:

diff --git a/intTests/test0036_global/test.c b/intTests/test0036_global/test.c
index 5980bc52..1212d461 100644
--- a/intTests/test0036_global/test.c
+++ b/intTests/test0036_global/test.c
@@ -1,15 +1,15 @@
-int x = 0;
+unsigned int x = 0;
 
-int f(int y) {
+unsigned int f(unsigned int y) {
   x = x + 1;
   return x + y;
 }
 
-int g(int z) {
+unsigned int g(unsigned int z) {
   x = x + 2;
   return x + z;
 }
 
-int h(int w) {
+unsigned int h(unsigned int w) {
   return g(f(w));
 }
diff --git a/intTests/test0036_global/test.saw b/intTests/test0036_global/test.saw
index 07ee72d8..ba56a9a4 100644
--- a/intTests/test0036_global/test.saw
+++ b/intTests/test0036_global/test.saw
@@ -10,6 +10,7 @@ f_spec <- llvm_verify m "f" [] true (do {
     y <- llvm_fresh_var "y" (llvm_int 32);
     init_global "x";
     llvm_execute_func [llvm_term y];
+    llvm_points_to (llvm_global "x") (llvm_term {{ 1 : [32] }});
     llvm_return (llvm_term {{ 1 + y : [32] }});
 }) abc;
 
@@ -17,6 +18,7 @@ g_spec <- llvm_verify m "g" [] true (do {
     z <- llvm_fresh_var "z" (llvm_int 32);
     init_global "x";
     llvm_execute_func [llvm_term z];
+    llvm_points_to (llvm_global "x") (llvm_term {{ 2 : [32] }});
     llvm_return (llvm_term {{ 2 + z : [32] }});
 }) abc;
 
@@ -26,5 +28,6 @@ llvm_verify m "h" [f_spec, g_spec] true (do {
     w <- llvm_fresh_var "w" (llvm_int 32);
     init_global "x";
     llvm_execute_func [llvm_term w];
+    llvm_points_to (llvm_global "x") (llvm_term {{ 3 : [32] }});
     llvm_return (llvm_term {{ 4 + w : [32] }});
 }) abc;

However, that still doesn't verify:

[16:47:33.068] Loading file "/home/rscott/Documents/Hacking/Haskell/saw-script/intTests/test0036_global/test.saw"
[16:47:33.104] Verifying f ...
[16:47:33.105] Simulating f ...
[16:47:33.107] Checking proof obligations f ...
[16:47:33.137] Proof succeeded! f
[16:47:33.169] Verifying g ...
[16:47:33.170] Simulating g ...
[16:47:33.172] Checking proof obligations g ...
[16:47:33.173] Proof succeeded! g
[16:47:33.206] Verifying h ...
[16:47:33.207] Simulating h ...
[16:47:33.207] Registering overrides for `f`
[16:47:33.207]   variant `Symbol "f"`
[16:47:33.207] Registering overrides for `g`
[16:47:33.207]   variant `Symbol "g"`
[16:47:33.207] Matching 1 overrides of  f ...
[16:47:33.207] Branching on 1 override variants of f ...
[16:47:33.208] Applied override! f
[16:47:33.208] Matching 1 overrides of  g ...
[16:47:33.208] Branching on 0 override variants of g ...
[16:47:33.209] Stack trace:
"llvm_verify" (/home/rscott/Documents/Hacking/Haskell/saw-script/intTests/test0036_global/test.saw:27:1-27:12):
Symbolic execution failed.
Abort due to false assumption:
  /home/rscott/Documents/Hacking/Haskell/saw-script/intTests/test0036_global/test.saw:27:1: error: in overrideBranches
  No override specification applies for g.
  Arguments:
  - symbolic integer:  width = 32
  The following overrides had some preconditions that failed concretely:
  - Name: g
    Location: /home/rscott/Documents/Hacking/Haskell/saw-script/intTests/test0036_global/test.saw:17:11
    Argument types: 
    - i32
    Return type: i32
    * /home/rscott/Documents/Hacking/Haskell/saw-script/intTests/test0036_global/test.saw:17:11: error: in _SAW_verify_prestate
      Equality precondition
      Expected value (as a SAW value): 
      global_initializer(x)
      Expected value (as a Crucible value): 
      <zero : bitvectorType 4>
      Actual value: 
      literal integer: unsigned value = 1, signed value =  1, width = 32

  Run SAW with --sim-verbose=3 to see a description of each override.
Stack frame h
  Allocations:
    StackAlloc 6 0x4:[64] Mutable 4-byte-aligned test.c:13:29
  Writes:
    Indexed chunk:
      5 |->   *(5, 0x0:[64]) := 0x1:[32]
      6 |->   *(6, 0x0:[64]) := c@79:bv
Base memory
  Allocations:
    GlobalAlloc 5 0x4:[64] Mutable 4-byte-aligned x
    GlobalAlloc 4 0x0:[64] Immutable 1-byte-aligned [defined function ] h
    GlobalAlloc 3 0x0:[64] Immutable 1-byte-aligned [defined function ] g
    GlobalAlloc 2 0x0:[64] Immutable 1-byte-aligned [defined function ] f
    GlobalAlloc 1 0x0:[64] Immutable 1-byte-aligned [external function] llvm.dbg.declare
  Writes:
    Indexed chunk:
      5 |->   *(5, 0x0:[64]) := 0

I thought that it would, given this comment:

// Note that the f and g overrides are not actually used for
// rewriting, because their preconditions aren't met.
llvm_verify m "h" [f_spec, g_spec] true (do {

However, the fact that f and g's preconditions aren't met results in an error, if I'm reading the message above correctly. Is this expected?

RyanGlScott added a commit that referenced this issue May 7, 2021
Previously, `test0036_global` only passed by sheer accident, as the overrides
it was registering would have failed to apply at a lower optimization level.
To make sure that all of the relevant code paths are exercised, this patch
reworks `test0036_global` such that:

* There are now separate `.saw` files for testing different properties when
  `test.c` is compiled at `-O1` or `-O2`. I've also checked in the relevant
  `.ll` files so that the difference between the generated `.bc` files at
  different optimization settings can be examined. For more details, consult
  the `README` in the `test0036_global` directory.
* `test.c` now uses `unsigned int`s instead of `int`s to avoid complicating
  things with signed overflow.

Fixes #1155.
@RyanGlScott
Copy link
Contributor Author

I took some liberties in reworking test0036_global as a part of #1287. Please check to see if I've preserved the original spirit of the test, but I believe the new version should test the same code paths, plus some more that weren't previously being tested.

@mergify mergify bot closed this as completed in #1287 Nov 12, 2021
@sauclovian-g sauclovian-g added test assets Issues involving test programs or other test assets and removed tooling: test infrastructure Issues involving test infrastructure or test execution, or making SAW more testable labels Oct 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Issues involving documentation subsystem: crucible-llvm Issues related to LLVM bitcode verification with crucible-llvm test assets Issues involving test programs or other test assets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants