5 ms·

The complaints about clang optimizations are misplaced; the example relies on undefined behavior; regardless of what you think about it, all compilers are free

The complaints about clang optimizations are misplaced; the example relies on undefined behavior; regardless of what you think about it, all compilers are free to do such things. This is also a classic case of optimizing for the wrong and uncommon things (billions of objc_msgsend calls in a tight loop), instead of the really common stuff (like leaks/crashes due to a mistake in memory management, lower productivity across your entire team due to time wasted thinking about retain/release or tracking down aforementioned leaks, etc).

mpweiher 12y ago
Two points. 1. Breaking reflexivity is not OK. Ever. a==a should hold. 2. The compiler knows that this is undefined behavior, and uses this to "optimize" to a wrong result. However, it does not warn me about the use of undefined behavior. Not with -Wall, not with -Wpedantic. marcel@localhost[tmp]cc -Wall -Wpedantic -Os -o undefined-realloc undefined-realloc.c marcel@localhost[tmp]./undefined-realloc 1 2 I can't think of any reason for this to be OK. Either warn me, or don't use it. Silently breaking reflexivity...did I mention "not OK"? Of course, that's only a minor point in the article, but it does capture the theme that's explored in the main point.
- foxhill 12y ago
  the compiler will rely on undefined behaviour all the time for any given set of optimisations, a lot of them are harmless. the compiler can't know if a use of undefined behaviour is intentional and safe, or unintentional and dangerous. you could then argue that there are some cases in which undefined behaviour is obviously a mistake. and this is what the static code analysis in clang does. it's just not on by default.
  - Kliment 12y ago
    Harmless or not, I'd like to have a way to know when the compiler is relying on undefined behavior for optimization, and use that to make an informed decision. I'd argue a very large number of the cases where undefined behavior is used by the compiler are actual mistakes in the code, and having a way to get those marked by the compiler would be useful.
    - mikeash 12y ago
      There's actually a pretty simple way to determine whether the compiler is relying on undefined behavior for optimization. Simply evaluate the following two criteria: 1. Are you using a C compiler? 2. Are optimizations enabled? If the answer to both of these is "yes", then the compiler is almost certainly relying on undefined behavior for optimization.
    - foxhill 12y ago
      well, this would mean you'd get feedback every time you declare a variable without giving it an initial value? (even if the value is never read from before being written to - i.e is entirely safe to do?) or how about when you write a for loop? for(int i=0; i<LIMIT; i++) .. the compiler assumes that i cannot overflow in this case, because that would be undefined behaviour. on the most obvious level, it doesn't need to do an overflow check on each loop iteration, if it exploits undefined behaviour. and no, it simply can't check the value of LIMIT before the loop, because i may be changed in the loop body, or LIMIT itself may change. this also allows a myriad of other loop optimisations. pipelining, vectorization, unrolling.. i don't think you realise just how often undefined behaviour is used in compiler optimisations.
      - Kliment 12y ago
        Yes, and having the compiler tell me these things would be welcome. I'm not saying I don't WANT those optimizations, I just want to be aware when they happen.
        foxhill 12y ago
        for even simple programs, this would produce a very large output. we've become accustomed to the idea that C is a low level language, but actually it's somewhat high level. optimisations are in your code everywhere, and if a C compiler were to list even only the ones that exploit undefined behaviour.. well, it would essentially be noise.
- hackcasual 12y ago
  There's more discussion about this on this reddit thread: http://www.reddit.com/r/programming/comments/x1ql4/undefined_behavior_in_c_contest_winners/ http://www.reddit.com/r/programming/comments/x1ql4/undefined... The problem is that realloc builtin's return type is internally flagged as noalias, making clang think it was safe to use constant propagation for the printf. I guess LLVM doesn't take the equality check into account. > 1. Breaking reflexivity is not OK. Ever. a==a should hold. This reminds me of the "proof" that 1=2. Once an undefined operation is introduced, the end result means nothing. > 2. The compiler knows that this is undefined behavior, and uses this to "optimize" to a wrong result. Maybe, maybe not. It's not a requirement that a compiler even recognizes that undefined behavior happens. In many cases behavior is undefined simply because it would be too expensive or impractical for the compiler to deal with that behavior.
