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

[Uros Bizjak <ubizjak at gmail dot com>]

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.