[PATCH, i386]: Fix PR 67484, asan detects heap-use-after-free with target options

[Richard Biener]

And it is initialized via

void
cl_target_option_save (struct cl_target_option *ptr, struct gcc_options *opts)
{
  if (targetm.target_option.save)
    targetm.target_option.save (ptr, opts);

  ptr->x_recip_mask = opts->x_recip_mask;
  ptr->x_ix86_isa_flags = opts->x_ix86_isa_flags;
  ptr->x_ix86_fpmath = opts->x_ix86_fpmath;
  ptr->x_target_flags = opts->x_target_flags;
}

which uses a target hook to copy from gcc_options to cl_target_options...
(what a maze), and ix86_function_specific_save also plain copies the
pointers.

Richard.

On Wed, Sep 16, 2015 at 11:08 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> I see in gtype-desc.c:
>
> void
> gt_ggc_mx_cl_target_option (void *x_p)
> {
>   struct cl_target_option * const x = (struct cl_target_option *)x_p;
>   if (ggc_test_and_set_mark (x))
>     {
>       gt_ggc_m_S ((*x).x_ix86_arch_string);
>       gt_ggc_m_S ((*x).x_ix86_recip_name);
>       gt_ggc_m_S ((*x).x_ix86_tune_ctrl_string);
>       gt_ggc_m_S ((*x).x_ix86_tune_memcpy_strategy);
>       gt_ggc_m_S ((*x).x_ix86_tune_memset_strategy);
>       gt_ggc_m_S ((*x).x_ix86_tune_string);
>     }
>
> so it certainly does not expect heap allocated strings in
> ix86_arch_string and friends.
>
> Richard.
>
> On Wed, Sep 16, 2015 at 10:59 AM, Uros Bizjak <ubizjak@gmail.com> wrote:
>> On Wed, Sep 16, 2015 at 10:45 AM, Richard Biener
>> <richard.guenther@gmail.com> wrote:
>>
>>>> As mentioned in the PR, ix86_valid_target_attribute_tree creates
>>>> temporary copies of current options strings and saves *pointers* to
>>>> these copies with build_target_option_node. A couple of lines below,
>>>> these temporary copies are freed, leaving dangling pointers in the
>>>> saved structure.
>>>>
>>>> Use xstrndup to create permanent copy of string on the heap. This will
>>>> however create a small leak, as this copy is never deallocated.
>>>>
>>>> There is no test infrastructure to check for memory errors, so there
>>>> is no testcase added.
>>>>
>>>> 2015-09-15  Uros Bizjak  <ubizjak@gmail.com>
>>>>
>>>>     PR target/67484
>>>>     * config/i386/i386.c (ix86_valid_target_attribute_tree):
>>>>     Use xstrdup to copy option_strings to opts->x_ix86_arch_string and
>>>>     opts->x_ix86_tune_string.
>>>>
>>>> Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
>>>>
>>>> I'll wait a couple of days for possible comments on the above solution.
>>>
>>> I thought we have a custom destructor for target_option_node.  Ah, no,
>>> that was for target_globals.  I suppose we could add one to cl_target_option
>>> as well.  Note that currently the strings are not GTY((skip)) so it seems
>>> we expect ggc allocated strings there?  Which means the xstrdup in
>>> ix86_valid_target_attribute_inner_p should be ggc_strdup?
>>
>> This is a bit over my knowledge of option processing, but please note
>> that the only function that performs non-recursive call to
>> ix86_valid_target_attribute_inner_p also frees the strings, allocated
>> by mentioned function.
>>
>> Uros.