public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range
@ 2020-12-14 19:05 ishikawa at yk dot rim.or.jp
  2020-12-14 19:08 ` [Bug tree-optimization/98281] " ishikawa at yk dot rim.or.jp
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: ishikawa at yk dot rim.or.jp @ 2020-12-14 19:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

            Bug ID: 98281
           Summary: - -Wformat-truncation false positive due to excessive
                    integer range
           Product: gcc
           Version: 10.2.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: tree-optimization
          Assignee: unassigned at gcc dot gnu.org
          Reporter: ishikawa at yk dot rim.or.jp
  Target Milestone: ---

Created attachment 49763
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49763&action=edit
Code that triggered the error.

Actually there was bug 94021 but that was for 9.2.1, and this is with 10.2.0,
and the error is subtly different. So I am submitting this bug entry.

Compared with the example in bug 94021 comment 4, the bug is slightly
different.

gcc --version
gcc (Debian 10.2.0-19) 10.2.0

The source code is from mozilla's thunderbird.

The error I observed is:

In file included from Unified_c_libical_src_libical1.c:20:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical/src/libical/icalvalue.c:
In function ‘icalvalue_utcoffset_as_ical_string_r’:
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical/src/libical/icalvalue.c:897:20:
error: ‘%02d’ directive output may be truncated writing between 2 and 6 bytes
into a region of size between 2 and 4 [-Werror=format-truncation=]
  897 |     snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s));
      |                    ^~~~~~~~~~~~~~~~
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical/src/libical/icalvalue.c:897:20:
note: directive argument in the range [1, 338339]
In file included from /usr/include/stdio.h:867,
                 from
/NEW-SSD/moz-obj-dir/objdir-tb3/dist/system_wrappers/stdio.h:3,
                 from
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical/src/libical/icaltimezone.c:34,
                 from Unified_c_libical_src_libical1.c:2:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:67:10: note:
‘__builtin___snprintf_chk’ output between 8 and 14 bytes into a destination of
size 9
   67 |   return __builtin___snprintf_chk (__s, __n, __USE_FORTIFY_LEVEL - 1,
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   68 |        __bos (__s), __fmt, __va_arg_pack ());
      |        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~                           


This does not make sense, since the value(s) ought to be constrained
to fit into the final string.

Also, I am not sure what this [1, 338339] is valid for  WHICH variable.

The code snippet from the affected function: Sorry, I could no reproduce the
problem with a simplified source code. There must be an optimization issue
involved.

--- begin quote 
static char* icalvalue_utcoffset_as_ical_string_r(const icalvalue* value)
{
    int data,h,m,s;
    char sign;
    char* str;

    if(!((value!=0))) { icalerror_set_errno(ICAL_BADARG_ERROR); return 0;};

    str = (char*)icalmemory_new_buffer(9);
    data = icalvalue_get_utcoffset(value);

    if (abs(data) == data){
 sign = '+';
    } else {
 sign = '-';
    }



    if (data >= 3600 * 24 || data <= - 3600 * 24) {
        snprintf(str,9,"+0000");
        return str;
    }

    if(data < 0)
        data = - data;







    h = data/3600;
    m = (data - (h*3600))/ 60;
    s = (data - (h*3600) - (m*60));

    if (s > 0)
    snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s));
    else
    snprintf(str,9,"%c%02d%02d",sign,abs(h),abs(m));

    return str;
}

--- end quote

The intention is that the following conditions hold before snprintf is invoked.

h is in [0, 24)
m is in [0, 60)
s is in [0, 60)

I wonder where "[1, 338339]" comes from.

Yes, I know the original code does something funny like abs(data) == data,

The preprocessed source file is in the attachment.
The command script to compile the source file is in another comment.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range
  2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
@ 2020-12-14 19:08 ` ishikawa at yk dot rim.or.jp
  2020-12-14 19:21 ` [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0) ishikawa at yk dot rim.or.jp
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: ishikawa at yk dot rim.or.jp @ 2020-12-14 19:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

--- Comment #1 from ishikawa,chiaki <ishikawa at yk dot rim.or.jp> ---
The command to compile the source file uploaded.
(Place it in /tmp)

cd tmp

export TERM=dumb

/usr/bin/gcc-10 -std=gnu99 -o /tmp/Unified_c_libical_src_libical1.o -c 
-I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/system_wrappers -include
/NEW-SSD/NREF-COMM-CENTRAL/mozilla/config/gcc_hidden.h -U_FORTIFY_SOURCE
-D_FORTIFY_SOURCE=2 -fstack-protector-strong -DDEBUG=1 -DHAVE_CONFIG_H
-DHAVE_SNPRINTF -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL
-DSTATIC_EXPORTABLE_JS_API
-I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical/src/libical
-I/NEW-SSD/moz-obj-dir/objdir-tb3/comm/calendar/libical/src/libical
-I/NEW-SSD/NREF-COMM-CENTRAL/mozilla/comm/calendar/libical
-I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include
-I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nspr
-I/NEW-SSD/moz-obj-dir/objdir-tb3/dist/include/nss -fPIC -include
/NEW-SSD/moz-obj-dir/objdir-tb3/mozilla-config.h -DMOZILLA_CLIENT
-fno-builtin-strlen -Wl,--gdb-index -Dfdatasync=fdatasync -DDEBUG_4GB_CHECK
-DUSEHELGRIND=1 -DUSEVALGRIND=1 -DDEBUG -g -gsplit-dwarf -Werror=sign-compare
-Werror=unused-result -Werror=unused-variable -Werror=format -fuse-ld=gold
-fno-strict-aliasing -ffunction-sections -fdata-sections -fno-math-errno
-pthread -pipe -g -g -Og -fvar-tracking -gdwarf-4 -fvar-tracking-assignments
-freorder-blocks -fno-omit-frame-pointer -funwind-tables -Wall -Wempty-body
-Wignored-qualifiers -Wpointer-arith -Wsign-compare -Wtype-limits
-Wunreachable-code -Wduplicated-cond -Wno-error=maybe-uninitialized
-Wno-error=deprecated-declarations -Wno-error=array-bounds
-Wno-error=coverage-mismatch -Wno-error=free-nonheap-object
-Wno-multistatement-macros -Wno-error=class-memaccess
-Wno-error=deprecated-copy -Wformat -Wformat-overflow=2
-Werror=implicit-function-declaration -Wno-psabi  -MD -MP -MF
.deps/Unified_c_libical_src_libical1.o.pp    Unified_c_libical_src_libical1.c


We get the following error at the end, but please ignore it.
Unified_c_libical_src_libical1.c: At top level:
Unified_c_libical_src_libical1.c:48: fatal error: opening dependency file
.deps/Unified_c_libical_src_libical1.o.pp: No such file or directory
   48 | #pragma GCC visibility pop

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0)
  2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
  2020-12-14 19:08 ` [Bug tree-optimization/98281] " ishikawa at yk dot rim.or.jp
