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

Struct field accesses generate duplicate proof obligations for pointer validity #585

Closed
brianhuffman opened this issue Nov 12, 2019 · 0 comments · Fixed by GaloisInc/crucible#347

Comments

@brianhuffman
Copy link
Contributor

Consider this C code that accesses fields from an array of structs:

#include <stdint.h>

typedef struct { uint32_t fst; uint32_t snd; } pair_t;
typedef struct { uint32_t head; pair_t tail; } triple_t;

void test(uint64_t i, triple_t *list) {
    list[i].tail.snd = 17;
}

Then we do a verification with crucible_llvm_verify, passing in an array of size 100:

crucible_llvm_verify bc "test" [] false
  do {
    i <- crucible_fresh_var "i" i64;
    crucible_precond {{ i < 100 }};
    let ty = llvm_array 100 (llvm_struct "struct.triple_t");
    list <- crucible_fresh_var "list" ty;
    list_ptr <- crucible_alloc ty;
    crucible_points_to list_ptr (crucible_term list);
    crucible_execute_func [crucible_term i, list_ptr];
  }
  do {
    print_goal;
    z3;
  };

Symbolic simulation of the address calculations then produces the following proof obligations (the memory write produces a couple more of its own, which are omitted):

  • 0xf555555555555556 <=$ i /\ i <=$ 0x0aaaaaaaaaaaaaaa
  • 12 * i <= 1200
  • 12 * i <= 1200
  • 4 + 12 * i <= 1200
  • 4 + 12 * i <= 1200
  • 8 + 12 * i <= 1200
    Notice that some of the obligations are exact duplicates of each other. Also note that the first proof obligation is to ensure that 12 * i does not cause signed overflow.

I figured out the reason for the duplication; the key is the getelementptr instructions in the LLVM:

  %7 = getelementptr inbounds %struct.triple_t, %struct.triple_t* %5, i64 %6, !dbg !57
  %8 = getelementptr inbounds %struct.triple_t, %struct.triple_t* %7, i32 0, i32 1, !dbg !59
  %9 = getelementptr inbounds %struct.pair_t, %struct.pair_t* %8, i32 0, i32 1, !dbg !60

The first one is for [i], the second is for .tail, and the third is for .snd. But note that the getelementptr instruction for a field access includes a first argument which is i32 0. This is actually treated as an array index, so what we're actually translating into Crucible is this:

    list[i][0].tail[0].snd = 17;

I think it is the extra array indexing at offset 0 that is producing the extra proof obligations. This problem only appears when combining symbolic array offsets with struct field accesses, because when the offsets are totally concrete, then the bounds checks can be evaluated statically, and they don't produce proof obligations at all.

To fix this, I think we just need to make a special case in the evaluation of getelementptr for arrays, to skip the case when the array index is a concrete 0.

brianhuffman pushed a commit to GaloisInc/crucible that referenced this issue Nov 13, 2019
Now `doPtrAddOffset` only asserts pointer validity of the result when
it actually produces a new pointer; if the offset is a concrete zero,
then it returns the original pointer which was already assumed to be
valid.

Fixes GaloisInc/saw-script#585.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
1 participant