Re: crash upon startup in GWorkcenter, Recycler, ProjectCenter and Gorm

[David Chisnall]

> On 29 Dec 2017, at 05:56, Josh Freeman <gnustep_lists@twilightedge.com> wrote:
> 
> 
> On Dec 28, 2017, at 9:25 AM, Richard Frith-Macdonald wrote:
> 
>>> On 28 Dec 2017, at 14:00, Fred Kiefer <fredkiefer@gmx.de> wrote:
>>> First a thank you to both Josh and Richard for gathering so much 
>>> information on this issue and to Richard for the quick workaround. I hope 
>>> this lets simple GNUstep applications run on systems with non-fragile 
>>> ivars. But it is definitely not the final solution for the issue. I was 
>>> hoping for David to provide more insight into the working of the 
>>> non-fragile code. When ever a class gets loaded the compiler or the runtime 
>>> have to provide information about the ivar layout and this seems to be off 
>>> for the case where the class comes from a different compilation unit and 
>>> has a structure within. (or at least this is what the current findings 
>>> suggest) Most likely this information comes from the compiler, as the 
>>> runtime itself seems to have the correct information. But according to 
>>> Josh’s finding this information isn’t just hard coded, it seems to change 
>>> even without changes to the compilation unit.
>>> 
>>> The decision we now have to make is whether this unresolved issue is bad 
>>> enough to block a GNUstep base release? I would suggest to go ahead with 
>>> that, but if anybody has a different view I am fine with that.
>> 
>> Yes, I forgot to mention;  when using Josh Freeman's debug outpupt 
>> (priinting ivar offsets as returned by the runtime and as produced by the 
>> compiler), I tried running a pure base tool as well as a gui test.
>> The behavior was consistently different in those two cases:  using a pure 
>> base tool, the offest given by the compiler for th->_gcontext matches that 
>> produced by the funtime functions, but from a gui tool it didn't.
>> In both cases, this is definitely with the code that prints the debug 
>> compiled into the base library, so it seems something about linking the gui 
>> library is causing the compiler generated code to misbehave.  Unfortunately 
>> I don't know how to figure out what might be making the difference.
> 
> 
>   When using the nonfragile ABI, the compiler emits code that calculates ivar 
> offsets using global variables - there's a global for each ivar, using the 
> naming convention:
> __objc_ivar_offset_[CLASS_NAME].[IVAR_NAME]
> 
>   For example, NSThread's _gcontext has a corresponding global in the emitted 
> code named, "__objc_ivar_offset_NSThread._gcontext".
> 
>   Nonfragile-ABI-compiled code calculates the address of an object's ivar by 
> adding the value of the ivar's __objc_ivar_offset* global to the object's 
> address.
> 
> 
>   The libobjc2 runtime keeps track of a class' ivar offsets in two places, 
> both found in the class' objc_class runtime structure:
> 
> 1) The first place ivar offsets are stored are in the class struct's 'ivars' 
> member; Within 'ivars' (type: 'struct objc_ivar_list') is an array of ivars 
> (type: 'struct objc_ivar'); The objc_ivar struct contains 3 members 
> describing an instance variable: name, type (encoding), & offset.
>   When using the runtime's API to get an ivar offset (eg. offset = 
> ivar_getOffset(class_getInstanceVariable([NSThread class], "_gcontext")); ), 
> this is where the offset value comes from: an ivar struct's offset member, 
> from the ivar list inside the class struct's 'ivars'.
> 
> 2) The second place ivar offsets are stored are in the objc_class struct's 
> 'ivar_offsets' member (type: int**). 'ivar_offsets' is a table of int 
> pointers, one pointer for each ivar, ordering matches the ivars' order in the 
> class structure. (For example, _gcontext is NSThread's 12th ivar, so the 
> address of its global, __objc_ivar_offset_NSThread._gcontext, is found in 
> NSThread's (class struct's) ivar_offsets[11]; Changing the value of 
> NSThread's ivar_offsets[11] at runtime will change the value returned by the 
> compiled code for the statement: &thread->_gcontext; (or any other code that 
> accesses _gcontext).
> 
> 
>   When the objc2 runtime loads a class, it calls the function, 
> objc_compute_ivar_offsets(), in ivar.c, which walks the array of ivars in the 
> objc_class struct's 'ivars' member, calculates each ivar's correct offset, 
> saves the calculated offset value in the ivar struct, and (after checking for 
> the nonfragile-abi) also copies the ivar struct's offset value to the 
> corresponding entry in the class struct's 'ivar_offsets' table.

Thank you for that detailed analysis!

>   However, the previously-posted diagnostic patches show that the ivar offset 
> values from the ivars structs can be different from the offset values found 
> in the same class' 'ivar_offsets' table.
> 
>   Turns out the reason for this is because the class structs' 'ivar_offsets' 
> table pointers can (sometimes? always?) be set up incorrectly so that the 
> table actually doesn't contain the addresses of __objc_ivar_offset* globals. 
> (Oddly, the values pointed to by the table do seem to correspond to the 
> compiler's initial guesses for the ivar offsets - though these guesses seem 
> to be wrong when the members are struct types - so it seems as though the 
> table pointers are pointing to a copy of the globals' initial state).
> 
>   If the 'ivar_offsets' table pointers are incorrect, changing the referenced 
> values will have no effect on the values of the __objc_ivar_offset* globals, 
> or on addresses calculated when accessing ivars - the addresses will instead 
> be calculated using the globals' initial values (containing the 
> sometimes-incorrect compiler guesses).

This seems to be due to the code I wrote to try to allow a gradual migration to 
the non-fragile ABI.  The problem is that we emit multiple __objc_ivar_offset 
globals and they *should* have a linkage type that makes the linker throw away 
all except one of them.  This is pretty horrible, but it meant that we’d still 
have the variables for the non-fragile ABI, even if the superclass didn’t 
provide them.

>   Attached are two files:
> 
> libobjc2_ivar_offsets_mismatch_fix.diff:
>   Patch for libobjc2 that adds functionality in objc_compute_ivar_offsets() 
> to make sure the current ivar's 'ivar_offsets' table entry points to the 
> address of its corresponding global; The global's address is found by a 
> manual lookup of the __objc_ivar_offset* symbol, using the dynamic linker's 
> dlsym() function.
>   This patch has so far only been tested on Ubuntu 16.04 32bit.

The patch doens’t look quite right: it seems to be using NULL for dlsym, rather 
than RTLD_DEFAULT, which would search the current binary (libobjc2.so) instead 
of the default symbol search path.  It’s also doing a lot of work to avoid 
calling asprintf.

I’m a bit hesitant to rely on this, because I’ve found that dlsym is pretty 
unreliable across platforms (particularly on stripped binaries, which packaging 
systems seem to love).

The correct fix is probably to patch clang to not bother emitting the default 
initialisation for the symbols in compilation units other than the one where 
the ivar is actually created.  This would also fix some of the linker warnings 
that I’ve seen recently.

David