@ 2020-12-14 19:21 ` ishikawa at yk dot rim.or.jp
  2020-12-17 17:14 ` msebor at gcc dot gnu.org
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: ishikawa at yk dot rim.or.jp @ 2020-12-14 19:21 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

--- Comment #2 from ishikawa,chiaki <ishikawa at yk dot rim.or.jp> ---
Created attachment 49764
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=49764&action=edit
The patch that I had for 94021

Funny I thought this was gone for a while with gcc-9 and an earlier 10 (?)

I say this because I had a patch as attached locally (which I produced since I
reported bug 94021).
But I could compile the source without the patch this Fall., it seems. Yeah, I
took it out from my local patch queeu.

Maybe something changed.

Also, please notice that the first part of the patch is for the case quite
similar to bug 94021 comment 4 test example, and I think that was somehow taken
care of (?)

Anyway, the bug(s) still persist for the static snprintf format check.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0)
  2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
  2020-12-14 19:08 ` [Bug tree-optimization/98281] " ishikawa at yk dot rim.or.jp
  2020-12-14 19:21 ` [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0) ishikawa at yk dot rim.or.jp
@ 2020-12-17 17:14 ` msebor at gcc dot gnu.org
  2020-12-24  7:08 ` ishikawa at yk dot rim.or.jp
  2020-12-24  7:11 ` ishikawa at yk dot rim.or.jp
  4 siblings, 0 replies; 6+ messages in thread
From: msebor at gcc dot gnu.org @ 2020-12-17 17:14 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

Martin Sebor <msebor at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |DUPLICATE
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #3 from Martin Sebor <msebor at gcc dot gnu.org> ---
The warning works as designed but the range information it depends on is less
than perfect.  As discussed in pr94021 that's a known limitation of the current
range propagation infrastructure.  GCC 11 adds an improved range engine known
as the Ranger that's expected to remedy this but it is yet to be integrated
with the sprintf/strlen pass.  The argument ranges can be constrained by using
a "narrower" directive such as %hhu.

A small test case that helps see what's going on is this:

$ gcc -O2 -S -Wall -fdump-tree-strlen-all=/dev/stdout pr98281.c
void f (char *d, int i)
{
  if (i >= 3600 * 24 || i <= -3600 * 24)
    return;
  if (i < 0)
    i = -i;

  int h = i / 3600;
  int m = (i - (h * 3600)) / 60;
  int s = (i - (h * 3600) - (m * 60));

  if (s > 0)
    __builtin_snprintf (d, 9, "%c%02d%02d%02d", '+', h, m, s);
}

;; Function f (f, funcdef_no=0, decl_uid=1944, cgraph_uid=1, symbol_order=0)
...
Intersecting
  int [-169140, 169199]
and
  int [1, 169199]
to
  int [1, 169199]
pushing new range for s_11: int [1, 169199]  EQUIVALENCES: { s_11 } (1
elements)   <<< last argument (s)
pr98281.c:13: snprintfD.977: objsize = 9, fmtstr = "%c%02d%02d%02d"            
   <<< snprintf call
  Directive 1 at offset 0: "%c"
    Result: 1, 1, 1, 1 (1, 1, 1, 1)
  Directive 2 at offset 2: "%02d"
    Result: 2, 2, 2, 2 (3, 3, 3, 3)
  Directive 3 at offset 6: "%02d"                                              
   <<< last directive
    Result: 2, 5, 5, 5 (5, 8, 8, 8)                                            
   <<< output between 2 and 5 bytes
  Directive 4 at offset 10: "%02d"
pr98281.c: In function ‘f’:
pr98281.c:13:42: warning: ‘%02d’ directive output may be truncated writing
between 2 and 6 bytes into a region of size between 1 and 4
[-Wformat-truncation=]
   13 |     __builtin_snprintf (d, 9, "%c%02d%02d%02d", '+', h, m, s);
      |                                          ^~~~
pr98281.c:13:31: note: directive argument in the range [1, 169199]
   13 |     __builtin_snprintf (d, 9, "%c%02d%02d%02d", '+', h, m, s);
      |                               ^~~~~~~~~~~~~~~~
    Result: 2, 6, 6, 6 (7, 14, 14, 14)
  Directive 5 at offset 14: "", length = 1
pr98281.c:13:5: note: ‘__builtin_snprintf’ output between 8 and 15 bytes into a
destination of size 9
   13 |     __builtin_snprintf (d, 9, "%c%02d%02d%02d", '+', h, m, s);
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
voidD.51 f (charD.7 * dD.1942, intD.6 iD.1943)
{
  ...
  # RANGE [-169140, 169199]                                                    
   <<< last argument (negative?)
  s_11 = _4 + _6;
  ...
  snprintfD.977 (d_13(D), 9, "%c%02d%02d%02d", 43, h_9, m_10, s_11);
  ...
}

*** This bug has been marked as a duplicate of bug 94021 ***

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0)
  2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
                   ` (2 preceding siblings ...)
  2020-12-17 17:14 ` msebor at gcc dot gnu.org
@ 2020-12-24  7:08 ` ishikawa at yk dot rim.or.jp
  2020-12-24  7:11 ` ishikawa at yk dot rim.or.jp
  4 siblings, 0 replies; 6+ messages in thread
From: ishikawa at yk dot rim.or.jp @ 2020-12-24  7:08 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

--- Comment #4 from ishikawa,chiaki <ishikawa at yk dot rim.or.jp> ---
(In reply to Martin Sebor from comment #3)
> The warning works as designed but the range information it depends on is
> less than perfect.  As discussed in pr94021 that's a known limitation of the
> current range propagation infrastructure.  GCC 11 adds an improved range
> engine known as the Ranger that's expected to remedy this but it is yet to
> be integrated with the sprintf/strlen pass.  The argument ranges can be
> constrained by using a "narrower" directive such as %hhu.
> 

Thank you for the detailed explanation.

It would be great to see this Ranger incorporated into the sprintf/strlen pass.
I say this because I found a bug in a popular code which this feature could
have found.  (The bug was found by the dynamic check done by ASAN-build.)

Thank you for continuously developing GCC with these new features.
Happy festive season to all.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0)
  2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
                   ` (3 preceding siblings ...)
  2020-12-24  7:08 ` ishikawa at yk dot rim.or.jp
@ 2020-12-24  7:11 ` ishikawa at yk dot rim.or.jp
  4 siblings, 0 replies; 6+ messages in thread
