From: Jakub Jelinek <jakub@redhat.com>
To: Martin Sebor <msebor@gmail.com>
Cc: Gcc Patch List <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] handle integer overflow/wrapping in printf directives (PR 78622)
Date: Thu, 01 Dec 2016 07:26:00 -0000 [thread overview]
Message-ID: <20161201072647.GF3541@tucnak.redhat.com> (raw)
In-Reply-To: <dfa942c1-cdc2-fea5-fe49-543d01b19983@gmail.com>
On Wed, Nov 30, 2016 at 08:26:04PM -0700, Martin Sebor wrote:
> @@ -795,6 +795,43 @@ get_width_and_precision (const conversion_spec &spec,
> *pprec = prec;
> }
>
> +/* With the range [*ARGMIN, *ARGMAX] of an integer directive's actual
> + argument, due to the conversion from either *ARGMIN or *ARGMAX to
> + the type of the directive's formal argument it's possible for both
> + to result in the same number of bytes or a range of bytes that's
> + less than the number of bytes that would result from formatting
> + some other value in the range [*ARGMIN, *ARGMAX]. This can be
> + determined by checking for the actual argument being in the range
> + of the type of the directive. If it isn't it must be assumed to
> + take on the full range of the directive's type.
> + Return true when the range has been adjusted, false otherwise. */
> +
> +static bool
> +adjust_range_for_overflow (tree dirtype, tree *argmin, tree *argmax)
> +{
> + tree dirmin = TYPE_MIN_VALUE (dirtype);
> + tree dirmax = TYPE_MAX_VALUE (dirtype);
> +
> + if (tree_int_cst_lt (*argmin, dirmin)
> + || tree_int_cst_lt (dirmax, *argmin)
> + || tree_int_cst_lt (*argmax, dirmin)
> + || tree_int_cst_lt (dirmax, *argmax))
> + {
> + if (TYPE_UNSIGNED (dirtype))
> + {
> + *argmin = dirmin;
> + *argmax = dirmax;
> + }
> + else
> + {
> + *argmin = integer_zero_node;
> + *argmax = dirmin;
> + }
> + return true;
> + }
Isn't this too simplistic? I mean, if you have say dirtype of signed char
and argmin say 4096 + 32 and argmax say 4096 + 64, (signed char) arg
has range 32, 64, while I think your routine will yield -128, 127 (well,
0 as min and -128 as max as that is what you return for signed type).
Can't you subtract argmax - argmin (best just in wide_int, no need to build
trees), and use what you have just for the case where that number doesn't
fit into the narrower precision, otherwise if argmin - (dirtype) argmin
== argmax - (dirtype) argmax, just use (dirtype) argmin and (dirtype) argmax
as the range, and in case that it crosses a boundary figure if you can do
anything than the above? Guess all cases of signed/unsigned dirtype and/or
argtype need to be considered.
Also, is argmin and argmax in this case the actual range (what should go
into res.arg{min,max}), or the values with shortest/longest representation?
Wouldn't it be better to always compute the range of values that can be
printed and only later on (after all VR_RANGE and VR_VARYING handling)
transform that into the number with shortest/longest representation in that
range? Perhaps even using different variable names for the latter would
make things clearer (argshortest, arglongest or whatever).
Jakub
next prev parent reply other threads:[~2016-12-01 7:26 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-12-01 3:26 Martin Sebor
2016-12-01 7:26 ` Jakub Jelinek [this message]
2016-12-01 9:15 ` Jakub Jelinek
2016-12-02 2:31 ` Martin Sebor
2016-12-02 20:52 ` Jeff Law
2016-12-02 22:54 ` Martin Sebor
2016-12-07 18:43 ` Jeff Law
2016-12-07 19:18 ` Martin Sebor
2016-12-12 18:18 ` Jeff Law
2016-12-05 20:26 ` Jakub Jelinek
2016-12-06 3:43 ` Martin Sebor
2016-12-07 18:58 ` Jeff Law
2016-12-12 0:21 ` Martin Sebor
2016-12-12 20:28 ` Jeff Law
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20161201072647.GF3541@tucnak.redhat.com \
--to=jakub@redhat.com \
--cc=gcc-patches@gcc.gnu.org \
--cc=msebor@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).