Re: [PATCH v2] tree-object-size: Support strndup and strdup

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2022-11-22 15:43, Jeff Law wrote:
> 
> On 11/21/22 07:27, Siddhesh Poyarekar wrote:
>> On 2022-11-20 10:42, Jeff Law wrote:
>>>
>>> On 11/4/22 06:48, Siddhesh Poyarekar wrote:
>>>> Use string length of input to strdup to determine the usable size of 
>>>> the
>>>> resulting object.  Avoid doing the same for strndup since there's a
>>>> chance that the input may be too large, resulting in an unnecessary
>>>> overhead or worse, the input may not be NULL terminated, resulting in a
>>>> crash where there would otherwise have been none.
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>>     * tree-object-size.cc (todo): New variable.
>>>>     (object_sizes_execute): Use it.
>>>>     (strdup_object_size): New function.
>>>>     (call_object_size): Use it.
>>>>
>>>> gcc/testsuite/ChangeLog:
>>>>
>>>>     * gcc.dg/builtin-dynamic-object-size-0.c (test_strdup,
>>>>     test_strndup, test_strdup_min, test_strndup_min): New tests.
>>>>     (main): Call them.
>>>>     * gcc.dg/builtin-dynamic-object-size-1.c: Silence overread
>>>>     warnings.
>>>>     * gcc.dg/builtin-dynamic-object-size-2.c: Likewise.
>>>>     * gcc.dg/builtin-dynamic-object-size-3.c: Likewise.
>>>>     * gcc.dg/builtin-dynamic-object-size-4.c: Likewise.
>>>>     * gcc.dg/builtin-object-size-1.c: Silence overread warnings.
>>>>     Declare free, strdup and strndup.
>>>>     (test11): New test.
>>>>     (main): Call it.
>>>>     * gcc.dg/builtin-object-size-2.c: Silence overread warnings.
>>>>     Declare free, strdup and strndup.
>>>>     (test9): New test.
>>>>     (main): Call it.
>>>>     * gcc.dg/builtin-object-size-3.c: Silence overread warnings.
>>>>     Declare free, strdup and strndup.
>>>>     (test11): New test.
>>>>     (main): Call it.
>>>>     * gcc.dg/builtin-object-size-4.c: Silence overread warnings.
>>>>     Declare free, strdup and strndup.
>>>>     (test9): New test.
>>>>     (main): Call it.
>>>
>>> I'm struggling to see how the SSA updating is correct.  Yes we need 
>>> to update the virtuals due to the introduction of the call to strlen, 
>>> particularly when SRC is not a string constant.  But do we need to do 
>>> more?
>>>
>>> Don't we end up gimplifying the 1 + strlenfn (src) expression? Can 
>>> that possibly create new SSA_NAMEs?  Do those need to be put into SSA 
>>> form? I feel like I'm missing something here...
>>
>> We do all of that manually in gimplify_size_expressions, the only 
>> thing left to do is updating virtuals AFAICT.
> 
> I guess it's actually buried down in force_gimple_operand and I guess 
> for temporaries they're not going to be alive across the new gimple 
> sequence and each destination gets its own SSA_NAME, so it ought to be 
> safe.  Just had to work a bit further through things.
> 
> OK for the trunk.

Thanks, pushed with the trivial fixup that Prathamesh suggested, i.e. 
replaced 'if (!strndup)' with 'else'.

Sid