public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jeff Law <law@redhat.com>
To: gcc-patches <gcc-patches@gcc.gnu.org>
Subject: [PR middle-end/82123] 00/06 Use EVRP range data in sprintf warnings
Date: Tue, 20 Feb 2018 18:48:00 -0000	[thread overview]
Message-ID: <e8fcd96e-e3f8-c822-6434-7614804ce670@redhat.com> (raw)

So I'm finally getting back to this.

To recap, the issue here is the sprintf pass is querying global range
information.  As a result the range for the key object is not narrowed
by the conditionals leading to the sprintf call and we get a false
positive (pr81592 and pr82123).

The plan for the last few months was to use the embeddable range
analyzer to get the narrowed range.  I'd dropped in a bit of
infrastructure to do that a while back, but got side tracked by spectre
and meltdown before I could push it to completion.

This patch completes the work.  It ties the range analyzer into the
dominator walk (which was trivial) and arranges to query the range
analyzer rather than the global data.

The patch is bigger than one might expect primarily because the points
where we want to issue the queries are in free functions rather than in
member functions.

Rather than go through a round of refactoring to bring those free
functions into the class hierarchy, I just passed around the range data.
 It's the same trivial change in a bunch of places to pass vr_values
down.  So it looks big, but is dead simple in reality.

We query the data in 3 places in the fairly obvious way.  My initial
patch actually created a get_range_info member function within vr_values
that had the same signature as the free function get_range_info  in
tree-ssanames.c

That works, but ultimately I decided it was more confusing than using
the existing get_value_range member function.  So this version uses the
existing get_value_range member function.

The fix is broken into a half-dozen patches in my local tree.  I didn't
see any value in squashing them together -- so I'm posting them as a 6
series kit, even though they're pretty simple.

Bootstrapped and regression tested on x86_64-linux-gnu.  Verified it
fixes both 81592 and 82123.

Jeff

             reply	other threads:[~2018-02-20 18:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-20 18:48 Jeff Law [this message]
2018-02-21  0:00 ` Joseph Myers
2018-02-21  2:38   ` 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=e8fcd96e-e3f8-c822-6434-7614804ce670@redhat.com \
    --to=law@redhat.com \
    --cc=gcc-patches@gcc.gnu.org \
    /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).