[llvm-testresults] Red bots at night, buildczar's delight?

OK, I guess no one else wants to express an opinion on this, so i went with this one. Real-world code may have some problems down the road, but nothing that can’t be fixed.

Sadly, the world is not so simple it seems. Let me tell you a bitter tale of sorrow and tears.

We started deploying this internally, and found code that broke. That wasn’t too surprising. Then we found out why. The code was “correct”. It included <stdlib.h> just like it should:

#include <pmmintrin.h>
#include <stdlib.h>

or some such. Seems fine? It would be on a mac. But not on Linux. Oh no. Our fearless leaders have declared that they don’t see any problem defining libc functions like this:

#ifdef cplusplus
void *malloc(size_t size) throw();
#else

#endif

Yes, that’s an exception specifier on a C malloc function. Apparently this optimizes something for GCC, but sadly it’s utterly and completely non-conforming.

Now we get really interesting, because of course C allows (and many people do) just declaring the malloc function directly rather than adding a header file. That’s great, but then you have to some how intuit whether or not your favorite standard library has broken their declaration in this way, or you get conflicting declarations of the function.

Ok, we can deal with this much. Clang has an egregious hack that will merge these specifiers and accept re-declarations missing them when the original declaration comes from within a system header. So we should be good. Look back up at the includes I mentioned, now back to me. The includes pull in mm_malloc.h first, then stdlib.h. So the first declaration doesn’t have the specifier, and the second one does.

I have no idea what the best way to solve this is. We could extend the egregious hack to allow the reverse ordering, or we could add the stdlib.h include in mm_malloc only wwhen in a hosted environment. Thoughts?

OK, I guess no one else wants to express an opinion on this, so i went with this one. Real-world code may have some problems down the road, but nothing that can’t be fixed.

Sadly, the world is not so simple it seems. Let me tell you a bitter tale of sorrow and tears.

We started deploying this internally, and found code that broke. That wasn’t too surprising. Then we found out why. The code was “correct”. It included <stdlib.h> just like it should:

#include <pmmintrin.h>
#include <stdlib.h>

or some such. Seems fine? It would be on a mac. But not on Linux. Oh no. Our fearless leaders have declared that they don’t see any problem defining libc functions like this:

#ifdef cplusplus
void *malloc(size_t size) throw();
#else

#endif

Yes, that’s an exception specifier on a C malloc function. Apparently this optimizes something for GCC, but sadly it’s utterly and completely non-conforming.

Now we get really interesting, because of course C allows (and many people do) just declaring the malloc function directly rather than adding a header file. That’s great, but then you have to some how intuit whether or not your favorite standard library has broken their declaration in this way, or you get conflicting declarations of the function.

Ok, we can deal with this much. Clang has an egregious hack that will merge these specifiers and accept re-declarations missing them when the original declaration comes from within a system header. So we should be good. Look back up at the includes I mentioned, now back to me. The includes pull in mm_malloc.h first, then stdlib.h. So the first declaration doesn’t have the specifier, and the second one does.

Hmmm, I guess this explains why the testcase changes I checked in didn’t actually fix anything…

I have no idea what the best way to solve this is. We could extend the egregious hack to allow the reverse ordering, or we could add the stdlib.h include in mm_malloc only wwhen in a hosted environment. Thoughts?

Given that we’re hacking around this header file bug in the compiler, don’t we have to allow the reverse ordering anyway?
#include <stddef.h>
extern void* malloc(size_t);
#include <stdlib.h>

As for the freestanding issue, the gcc version has #if __STDC_HOSTED in mmintrin.h, around the (only) include for mm_malloc.h. It seems to me we might as well do that; the mm_malloc functions call malloc and free, so they won’t work in a freestanding environment.

Given that we’re hacking around this header file bug in the compiler, don’t we have to allow the reverse ordering anyway?
#include <stddef.h>
extern void* malloc(size_t);
#include <stdlib.h>

I think so, but I worry about further hacks. =[ Seems likely we have no choice.

As for the freestanding issue, the gcc version has #if __STDC_HOSTED in mmintrin.h, around the (only) include for mm_malloc.h. It seems to me we might as well do that; the mm_malloc functions call malloc and free, so they won’t work in a freestanding environment.

Ok, I’ll work on both angles of this: STDC_HOSTED guards for mm_malloc.h includes, and allowing a merge in both directions.

Thanks for the note, and let me know if I can help fix any more fallout from it.

For reference, I’m now convinced this isn’t a good idea. GCC actually rejects the construct you propose, and POSIX is pretty clear that this type of redeclaring isn’t allowed: ‘malloc’ is in its reserved set of identifiers that can be implemented via function-like macros. Clang shouldn’t get more lax than GCC here, and we don’t want to start forward declaring these functions.

I’m going to revert my change to mm_malloc.h, and use STDC_HOSTED to guard our only inclusion of it. I’m also going to temporarily remove it from the regression test suite while I discuss w/ Clang devs in general whether the un-hosted requirement for the test suite is going to remain feasible. Sorry again for the noise here. =[

I’m not disagreeing with your ultimate decision here, but for the record, these are system headers; they’re permitted to make assumptions about other system headers, it’s just a porting burden to actually do so.

John.