- ridiculous_fish 12y ago
  1. a==a is false for NaN, so it's not quite "Ever" 2. The compiler does not know this is undefined behavior. Dereferencing a pointer that has been previously passed to realloc is not necessarily undefined behavior. realloc may fail and return NULL; in that case the passed-in pointer remains valid.
  - tarpherder 12y ago
    Just a note: "a==a is false for NaN" is only true if compiled with precise or strict floating point mode. It will return true in fast mode, which is often what is used. Do not rely on this behavior for checking NaNs, use isnan instead.
- __david__ 12y ago
  > 1. Breaking reflexivity is not OK. Ever. a==a should hold. No. Once you've invoked undefined behavior, all bets are off. There's a trivial solution: Don't do that. Once you've called realloc() and it's returned non-NULL, p is gone--for all anyone knows it points to random nothingness. Yes, it seems weird that the library was able to enlarge it in-place but the pointers end up non-equivalent. But you can't ever blindly rely on the in-place realloc() optimization anyway. Simply put, using p blindly after a successful realloc() is a use-after-free bug, no matter what. If you use realloc() correctly you will never see this. Note that if you've proven the equivalence to the compiler it works fine: #include <stdio.h> #include <stdlib.h> int main() { int *p = (int*)malloc(sizeof(int)); int *q = (int*)realloc(p, sizeof(int)); if (p == q) { *p = 1; *q = 2; printf("%d %d\n", *p, *q); } } Production code should never do this, though (it's pointless). The safest, most correct thing to do is to stop using p after a successful realloc().
  - foxhill 12y ago
    actually, realloc infrequently moves data around, especially if it's large. quite often the returned pointer value is the same. but yes, even if it has been free'd, the page in which it resided is likely still valid, and free doesn't zero out memory. so, dereferencing p even after realloc will likely point to a section of memory with the data that was previously there, in it. that said, it is, quite rightly, a use-after-free 101 bug. i only mention because a lot of people see this behaviour, and it only explodes sometime later (either exceed page boundary in old data, or getting stale data later on..)
    - hackcasual 12y ago
      I believe realloc must return the same memory spot if the size (and maybe type?) didn't change.
      - ygra 12y ago
        The type is irrelevant , as realloc gets a void* and a size. But the standard says nothing about that a resize has to happen in-place if possible (which would be the case if it gets smaller or remains the same size). There is no requirement for the case that the requested size is the same as the memory block had before. That being said, most realloc implementations will probably hand out the same pointer again if size <= old_size; it just makes sense to move only when needed. But that behaviour is not required.
- deleted 12y ago
  [deleted]
- mikeash 12y ago
  In what way is reflexivity broken in your example? I don't see anywhere that a==a doesn't hold.

mpweiher 12y ago

Two points. 1. Breaking reflexivity is not OK. Ever. a==a should hold. 2. The compiler knows that this is undefined behavior, and uses this to "optimize" to a wrong result. However, it does not warn me about the use of undefined behavior. Not with -Wall, not with -Wpedantic. marcel@localhost[tmp]cc -Wall -Wpedantic -Os -o undefined-realloc undefined-realloc.c marcel@localhost[tmp]./undefined-realloc 1 2 I can't think of any reason for this to be OK. Either warn me, or don't use it. Silently breaking reflexivity...did I mention "not OK"? Of course, that's only a minor point in the article, but it does capture the theme that's explored in the main point.

foxhill 12y ago
the compiler will rely on undefined behaviour all the time for any given set of optimisations, a lot of them are harmless. the compiler can't know if a use of undefined behaviour is intentional and safe, or unintentional and dangerous. you could then argue that there are some cases in which undefined behaviour is obviously a mistake. and this is what the static code analysis in clang does. it's just not on by default.
- Kliment 12y ago
  Harmless or not, I'd like to have a way to know when the compiler is relying on undefined behavior for optimization, and use that to make an informed decision. I'd argue a very large number of the cases where undefined behavior is used by the compiler are actual mistakes in the code, and having a way to get those marked by the compiler would be useful.
  - mikeash 12y ago
    There's actually a pretty simple way to determine whether the compiler is relying on undefined behavior for optimization. Simply evaluate the following two criteria: 1. Are you using a C compiler? 2. Are optimizations enabled? If the answer to both of these is "yes", then the compiler is almost certainly relying on undefined behavior for optimization.
  - foxhill 12y ago
    well, this would mean you'd get feedback every time you declare a variable without giving it an initial value? (even if the value is never read from before being written to - i.e is entirely safe to do?) or how about when you write a for loop? for(int i=0; i<LIMIT; i++) .. the compiler assumes that i cannot overflow in this case, because that would be undefined behaviour. on the most obvious level, it doesn't need to do an overflow check on each loop iteration, if it exploits undefined behaviour. and no, it simply can't check the value of LIMIT before the loop, because i may be changed in the loop body, or LIMIT itself may change. this also allows a myriad of other loop optimisations. pipelining, vectorization, unrolling.. i don't think you realise just how often undefined behaviour is used in compiler optimisations.
    - Kliment 12y ago
      Yes, and having the compiler tell me these things would be welcome. I'm not saying I don't WANT those optimizations, I just want to be aware when they happen.
      - foxhill 12y ago
        for even simple programs, this would produce a very large output. we've become accustomed to the idea that C is a low level language, but actually it's somewhat high level. optimisations are in your code everywhere, and if a C compiler were to list even only the ones that exploit undefined behaviour.. well, it would essentially be noise.
