Compiler Writers Gone Wild: ARC Madness
blog.metaobject.com
blog.metaobject.com
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).
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
Of course, that's only a minor point in the article, but it does capture the theme that's explored in the main point.
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.
or how about when you write a for loop?
for(int i=0; i<LIMIT; i++)
..
i don't think you realise just how often undefined behaviour is used in compiler optimisations.
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.
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.
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.
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);
}
}
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..)
http://clang.llvm.org/docs/AutomaticReferenceCounting.html
Tip to writers: Even if you think your audience knows what an acronym stands for, spell it out or provide a link the first time you use it. You never know when other people will see your article who are interested in the general area but unfamiliar with your specific topic.
And of course a search for "ARC" turns up all kinds of unrelated topics. In fact, the only software-related reference on the first page of a Google search is Paul Graham's Arc.
It would have been fine if the first mention of ARC in the article was more specific: "Objective-C ARC".
This is different from x86 or WWDC, where the very first match on a Google search is exactly what you'd be looking for.
(FWIW, I upvoted your comment because I always appreciate interesting feedback.)
Really, I think this is just another example of how we should name everything using UUIDs rather than pronounceable chains of letters.
That's still 250% more instructions than the necessary "xor eax, eax; ret". I find it a bit disappointing that the compiler would miss this trivial optimisation opportunity, while at the same time doing subtle and often unwanted things with undefined behaviour. C was conceived as a "portable assembly language", and as much as the language lawyers, theoreticists, and compiler writers love to exercise their "undefined behaviour" rights and try to dissuade others from thinking of it that way, that's what people use it for and that's what they'll expect.
Rather, undefined behaviour should be interpreted as "do the obvious thing, the results may differ on different platforms, and turn off all optimisations exploiting it because it's either intentional or the programmer has made a mistake so the compiler should also generate obviously wrong code." A warning would be nice too. Instead of blind adherence to the standard, consider what behaviour programmers actually expect, and write compilers accordingly. One example of this is casting integers to pointers of various types, and using them to access hardware. Device drivers would be impossible to write if the compiler decided that such undefined behaviour meant it could remove the accesses completely (which is completely legal according to the standard.)
More examples: dereferencing a null pointer should generate code that accesses address 0. Signed integers should wrap around on overflow. Shifts should do whatever the shift instruction of the architecture does. Trying to access beyond the end of the array should try to access memory there. Division by zero will do what the machine instruction would do. Etc.
When optimisation is enabled, all extraneous instructions should disappear.
I don't know why people like the current situation so much. Strikes me it's bad for everybody. If you know how the underlying system works, you'll be confused by this second pile of strange rules that you now have to keep on top of; if you don't know the underlying system works, you're no better served by these new rules than you would have been by simply finding out. And it's not even like most systems these days vary all that much! - assuming you can even find something that isn't ARM/x86/x64 in the first place <-- this statement may be mildly tongue in cheek.
Very strange.
My favourite link on the subject is http://robertoconcerto.blogspot.co.uk/2010/10/strict-aliasin.... "The C compiler writers have no idea who their users are" seems plausible :)
"It isn't clear why those retains/releases were crashing, all the objects involved looked OK in the debugger, but at least we will no longer be puzzled by code that can't possibly crash...crashing, and therefore have a better chance of actually debugging it."
I'm also not very impressed with trotting out the assembly without using it to diagnose the crash. Particularly not if it's being used to disparage ARC (wrongly, I believe).
Here's my theory:
One or both of the parameters were passed in from unsafe references, and the objects had been deallocated. According to a simple reading of the code, you would not expect this to be a problem, because the method doesn't attempt to access either of them (it's equivalent to passing an invalid pointer to a C function, and then not accessing it).
However ARC subtly changes the semantics of the language. Strictly speaking, the parameters are supposed to be valid, as ARC adds retain/release calls for them (the fact that those are normally optimised away is purely that - an optimisation). So the bug here was that invalid pointers were being passed to a method which does actually use them (albeit non-obviously). As soon as retain was called on the invalid pointer, it ended up accessing or calling to some arbitrary location in memory, hence the crash.