V: element pointer dereference after array realloc

[oconnor663]

The readme mentions that "no undefined behavior" is a key feature of V. Does the following program demonstrate undefined behavior?

// Make an array of numbers.
mut nums := [0, 1, 2, 3]

// Take a reference to the last element.
x := &nums[3]

// Read the reference and print it. Prints 3.
println(*x)

// Add a new element to the array.
nums << 4

// Read the reference and print it again.
// Expected: Either prints 3 again, or a bounds check fails, or a compiler error.
// Actual: Prints some other number, a different value each time.
println(*x)

It looks like inserting an element causes the array to be reallocated, which invalidates the reference. Valgrind confirms that this is an "Invalid read of size 4". (Edit: Confirmed this in the generated C code below.)

Does V consider this to be undefined behavior? If so, is there a plan to catch dangling pointer reads like the one above at compile time? If not, what is V's definition of undefined behavior?

[nyaa8]

Hello!
I can't replicate this issue. Is it fixed now?

Thank you for reading this, and sorry in advance, I am new to V. 🙇‍♀️

Details

OS: macOS Catalina 10.15.5, Hackintosh (iMac15,1)
Shell: fish shell, version 3.1.2
V: 0.1.28 91c9c0c

[Ivo-Balbaert]

On Windows 10 and V version V 0.1.28 eb47ce1, the second println(*x) always prints 0, whereas the expected value is 3.

[oconnor663]

If I understand the underlying issue correctly, the result is going to vary a lot depending on compiler settings and what the rest of the program is doing. We're trying to read old memory that's been freed, and the original value might still be there, or it might not. If it's not there, it might get overwritten by something consistent, or it might be random-looking.

[oconnor663]

We can see what's going on in the transpiled C code like this:

❯ cat test.v
mut nums := [0, 1, 2, 3]
x := &nums[3]
println(*x)
nums << 4
println(*x)

❯ TMPDIR=. v test.v

❯ tree
.
├── test
├── test.v
└── v
    ├── test.tmp.c
    └── test.tmp.c.rsp

Now if we look in test.tmp.c, we see these functions:

static void main__main() {
    array_int nums = new_array_from_c_array(4, 4, sizeof(int), _MOV((int[4]){0, 1, 2, 3}));
    int* x = &(*(int*)array_get(nums, 3));
    println(int_str(*x));
    array_push(&nums, _MOV((int[]){ 4 }));
    println(int_str(*x));
}

static void array_push(array* a, voidptr val) {
    array_ensure_cap(a, a->len + 1);
    { // Unsafe block
        memcpy(((byteptr)(a->data)) + a->element_size * a->len, val, a->element_size);
    }
    a->len++;
}

inline static void array_ensure_cap(array* a, int required) {
    if (required <= a->cap) {
        return;
    }
    int cap = (a->cap == 0 ? (2) : (a->cap * 2));
    while (required > cap) {
        cap *= 2;
    }
    if (a->cap == 0) {
        a->data = vcalloc(cap * a->element_size);
    } else {
        a->data = v_realloc(a->data, ((u32)(cap * a->element_size)));
    }
    a->cap = cap;
}

This is the expected problem. array_get obtains a pointer into the array buffer, but array_push calls array_ensure_cap which calls realloc, invalidating that pointer.

[JalonSolov]

Bottom line: As soon as you start messing with pointers and references, it is far too easy to cause undefined behavior. In fact, this particular type of thing should not be allowed outside an unsafe block, for that very reason.

[medvednikov]

This is an old bug, and this will be fixed. *x will always have 3

[oconnor663]

@medvednikov is the plan to keep the backing array alive as long as there are still pointers to it, like Go does? Will that require some kind of GC?

[Ivo-Balbaert]

Please solve this issue before v 0.2 is released.