[PATCH] omGeneric: Fix NULL pointer dereferences in set_fontset_extents

[Alan Coopersmith]

On 07/17/15 11:21 AM, Ismael Luceno wrote:
> On Wed, 15 Jul 2015 15:57:17 -0300
> Ismael Luceno <ismael at iodev.co.uk> wrote:
>> On Wed, 15 Jul 2015 07:24:59 -0700
>> Alan Coopersmith <alan.coopersmith at oracle.com> wrote:
>>> On 07/14/15 09:36 PM, Ismael Luceno wrote:
>>>> @@ -410,7 +410,7 @@ set_fontset_extents(
>>>>    		font_data = (FontData) font_set->vrotate;
>>>>    		font_data_count = font_set->vrotate_num;
>>>>    		for( ; font_data_count-- ; font_data++) {
>>>> -		    if(font_data->font != NULL) {
>>>> +		    if(font_data && font_data->font) {
>>>>    			check_fontset_extents(&overall,
>>>> &logical_ascent, &logical_descent,
>>>>    					      font_data->font);
>>>
>>> This one is more definitively handled in the wrapping check:
>>>
>>>               if(font_set->vrotate_num > 0 && font_set->vrotate !=
>>> NULL) {
>>>
>>> I can't see any way font_data could ever be NULL here.
>>>
>>
>> I've been thinking it may be a bug in GCC (5.1), I see it optimizes
>> out a lot around there, but had no time to confirm it yet...
>>
>> I can upload the output if you want to look at the machine code.
>>
>> I will try building with different flags to point out the problem
>> (might take some time, there's a lot to test and I'm quite busy with
>> other stuff).
>
> It seems to be an issue with GCC's speculative load scheduling, but
> I'm not entirely sure that what GCC is doing is wrong...

Are you building with strict-aliasing on or off?  Since the check is
done before casting, I wonder if gcc is deciding they can't alias due
to the type differences.

-- 
	-Alan Coopersmith-              alan.coopersmith at oracle.com
	 Oracle Solaris Engineering - http://blogs.oracle.com/alanc