8 ms·

FYI, at least in C/C++, the compiler is free to throw away assignments to any memory pointed to by a pointer if said pointer is about to be passed to free(), so

by dmitrygr 10mo ago

FYI, at least in C/C++, the compiler is free to throw away assignments to any memory pointed to by a pointer if said pointer is about to be passed to free(), so depending on how you did this, no perf impact could have been because your compiler removed the assignment. This will even affect a call to memset() see here: https://godbolt.org/z/rMa8MbYox https://godbolt.org/z/rMa8MbYox

shakna 10mo ago
However, if you recast to volatile, the compiler will keep it: #include <stdlib.h> #include <string.h> void free(void* ptr); void not_free(void* ptr); void test_with_free(char* ptr) { ptr[5] = 6; void *(* volatile memset_v)(void *s, int c, size_t n) = memset; memset_v(ptr + 2, 3, 4); free(ptr); } void test_with_other_func(char* ptr) { ptr[5] = 6; void *(* volatile memset_v)(void *s, int c, size_t n) = memset; memset_v(ptr + 2, 3, 4); not_free(ptr); }
- maxlybbert 10mo ago
  Newer versions of C++ (and C, apparently) have functions so that the cast isn't necessary ( https://en.cppreference.com/w/c/string/byte/memset.html https://en.cppreference.com/w/c/string/byte/memset.html ).
- cperciva 10mo ago
  That code is not guaranteed to work. Declaring memset_v as volatile means that the variable has to be read, but does not imply that the function must be called; the compiler is free to compile the function call as "tmp = memset_v; if (tmp != memset) tmp(...)" relying on its knowledge that in the likely case of equality the call can be optimized away.
  - shakna 10mo ago
    Whilst the C standard doesn't guarantee it, both LLVM and GCC _do_. They have implementation-defined that it will work, so are not free to optimise it away. [0] https://llvm.org/docs/LangRef.html#llvm-memset-intrinsics https://llvm.org/docs/LangRef.html#llvm-memset-intrinsics [1] https://gitweb.git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/memset_explicit.c;hb=refs/heads/stable-202301 https://gitweb.git.savannah.gnu.org/gitweb/?p=gnulib.git;a=b...
    - raverbashing 10mo ago
      Yeah the C committee is wrong here
      - uecker 10mo ago
        I don't see why? The C committee gave you memset_explicit. But note that there is still no guarantee that information can not leak. This is generally a very hard problem as information can leak in many different ways as it may have been copied by the compiler. Fully memory safe languages (so "Safe Rust" but not necessarily real-word Rust) would offer a bit more protection by default, but then there are still side-channel issues.
        raverbashing 10mo ago
        Because, for the 1384th time, they're pretending they can ignore what the programmer explicitly told them to do Creating memset_explicit won't fix existing code. "Oh but what if maybe" is just cope. If I do memset then free then that's what I want to do And the way things go I won't be surprised if they break memset_explicit for some other BS reason and then make you use memset_explicit_you_really_mean_it_this_time
        uecker 10mo ago
        Your problem is not the C committee but your lack of understanding how optimizing compilers work. WG14 could, of course, specify that a compiler has do exactly what you tell it to do. And in fact, every compiler supports this already: Im most cases even by default! Just do not turn on optimization. But this is not what most people want. Once you accept that optimizing compilers do, well, optimizations, the question is what should be allowed and what not. Both inlining "memset" and eliminating dead stores are both simply optimizations which people generally want. If you want a store not to be eliminated by a compiler, you can make it volatile. The C standard says this can not be deleted by optimizations. The criticism with this was that later undefined behavior could "undo" this by "travelling in time". We made it clear in ISO C23 that this not allowed (and I believe it never was) - against protests from some compiler folks. Compilers still do not fully conform to this, which shows the limited power WG14 has to change reality.
kentonv 10mo ago
I patched the free() implementation itself, not the code that calls free(). I did, of course, test it, and anyway we now run into the "freed memory" pattern regularly when debugging (yes including optimized builds), so it's definitely working.

shakna 10mo ago

However, if you recast to volatile, the compiler will keep it: #include <stdlib.h> #include <string.h> void free(void* ptr); void not_free(void* ptr); void test_with_free(char* ptr) { ptr[5] = 6; void *(* volatile memset_v)(void *s, int c, size_t n) = memset; memset_v(ptr + 2, 3, 4); free(ptr); } void test_with_other_func(char* ptr) { ptr[5] = 6; void *(* volatile memset_v)(void *s, int c, size_t n) = memset; memset_v(ptr + 2, 3, 4); not_free(ptr); }

maxlybbert 10mo ago
Newer versions of C++ (and C, apparently) have functions so that the cast isn't necessary ( https://en.cppreference.com/w/c/string/byte/memset.html https://en.cppreference.com/w/c/string/byte/memset.html ).
cperciva 10mo ago
That code is not guaranteed to work. Declaring memset_v as volatile means that the variable has to be read, but does not imply that the function must be called; the compiler is free to compile the function call as "tmp = memset_v; if (tmp != memset) tmp(...)" relying on its knowledge that in the likely case of equality the call can be optimized away.
- shakna 10mo ago
  Whilst the C standard doesn't guarantee it, both LLVM and GCC _do_. They have implementation-defined that it will work, so are not free to optimise it away. [0] https://llvm.org/docs/LangRef.html#llvm-memset-intrinsics https://llvm.org/docs/LangRef.html#llvm-memset-intrinsics [1] https://gitweb.git.savannah.gnu.org/gitweb/?p=gnulib.git;a=blob_plain;f=lib/memset_explicit.c;hb=refs/heads/stable-202301 https://gitweb.git.savannah.gnu.org/gitweb/?p=gnulib.git;a=b...
  - raverbashing 10mo ago
    Yeah the C committee is wrong here
    - uecker 10mo ago
      I don't see why? The C committee gave you memset_explicit. But note that there is still no guarantee that information can not leak. This is generally a very hard problem as information can leak in many different ways as it may have been copied by the compiler. Fully memory safe languages (so "Safe Rust" but not necessarily real-word Rust) would offer a bit more protection by default, but then there are still side-channel issues.
      - raverbashing 10mo ago
        Because, for the 1384th time, they're pretending they can ignore what the programmer explicitly told them to do Creating memset_explicit won't fix existing code. "Oh but what if maybe" is just cope. If I do memset then free then that's what I want to do And the way things go I won't be surprised if they break memset_explicit for some other BS reason and then make you use memset_explicit_you_really_mean_it_this_time
        uecker 10mo ago
        Your problem is not the C committee but your lack of understanding how optimizing compilers work. WG14 could, of course, specify that a compiler has do exactly what you tell it to do. And in fact, every compiler supports this already: Im most cases even by default! Just do not turn on optimization. But this is not what most people want. Once you accept that optimizing compilers do, well, optimizations, the question is what should be allowed and what not. Both inlining "memset" and eliminating dead stores are both simply optimizations which people generally want. If you want a store not to be eliminated by a compiler, you can make it volatile. The C standard says this can not be deleted by optimizations. The criticism with this was that later undefined behavior could "undo" this by "travelling in time". We made it clear in ISO C23 that this not allowed (and I believe it never was) - against protests from some compiler folks. Compilers still do not fully conform to this, which shows the limited power WG14 has to change reality.

kentonv 10mo ago

I patched the free() implementation itself, not the code that calls free(). I did, of course, test it, and anyway we now run into the "freed memory" pattern regularly when debugging (yes including optimized builds), so it's definitely working.