analyzer icon indicating copy to clipboard operation
analyzer copied to clipboard

Handle offsets on casted pointers soundly

Open jerhard opened this issue 1 year ago • 1 comments

Goblint is currently not sound in some cases when dealing with offsets taken on casted pointers.

1. Comparison of casted pointers with offsets

On the current master, Goblint claims that the last two asserts will pass, even though they actually fail when compiled with Gcc. For the first two asserts Goblint says that the result is unknown, but this sound imprecision seems more incidental.

https://github.com/goblint/analyzer/blob/3cf768b7af36882ddf81fd27809b259bfe3b016f/tests/regression/69-addresses/02-array-cast.c#L14-L23

In the example, the char-pointer b_char points to the int-array a. Then, in a conccrete runIn line 22, the variable a_4intoffset points to &a[4], whereas b_intoffset points to (char* a) + 4 = (int* a) + 1 = &a[1]. When creating the index offset in the abstraction, Goblint currently does not take into account the static type of the pointer the offset is taken from. Thus, for b_intoffset, it keeps &a with offset 4 as its value, the same as for a_4intoffset.

2. Reading and writing with casted pointers with offsets

Reading from and writing to an address with such an offsets also needs to be handled soundly. In particular, code casting b_intoffset back to int* and dereferencing it should be soundly abstracted. A related issue is #582, where only parts of a variable are overwritten with a char-pointer, and Goblint does not yield a sound value for a subsequent read. In #582, however, no offsets are involved.

jerhard avatar Mar 29 '23 15:03 jerhard

Regarding the idea of adjusting indexes to types to make this work correctly: it's done for casts in ValueDomain.cast_addr it seems, but when valuating PlusPI, the type is currently ignored for all the normal cases: https://github.com/goblint/analyzer/blob/d1df7aa79773f9fb577fb35a6ce212f77655dae0/src/analyses/base.ml#L264-L268 The type is there to handle some container_of hacks, but maybe it could also be used to adjust the scale of the added index. That might suffice.

sim642 avatar Mar 30 '23 12:03 sim642