-
Notifications
You must be signed in to change notification settings - Fork 84
Description
While working on witness IDE integration, @karoliineh has looked at some of our witness invariants, which has revealed some questionable ones:
-
Seemingly pointless reflexive equality:
- invariant: type: loop_invariant location: file_name: ../../sv-benchmarks/c/libvsync/arraylock.i line: 5446 column: 5 function: arraylock_init value: flags == flags format: c_expressionThis is actually a result of a scoping issue:
flagsis both a local variable in the function and a global variable. Internally one of them is renamed to something likeflags___0, but witness invariant generation translates back to original names, which both happen to be the same.
Thus, this requires a more complicated scope check in terms of original names of variables to check for possible shadowing in the original program. -
Seemingly pointless self-relation:
- invariant: type: loop_invariant location: file_name: ../../sv-benchmarks/c/pthread-divine/barrier_2t.i line: 771 column: 5 function: worker_fn value: (long long )i - (long long )i >= 0LL format: c_expressionThis is similar to the previous one, but here both
is are local variables in the same scope.
Thus, the check also needs to account for local multiple variables with the same name, possibly interacting with block scopes. -
Equality with string literal:
- invariant: type: loop_invariant location: file_name: ../../sv-benchmarks/c/coreutils-v8.31/comm_3args_ok.c line: 89784 column: 5 function: emit_ancillary_info_350238 value: '"Multi-call invocation" == infomap[1].node' format: c_expressionI think our address domain stores the string literal as
StrPtr, but as a C expression this perhaps isn't the proper way to check for string equality (comparing addresses of a literal). -
Possibly excessive function pointer casts: (our test also in sv-benchmarks)
- invariant: type: loop_invariant location: file_name: tests/regression/28-race_reach/19-callback_racing.c file_hash: a1ce836bced7c094813d29229908add6bb9373b2656bef1a46560c4458852342 line: 9 column: 3 function: foo value: callback == (int (*)())(& bar) format: c_expression - invariant: type: loop_invariant location: file_name: tests/regression/28-race_reach/19-callback_racing.c file_hash: a1ce836bced7c094813d29229908add6bb9373b2656bef1a46560c4458852342 line: 9 column: 3 function: foo value: (void *)callback == (void *)(& bar) format: c_expressionThe first invariant is from var_eq, the second from base. In both cases casts are inserted because
callbackhas typeint (*)(), but&barhas typeint (*)(void). Both are declared with()arguments, but in function definition this apparently means(void).
It would probably be good to fix our tests (with more precise function pointer types) and resubmit to sv-benchmarks.
However, independently we could maybe avoid inserting the casts because the function types should be compatible in some sense, just not identical.