Message ID | 1318156236.808725631@www2.webmail.us |
---|---|
State | New |
Headers | show |
On Oct 9, 2011, at 3:30 AM, Nicola Pero wrote: > Unfortunately, the report was correct in that clang is producing incorrect code and > abusing the higher bits of the class->info field to store some other information. The clang folks are pretty responsive. I'd always give them a chance to `fix' thier code, before putting hack-arounds in our code in general. > PS: In case anyone wonders, I do want the GNU Objective-C Runtime to be usable with > free, non-GCC Objective-C compilers. I think that's a fine goal. I like compatibility.
>> Unfortunately, the report was correct in that clang is producing incorrect code and >> abusing the higher bits of the class->info field to store some other information. > > The clang folks are pretty responsive. I'd always give them a chance to `fix' thier code, before putting hack-arounds in our code in general. That discussion did happen in private. It wasn't pleasant. They won't change their code. In fact, I just want to fix things and not get into more discussions. Anyhow, summarizing, the traditional GNU runtime ABI has the values 0x1L or 0x2L in the class->info field. But there is no formal definition document for the ABI, so all we can say is that GCC has always set that field to either 0x1L or 0x2L. By the way, the lack of a formal definition document is a problem, and if, at some point, I get to implement a new ABI for the GNU Objective-C runtime (which I want to do) I will produce a formal document describing it - so that anyone can implement a compatible compiler or runtime. But, for the existing ABI, there is no document describing it, hence all that can be said is that GCC only stores the values 0x1L or 0x2L in the class->field. The GNU runtime then uses some of the other bits to store information on the class at runtime - eg. when the class is +initialized it sets a bit, when it is resolved it sets another, etc. clang started abusing a higher bit of that field to store information not normally present in the ABI. That worked with older versions of the GNU runtime, because (by sheer chance in my view) the higher bit they set was not being used. The fact that it was not being used was an implementation accident (in my view) since other higher bits were actually used. The new GNU runtime included in GCC 4.6.x and higher has classes "in construction" (part of the new Objective-C API) and so the next available bit in the class->info field was used to keep track of the fact that a class is in construction. That was just the next available bit, but (unknown to me) it is precisely the bit that clang was (ab)using. As a consequence, code compiled with clang no longer works with the GNU runtime from GCC 4.6.x. As there is no formal definition document for the ABI, while it seems obvious to me that they broke the ABI (since they produce object files with some reserved bits set that no version of GCC would ever produce), they claim they didn't because their hack worked with GCC up to 4.5.x and the GNU runtime ignored whether that bit was set or not - up until 4.5.x. It's a standoff because they use that higher bit to basically produce a richer ABI, so they can't easily get rid of it now, and they won't. The hack-around I added clears this higher bit, unlocks the standoff and gets things to work again. Let's hope there are no more such issues, and if we introduce a new GNU Objective-C runtime ABI, we need to make sure it is well documented so that it is possible to easily ensure compatibility between different compilers and runtimes. Thanks
On Oct 11, 2011, at 2:05 AM, Nicola Pero wrote: >>> Unfortunately, the report was correct in that clang is producing incorrect code and >>> abusing the higher bits of the class->info field to store some other information. >> >> The clang folks are pretty responsive. I'd always give them a chance to `fix' thier code, before putting hack-arounds in our code in general. > > That discussion did happen in private. It wasn't pleasant. They won't change their code. Right, then, it isn't a bug, but rather a shared ABI that we choose to be compatible with. We fix in in our system by noticing how we must set or not set the bit in our abi document and code and go on with life, it is too short. > It's a standoff It isn't a standoff, we can choose to just fix the issue and be compatible, if we want.
> It isn't a standoff, we can choose to just fix the issue and be compatible, if we want.
I guess you're right and I'm probably using the wrong word - English is not my first language. ;-)
But I meant that they could have made the same choice to be compatible (by fixing the issue
in their compiler and making their GCC-compatible ABI output actually compatible with GCC;
they already have other, clang-only, GCC-incompatible ABIs in there, so why not make the
GCC-compatible one actually compatible with GCC ?), but they didn't.
Anyhow I completely agree with you that life is too short and we spent already way too much
time discussing this. It's fixed and let's move on. :-)
Thanks
Index: init.c =================================================================== --- init.c (revision 179711) +++ init.c (working copy) @@ -643,6 +643,15 @@ assert (CLS_ISMETA (class->class_pointer)); DEBUG_PRINTF (" installing class '%s'\n", class->name); + /* Workaround for a bug in clang: Clang may set flags other than + _CLS_CLASS and _CLS_META even when compiling for the + traditional ABI (version 8), confusing our runtime. Try to + wipe these flags out. */ + if (CLS_ISCLASS (class)) + __CLS_INFO (class) = _CLS_CLASS; + else + __CLS_INFO (class) = _CLS_META; + /* Initialize the subclass list to be NULL. In some cases it isn't and this crashes the program. */ class->subclass_list = NULL; Index: class.c =================================================================== --- class.c (revision 179711) +++ class.c (working copy) @@ -764,6 +764,15 @@ return objc_get_class (name)->class_pointer; } +/* This is not used by GCC, but the clang compiler seems to use it + when targetting the GNU runtime. That's wrong, but we have it to + be compatible. */ +Class +objc_lookup_class (const char *name) +{ + return objc_getClass (name); +} + /* This is used when the implementation of a method changes. It goes through all classes, looking for the ones that have these methods (either method_a or method_b; method_b can be NULL), and reloads Index: ChangeLog =================================================================== --- ChangeLog (revision 179711) +++ ChangeLog (working copy) @@ -1,3 +1,18 @@ +2011-10-09 Nicola Pero <nicola.pero@meta-innovation.com> + + PR libobjc/49883 + * init.c (__objc_exec_class): Work around a bug in clang's code + generation. Clang sets the class->info field to values different + from 0x1 or 0x2 (the only allowed values in the traditional GNU + Objective-C runtime ABI) to store some additional information, but + this breaks backwards compatibility. Wipe out all the bits in the + fields other than the first two upon loading a class. + +2011-10-09 Nicola Pero <nicola.pero@meta-innovation.com> + + * class.c (objc_lookup_class): Added back for compatibility with + clang which seems to emit calls to it. + 2011-10-08 Richard Frith-Macdonald <rfm@gnu.org> Nicola Pero <nicola.pero@meta-innovation.com>