Re: [PATCH] gimple-ssa-sprintf: Fix typo in range check

[Siddhesh Poyarekar <siddhesh@gotplt.org>]

On 2024-07-26 13:11, Jakub Jelinek wrote:
> On Thu, Jul 25, 2024 at 07:48:38PM -0400, Siddhesh Poyarekar wrote:
>> The code to scale ranges for wide chars in format_string incorrectly
>> checks range.likely to scale range.unlikely, which is a copy-paste typo
>> from the immediate previous condition.
>>
>> gcc/ChangeLog:
>>
>> 	gimple-ssa-sprintf.cc (format_string): Fix type in range check
>> 	for UNLIKELY for wide chars.
>>
>> Signed-off-by: Siddhesh Poyarekar <siddhesh@gotplt.org>
> 
> LGTM.

Thanks, pushed.

> What exactly the code really wants to do is unclear to me, what does
> the INT_MAX on the target have to do with the minimum/maximum/expected
> sizes of %S or %ls printed strings is unclear, target PTRDIFF_MAX

I think that is because the printf family returns the number of 
bytes/chars written in an int, which imposes the INT_MAX limitation on 
the format string expansion.

> maybe.  And why it uses this
> 	  if (slen.range.something < target_int_max ())
> 	    slen.range.something *= something_else;
> rather than say
> 	  slen.range.something
> 	    = MIN (slang.range.something * something_else, target_int_max ());
> perhaps with some overflow checking is also something that is hard to guess.

That's what I tried first but I settled for the minimal change because I 
didn't want to dig in deeper than I had time for to at the moment. 
Further down it checks if MAX and UNLIKELY cross INT_MAX and then resets 
to INT_MAX, but that looks suspicious on, e.g. 32-bit targets.  The code 
could use some refactoring/cleanup.

Thanks,
Sid