Hi Martin,

On 01/02/18 00:40, Martin Sebor wrote:
On 01/31/2018 10:36 AM, Renlin Li wrote:
Hi there,

I have a patch to fix to regressions we observed in armhf native
environment.

To effectively check out of range for format string, a target type
should be
used. And according to the standard, int type is used for "width" and
"precision"
field of format string.

Here target_strtol10 is rename to target_strtoi10, and fixed to use
target_int_max () which is target dependent.

The value returned by target_strtol10 is (target_int_max () + 1) when it
exceeds the range.
This is used to indicate its value exceeds target INT_MAX for the later
warning.

Sorry for not responding to your original email. It's still
in my inbox, just buried under a pile of stuff.

Using LONG_MAX is not ideal but unless I missed something
(it's been a while) I think using INT_MAX for the target would
be even less optimal.  Unless specified by the asterisk, width
and precision can be arbitrarily large.

I cannot find more document about this other than here: 
http://en.cppreference.com/w/c/io/fprintf

(optional) integer value or * that specifies minimum field width.
(optional) . followed by integer number or *, or neither that specifies 
precision of the conversion.

It only mentions a integer value, with no type. I assume it could be of type 
int?
It is indeed very unclear about the range of width and precision.

constraining either to INT_MAX would trigger warnings on LP64
targets for safe calls like:

   // INT_MAX is 2147483647
   sprintf (d, "%.2147483648s", "");

I think we want to use the maximum value of an unsigned type
with the greatest precision on the host.
 It will still warn
for directives with precisions in excess of HOST_WIDE_INT_MAX

Is it here a target type should be used to test the range against? Instead of 
the
host where the toolchain is built?

Regards,
Renlin

but short of using something like offset_int it's probably as
good as it's going to get.  (It has been suggested that
the whole pass would benefit from using offset_int to track
large numbers so if/when it's enhanced to make this change
it should also make the code behave more consistently across
different hosts.)




Martin


Is it Okay to commit?

Regards,
Renlin

gcc/ChangeLog:

2018-01-31  Renlin Li  <renlin...@arm.com>

    * gimple-ssa-sprintf.c (target_strtol10): Use target integer to check
    the range. Rename it into ....
    (target_strtoi10): This.
    (parse_directive): Compare with target int max instead of LONG_MAX.


On 20/06/17 12:00, Renlin Li wrote:
Hi Martin,

I did a little investigation into this. Please correct me if I missed
anything.

I build a native arm-linux-gnueabihf toolchain in armhf hardware.
It's ILP32. So in this situation:

HOST_WIDE_INT is long, which is 32-bit.
integer type 32-bit as well, so target_int_max () == LONG_MAX


gimple-ssa-sprintf.c line 2887
  /* Has the likely and maximum directive output exceeded INT_MAX?  */
  bool likelyximax = *dir.beg && res->range.likely > target_int_max ();

likelyximax will be false as the latter expression is always false.
res->range.likely is truncated to LONG_MAX (in target_strtol10 function)

I have checked in cross build environment (host x86_64), this variable
is true.

Regards,
Renlin


On 13/06/17 09:16, Renlin Li wrote:
Hi Martin,

On 04/06/17 23:24, Martin Sebor wrote:
On 06/02/2017 09:38 AM, Renlin Li wrote:
Hi Martin,

After r247444, I saw the following two regressions in
arm-linux-gnueabihf environment:

FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
line 119)
PASS: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
line 121)
FAIL: gcc.dg/tree-ssa/builtin-sprintf-warn-18.c  (test for warnings,
line 121)

The warning message related to those two lines are:
testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:119:3: warning:
'%9223372036854775808i' directive width out of range
[-Wformat-overflow=]

testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning:
'%.9223372036854775808i' directive precision out of range
[-Wformat-overflow=]

testsuite/gcc.dg/tree-ssa/builtin-sprintf-warn-18.c:121:3: warning:
'%.9223372036854775808i' directive precision out of range
[-Wformat-overflow=]

Did you notice similar things from your test environment, Christophe?

Looks like you're missing a couple of warnings.  I see the following
output with both my arm-linux-gnueabihf cross compiler and my native
x86_64 GCC, both in 32-bit and 64-bit modes, as expected by the test,
so I don't see the same issue in my environment.

Yes, it happens on arm-linux-gnueabihf native environment. the
warnings with "INT_MAX"
line are missing. I don't know if the host environment will cause the
difference.

Regards,
Renlin

Reply via email to