Re: [PATCH, generic] Fix for define_subst

[Richard Henderson <rth at redhat dot com>]

On 11/28/2012 09:20 AM, Michael Zolotukhin wrote:
>> Where was the null condition created in the first place?
> The reason it's happening is following. Before introduction of
> define_subst, function apply_iterators had the following loop:
>       condition = NULL;
>       FOR_EACH_VEC_ELT (iterator_uses, i, iuse)
>         {
>           v = iuse->iterator->current_value;
>           iuse->iterator->group->apply_iterator (iuse->ptr, v->number);
>           condition = join_c_conditions (condition, v->string);
>         }
>       apply_attribute_uses ();
>       x = copy_rtx_for_iterators (original);
>       add_condition_to_rtx (x, condition);
> 
> This loop always iterated at least once, so 'condition' always became
> not-null (though, it could become empty-string ""). So, function
> add_condition_to_rtx always had not-null string in the arguments.
> 
> With subst-iterators this loop is looking like this:
>       condition = NULL;
>       FOR_EACH_VEC_ELT (iterator_uses, i, iuse)
>         {
>           if (iuse->iterator->group == &substs)
>             continue;
>           v = iuse->iterator->current_value;
>           iuse->iterator->group->apply_iterator (iuse->ptr, v->number);
>           condition = join_c_conditions (condition, v->string);
>         }
> So, it's possible that join_c_condition wouldn't be called at all, and
> 'condition' will remain NULL. Then it goes to add_condition_to_rtx and
> we get the fail we've seen.
> 
> There is no mistake in such behaviour, but now we should be aware of
> possible NULL-string. It should be handled properly, and I see two
> places where we could do that - in join_c_conditions or in add_c_tests
> and maybe_eval_c_test.
> 

Well, there does seem to be a mistake -- the use of NULL in the first
place.  It seems to me that the easiest fix is

  condition = "";

right at the beginning.


r~