hackcasual 12y ago
There's more discussion about this on this reddit thread: http://www.reddit.com/r/programming/comments/x1ql4/undefined_behavior_in_c_contest_winners/ http://www.reddit.com/r/programming/comments/x1ql4/undefined... The problem is that realloc builtin's return type is internally flagged as noalias, making clang think it was safe to use constant propagation for the printf. I guess LLVM doesn't take the equality check into account. > 1. Breaking reflexivity is not OK. Ever. a==a should hold. This reminds me of the "proof" that 1=2. Once an undefined operation is introduced, the end result means nothing. > 2. The compiler knows that this is undefined behavior, and uses this to "optimize" to a wrong result. Maybe, maybe not. It's not a requirement that a compiler even recognizes that undefined behavior happens. In many cases behavior is undefined simply because it would be too expensive or impractical for the compiler to deal with that behavior.
ridiculous_fish 12y ago
1. a==a is false for NaN, so it's not quite "Ever" 2. The compiler does not know this is undefined behavior. Dereferencing a pointer that has been previously passed to realloc is not necessarily undefined behavior. realloc may fail and return NULL; in that case the passed-in pointer remains valid.
- tarpherder 12y ago
  Just a note: "a==a is false for NaN" is only true if compiled with precise or strict floating point mode. It will return true in fast mode, which is often what is used. Do not rely on this behavior for checking NaNs, use isnan instead.
__david__ 12y ago
> 1. Breaking reflexivity is not OK. Ever. a==a should hold. No. Once you've invoked undefined behavior, all bets are off. There's a trivial solution: Don't do that. Once you've called realloc() and it's returned non-NULL, p is gone--for all anyone knows it points to random nothingness. Yes, it seems weird that the library was able to enlarge it in-place but the pointers end up non-equivalent. But you can't ever blindly rely on the in-place realloc() optimization anyway. Simply put, using p blindly after a successful realloc() is a use-after-free bug, no matter what. If you use realloc() correctly you will never see this. Note that if you've proven the equivalence to the compiler it works fine: #include <stdio.h> #include <stdlib.h> int main() { int *p = (int*)malloc(sizeof(int)); int *q = (int*)realloc(p, sizeof(int)); if (p == q) { *p = 1; *q = 2; printf("%d %d\n", *p, *q); } } Production code should never do this, though (it's pointless). The safest, most correct thing to do is to stop using p after a successful realloc().
- foxhill 12y ago
  actually, realloc infrequently moves data around, especially if it's large. quite often the returned pointer value is the same. but yes, even if it has been free'd, the page in which it resided is likely still valid, and free doesn't zero out memory. so, dereferencing p even after realloc will likely point to a section of memory with the data that was previously there, in it. that said, it is, quite rightly, a use-after-free 101 bug. i only mention because a lot of people see this behaviour, and it only explodes sometime later (either exceed page boundary in old data, or getting stale data later on..)
  - hackcasual 12y ago
    I believe realloc must return the same memory spot if the size (and maybe type?) didn't change.
    - ygra 12y ago
      The type is irrelevant , as realloc gets a void* and a size. But the standard says nothing about that a resize has to happen in-place if possible (which would be the case if it gets smaller or remains the same size). There is no requirement for the case that the requested size is the same as the memory block had before. That being said, most realloc implementations will probably hand out the same pointer again if size <= old_size; it just makes sense to move only when needed. But that behaviour is not required.
deleted 12y ago
[deleted]
mikeash 12y ago
In what way is reflexivity broken in your example? I don't see anywhere that a==a doesn't hold.