From: ishikawa at yk dot rim.or.jp @ 2020-12-24  7:11 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

--- Comment #5 from ishikawa,chiaki <ishikawa at yk dot rim.or.jp> ---
(In reply to Martin Sebor from comment #3)
> The warning works as designed but the range information it depends on is
> less than perfect.  As discussed in pr94021 that's a known limitation of the
> current range propagation infrastructure.  GCC 11 adds an improved range
> engine known as the Ranger that's expected to remedy this but it is yet to
> be integrated with the sprintf/strlen pass.  The argument ranges can be
> constrained by using a "narrower" directive such as %hhu.
> 

Thank you for the detailed explanation.

It would be great to see this Ranger incorporated into the sprintf/strlen pass.
I say this because I found a bug in a popular code which this feature could
have found.  (The bug was found by the dynamic check done by ASAN-build.)

Thank you for continuously developing GCC with these new features.
Happy festive season to all.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-12-24  7:11 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-14 19:05 [Bug tree-optimization/98281] New: - -Wformat-truncation false positive due to excessive integer range ishikawa at yk dot rim.or.jp
2020-12-14 19:08 ` [Bug tree-optimization/98281] " ishikawa at yk dot rim.or.jp
2020-12-14 19:21 ` [Bug tree-optimization/98281] - -Wformat-truncation false positive due to excessive integer range (gcc 10.2.0) ishikawa at yk dot rim.or.jp
2020-12-17 17:14 ` msebor at gcc dot gnu.org
2020-12-24  7:08 ` ishikawa at yk dot rim.or.jp
2020-12-24  7:11 ` ishikawa at yk dot rim.or.jp

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).