Re: [PATCH] libobjc encoding fix

[Ovidiu Predescu <ovidiu at cup dot hp dot com>]

Hi,

OK, what you're describing here seems to be correct, my previous observation
seems wrong. Go ahead and commit the patch, lets see how the code works on the
rest of the platforms.

Thanks,
Ovidiu

On Wed, 12 Apr 2000 06:56:33 +0200, Jakub Jelinek <jakub@redhat.com> wrote:

> On Tue, Apr 11, 2000 at 02:03:31PM -0700, Ovidiu Predescu wrote:
> > Hi,
> > 
> > The patch is definitely not correct! The objc_layout_structure_next_member()
> > function computes the size and the alignment using 'prev_type' and not 'type'.
> 
> Maybe that was intended, but in any case the current code in
> objc_layout_structure_next_member() computes the size using 'prev_type' and
> alignment using 'type' (plus adjusts size in the alignment computation by
> the needed padding for the 'type' member) - and that makes perfect sense to
> me.
> What the code does is something like (simplified, just to show what is
> computed from what):
> 	if (!prev_type) { prev_type = type; type = next(type); return 1; }
> 	if (prev_type) { record_size += sizeof(prev_type); }
> 	if (end(type)) return 0;
> 	desired_align = alignof(type);
> 	record_align = MAX(record_align, desired_align);
> 	record_size += PAD(record_size, desired_align);
> 	prev_type = type;
> 	type = next(type);
> 	return 1;
> So, if the first line is there, then in the first run it just notes
> prev_type to be the first member, in the second run computes size of first
> member and alignment of the second member and so forth.
> My patch removes the first line, so that in the first invocation it computes
> alignment of the first member, in the second round computes size of first
> member and alignment of the second and so forth.
>
> As to the question someone else had why it does not trigger on i386 as well,
> I'll try it soon (am just bootstrapping there), but I guess it is because
> the failed tests were with a 64bit field as the first member and all the
> structures would be without that normally aligned to 32bits.
> 	
> > The code you removed is initializing prev_type and type to point to the right
> > values, which is obviously wrong.
> 
> See above. The code which was done for the !prev_type case is done at the
> very end of the function after computing alignment for the first member.

-- 
Ovidiu Predescu <ovidiu@cup.hp.com>
http://www.geocities.com/SiliconValley/Monitor/7464/