public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: gchare@google.com (Gabriel Charette)
To: reply@codereview.appspotmail.com, crowl@google.com,
	dnovillo@google.com,        gcc-patches@gcc.gnu.org
Subject: [pph] Initialize cache_ix in all paths in pph_start_record (issue4642045)
Date: Fri, 17 Jun 2011 23:18:00 -0000	[thread overview]
Message-ID: <20110617224918.BA4771C1E31@gchare.mtv.corp.google.com> (raw)

Read the comment in the diff for all the details.

I found this while working on my current patch, adding some assertion at the
end of the function would create a new build error and moving it around in the
function would stop showing the error.

I discussed this with Collin and Jeff and it appears that the compiler
changes it's inlining decisions based on very picky details. In this particular
case, inlining the function resulted in an error.

Tested with bootstrap build and pph regression testing.

2011-06-17  Gabriel Charette  <gchare@google.com>

	* gcc/cp/pph-streamer-in.c (pph_start_record):
	Initialize cache_ix in all paths.

diff --git a/gcc/cp/pph-streamer-in.c b/gcc/cp/pph-streamer-in.c
index b3c2ac9..186100f 100644
--- a/gcc/cp/pph-streamer-in.c
+++ b/gcc/cp/pph-streamer-in.c
@@ -204,7 +204,15 @@ pph_start_record (pph_stream *stream, unsigned *cache_ix)
   if (marker == PPH_RECORD_START || marker == PPH_RECORD_SHARED)
     *cache_ix = pph_in_uint (stream);
   else
-    gcc_assert (marker == PPH_RECORD_END);
+    {
+      gcc_assert (marker == PPH_RECORD_END);
+      /* Initialize CACHE_IX to an invalid index. Even though this
+	 is never used in practice, the compiler will throw an error
+	 if the optimizer inlines this function and discards the asserts
+	 in a given build as it will complain that " 'ix' may be used
+	 unititialized".  */
+      *cache_ix = -1;
+    }
 
   return marker;
 }

--
This patch is available for review at http://codereview.appspot.com/4642045

             reply	other threads:[~2011-06-17 22:49 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-06-17 23:18 Gabriel Charette [this message]
2011-06-18  2:49 Gabriel Charette
2011-06-22 19:10 dnovillo

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=20110617224918.BA4771C1E31@gchare.mtv.corp.google.com \
    --to=gchare@google.com \
    --cc=crowl@google.com \
    --cc=dnovillo@google.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=reply@codereview.appspotmail.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).