Hacker News Reader

Comment by nwallin | Hacker News Reader

c code:

    #include <stdlib.h>

    int foo(int x) {
        int* p = malloc(1024);
        x = p[x];
        free (p);
        return x;
    }

assembly:

    foo:                                    # @foo
            ret

https://godbolt.org/z/jfvKon5v7

View on HN

change `int* p = malloc(1024);` to `volatile int* p = malloc(1024);`

asm:

  foo:                                    # @foo
        push    rbx
        mov     ebx, edi
        mov     edi, 1024
        call    malloc@PLT
        movsxd  rcx, ebx
        mov     ebx, dword ptr [rax + 4\*rcx]
        mov     rdi, rax
        call    free@PLT
        mov     eax, ebx
        pop     rbx
        ret

https://godbolt.org/z/E6Teqns8b

That is because there is a read to p[x]. If it gets removed, the malloc and free disappear. volatile shouldn't appear in such code that does not handle MMIO memory.

Why not? I have used volatile countless times to work around optimisations

The optimisation is (absent a bug in the compiler) valid, unless you've written UB in there somewhere. The _reason_ to try not to volatile something is that optimisation is quite a powerful one; if the compiler can prove something written to memory can't have changed since it was written, then it doesn't need to load it from memory if it's later used. Hits to memory can be quite slow, so this optimisation entirely removing the hit can make a big difference if the code is performance sensitive.

In the general case you're right, in this case where the optimization is related to the usage of uninitialized memory I don't think that you're correct. Of course this means that the programmer must 'think like an optimizer' and rewrite is-member(i): return sparse[i] < n && dense[sparse[i]] == i as uint sparse = sparse[i] return sparse < n && dense[sparse] == i otherwise the compiler would generate two load instead of 1.

Reply on news.ycombinator.com