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 <ubiz...@gmail.com> wrote:
> On Wed, Sep 16, 2015 at 10:45 AM, Richard Biener
> <richard.guent...@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 <ubiz...@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.
