[PATCH] handle sprintf(d, "%s", ...) in gimple-ssa-sprintf.c

[Jeff Law]

On 05/03/2017 08:46 AM, Martin Sebor wrote:
> On 05/03/2017 04:20 AM, Richard Biener wrote:
>> On Tue, May 2, 2017 at 4:41 PM, Martin Sebor <msebor@gmail.com> wrote:
>>>>>> FWIW, my fix for bug 79062 is only partial (it gets the pass
>>>>>> to run but the warnings are still not issued).  I don't quite
>>>>>> understand what prevents the warning flag(s) from getting set
>>>>>> when -flto is used.  This seems to be a bigger problem than
>>>>>> just the sprintf pass not doing something just right.
>>>>>
>>>>>
>>>>> I've never dug deeply in the LTO stuff, but I believe we stream the
>>>>> compiler
>>>>> flags, so it could be something there.
>>>>
>>>>
>>>> We do.
>>>>
>>>>> Alternately you might be running into a case where in LTO mode we
>>>>> recreate
>>>>> base types.  Look for a type equality tester that goes beyond just
>>>>> testing
>>>>> pointer equality.
>>>>>
>>>>> ie, in LTO I think we'll create a type based on the streamed data, 
>>>>> but I
>>>>> also think we'll create various basic types.  Thus in LTO mode pointer
>>>>> equality may not be sufficient.
>>>>
>>>>
>>>> We make sure that for most basic types we end up re-using them where
>>>> possible.
>>>> char_type_node is an example where that generally doesn't work because
>>>> it's
>>>> value depends on a command-line flag.
>>>
>>>
>>> That answers the first part of the question of why the sprintf
>>> pass wouldn't run (or do anything) with -flto.   With it fixed
>>> (as in fold-const.c or tree-ssa-strlen.c as you suggested in
>>> bug 79602) it runs and the optimization does its job, but no
>>> warnings are issued.  The wan_foo_flags for warnings that are
>>> enabled implicitly (e.g., by -Wall or -Wextra on the command
>>> line) are clear.  There seem to be dependencies between warnings
>>> in c.opt that ignore LTO (as a language), but even with those
>>> corrected (i.e., with LTO added as a language to -Wformat and
>>> -Wall) the flags are still clear when LTO runs.  Does that ring
>>> any bells for you?
>>
>> You can look at the lto_opts section (it's just a string) and see
>> that we seem to fail to pass through -Wall (or any warning option
>> I tried).  This is because
>>
>>       /* Also drop all options that are handled by the driver as well,
>>          which includes things like -o and -v or -fhelp for example.
>>          We do not need those.  The only exception is -foffload 
>> option, if we
>>          write it in offload_lto section.  Also drop all diagnostic 
>> options.  */
>>       if ((cl_options[option->opt_index].flags & (CL_DRIVER|CL_WARNING))
>>           && (!lto_stream_offload_p || option->opt_index != 
>> OPT_foffload_))
>>         continue;
>>
>> which means you have to explicitely enable diagnostics you want at
>> link time at the moment.
>>
>> If you want to change that you have to do some changes to lto-wrapper.c
>> as for example only pass through warning options that are set on all
>> input files (warning options are not kept per function).
> 
> Great, thanks for the pointer!  I might tackle that at some point.
> 
> Jeff, given that we now understand why the warnings are suppressed
> (i.e., the root cause of the rest of bug 79062), are you okay with
> treating that independently of this PR (bug 77671) and committing
> this patch (including the two-line fix to enable the sprintf return
> value optimization with LTO)?
Yea.  I think you needed to add a function comment for is_call_safe and 
try_simplify_call.  With those additions this is OK.

Sorry for the delay.

jeff