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

Manuel López-Ibáñez <manu at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |manu at gcc dot gnu.org

--- Comment #16 from Manuel López-Ibáñez <manu at gcc dot gnu.org> ---
To be honest, I find the fancy UI too overwhelming and a bit redundant in
simple cases.

It is true that the diagnostics could be improved by some tweaks. I took some
of the ideas here and I came up with the following flexible template. 

1. First the main warning has only four variants, depending on whether we are
writing a fixed number or a range and whether we are sure or not:

warning: formatting %d bytes will/may overflow buffer '%BUFF' of size %BUFFSIZE

warning: formatting between %d and %d bytes will/may overflow buffer '%BUFF' of
size %BUFFSIZE

Example:

demo.c:6:3: warning: formatting 43 bytes will overflow 'buff' of size 16
[-Wformat-overflow=]
     6 |   sprintf (buf, "msg: %s\n", msg);
       |            ~~~   ^~~~~~~~~

2. The next note explains the details (for all cases in comment #1):

// bounded, definite truncation copying format string
zzz.c:15:30: note: the format string will be truncated at character 'B' after
%d bytes
   snprintf (d, sizeof d, "%iAB", 123);
                           ~~~^
// unbounded, definite overflow in a directve
zzz.c:18:15: note: the terminating nul will be written after the end of 'd'
   sprintf (d, "%i", 1235);
            ^
// unbounded, definite overflow copying format string
zzz.c:21:19: note: character ‘B’ of the format string will be written after the
end of 'd'
   sprintf (d, "%iAB", 123);
                ~~~^
(I don't think we need to specify the offset, the column number and the caret
line should already point to the correct 'B', otherwise, it is more important
to get that fixed)

// bounded, possible truncation a directive
zzz.c:24:27: note: argument 4 corresponding to ‘%i’ has range [‘1’,
‘-2147483648’] and may be truncated after 4 bytes
   snprintf (d, sizeof d, "%i", i);
                           ~^   ~

// bounded, possible truncation copying format string
zzz.c:27:27: note: argument 4 corresponding to '%i' has range [‘1’,
‘-2147483648’] and may be truncated after 4 bytes 
   snprintf (d, sizeof d, "%iAB", i);
                           ~^     ~
(these may look different, but they are exactly the same case)

// unbounded, possible overflow in a directive
zzz.c:30:16: note: argument 4 corresponding to '%i' has range [‘1’,
‘-2147483648’] and may write between 1 and 11 bytes after the end of 'd'
   sprintf (d, "%i", i);
                ~^   ~
// unbounded, possible overflow copying format string
zzz.c:37:19: note: character 'B' of the format string may be written after the
end of 'd'
   sprintf (d, "%sAB", s);
                ~~~^
3. And additional note may explain where the argument comes from for those
cases in (2) where we mention an argument:

demo.c:12:11: the value of 'msg' comes from here
    12 |   test_1 ("this is long enough to cause trouble")


I think it is not very useful to complicate it further because we would need to
consider cases where:

1. There are multiple directives of variable size.
2. The format string may be very long (too long to print).
3. The expanded arguments may be very long (too long to print).

I agree that in those cases, something like this:


   note: layout [-fsome-option-enabling-this]
   +----------------+-----------+--------+---------------+
   |format string   |starting at|    size|cumulative size|
   +----------------+-----------+--------+---------------+
   |"/"             |         0 |      1 |             1 |
   |"%s" on 'tmpdir'|         1 | 0 - 79 |        1 - 80 |
   |"/"             |    1 - 80 |      1 |        2 - 81 |
   |"%s" on 'fname' |    2 - 81 |  0 - 7 |        2 - 88 |
   |"-"             |    2 - 88 |      1 |        3 - 89 |
   |"%i" on 'num'   |    3 - 89 |  1- 16 |       4 - 105 |
   |".tmp"          |   4 - 105 |      4 |       8 - 109 |
   |NUL terminator  |   8 - 109 |      1 |       9 - 110 |
   +----------------+-----------+--------+---------------+

could be useful (but I would argue that the column 'starting at' is redundant
and that there should be a column 'argument' that gives the numerical number of
the argument for directives, because the argument may be something much much
longer than 'num' or even something that we cannot pretty-print).

In any case, I would not over-complicate the default diagnostics with many
details. Just have the option to generate the table under some -f option.

Reply via email to