public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Jakub Jelinek <jakub@redhat.com>
To: gcc-patches@gcc.gnu.org
Subject: [PATCH] Fix valgrind checking bootstrap with PCH (PR middle-end/56461)
Date: Fri, 01 Mar 2013 20:16:00 -0000	[thread overview]
Message-ID: <20130301201616.GG12913@tucnak.redhat.com> (raw)

Hi!

When I've tried --enable-checking=valgrind bootstrap recently, it failed
as soon as compiling first PCH header.  The problem is that in
ggc_internal_alloc_stat we often increase the requested size through
ggc_round_alloc_size_1 (size, &order, &object_size);
and there is no way to query the actual size, so for PCH saving all we
know is the larger object_size.  From size to object_size the memory
is marked as unaccessible for valgrind though:
  /* Make the bytes after the end of the object unaccessible.  Discard the
     handle to avoid handle leak.  */
  VALGRIND_DISCARD (VALGRIND_MAKE_MEM_NOACCESS ((char *) result + size,
                                                object_size - size));
thus when we're trying to save it into the PCH file (sure, we'll store
there usually the 0xaf bytes), we hit tons of valgrind errors.

Fixed by making the memory accessible again (and fully defined) for the
actual writing to PCH, and then restoring the previous state.

Also, I've plugged two memory leaks in PCH writing.

Bootstrapped/regtested on x86_64-linux and i686-linux, tested also with
valgrind checking on various testcases (full --enable-checking=yes,valgrind
bootstrap queued for the weekend).

Ok for trunk?

2013-03-01  Jakub Jelinek  <jakub@redhat.com>

	PR middle-end/56461
	* ggc-common.c (gt_pch_save): For ENABLE_VALGRIND_CHECKING,
	if VALGRIND_GET_VBITS is defined, temporarily make object
	memory all defined, and restore previous valgrind addressability
	and definability afterwards.  Free this_object at the end.

	* c-pch.c (pch_init): Free target_validity at the end.

--- gcc/ggc-common.c.jj	2013-01-24 17:04:30.000000000 +0100
+++ gcc/ggc-common.c	2013-03-01 16:32:24.547261238 +0100
@@ -561,6 +561,10 @@ gt_pch_save (FILE *f)
 
   ggc_pch_prepare_write (state.d, state.f);
 
+#if defined ENABLE_VALGRIND_CHECKING && defined VALGRIND_GET_VBITS
+  vec<char> vbits = vNULL;
+#endif
+
   /* Actually write out the objects.  */
   for (i = 0; i < state.count; i++)
     {
@@ -569,6 +573,50 @@ gt_pch_save (FILE *f)
 	  this_object_size = state.ptrs[i]->size;
 	  this_object = XRESIZEVAR (char, this_object, this_object_size);
 	}
+#if defined ENABLE_VALGRIND_CHECKING && defined VALGRIND_GET_VBITS
+      /* obj might contain uninitialized bytes, e.g. in the trailing
+	 padding of the object.  Avoid warnings by making the memory
+	 temporarily defined and then restoring previous state.  */
+      int get_vbits = 0;
+      size_t valid_size = state.ptrs[i]->size;
+      if (__builtin_expect (RUNNING_ON_VALGRIND, 0))
+	{
+	  if (vbits.length () < valid_size)
+	    vbits.safe_grow (valid_size);
+	  get_vbits = VALGRIND_GET_VBITS (state.ptrs[i]->obj,
+					  vbits.address (), valid_size);
+	  if (get_vbits == 3)
+	    {
+	      /* We assume that first part of obj is addressable, and
+		 the rest is unaddressable.  Find out where the boundary is
+		 using binary search.  */
+	      size_t lo = 0, hi = valid_size;
+	      while (hi > lo)
+		{
+		  size_t mid = (lo + hi) / 2;
+		  get_vbits = VALGRIND_GET_VBITS ((char *) state.ptrs[i]->obj
+						  + mid, vbits.address (),
+						  1);
+		  if (get_vbits == 3)
+		    hi = mid;
+		  else if (get_vbits == 1)
+		    lo = mid + 1;
+		  else
+		    break;
+		}
+	      if (get_vbits == 1 || get_vbits == 3)
+		{
+		  valid_size = lo;
+		  get_vbits = VALGRIND_GET_VBITS (state.ptrs[i]->obj,
+						  vbits.address (),
+						  valid_size);
+		}
+	    }
+	  if (get_vbits == 1)
+	    VALGRIND_DISCARD (VALGRIND_MAKE_MEM_DEFINED (state.ptrs[i]->obj,
+							 state.ptrs[i]->size));
+	}
+#endif
       memcpy (this_object, state.ptrs[i]->obj, state.ptrs[i]->size);
       if (state.ptrs[i]->reorder_fn != NULL)
 	state.ptrs[i]->reorder_fn (state.ptrs[i]->obj,
@@ -582,11 +630,29 @@ gt_pch_save (FILE *f)
 			    state.ptrs[i]->note_ptr_fn == gt_pch_p_S);
       if (state.ptrs[i]->note_ptr_fn != gt_pch_p_S)
 	memcpy (state.ptrs[i]->obj, this_object, state.ptrs[i]->size);
+#if defined ENABLE_VALGRIND_CHECKING && defined VALGRIND_GET_VBITS
+      if (__builtin_expect (get_vbits == 1, 0))
+	{
+	  (void) VALGRIND_SET_VBITS (state.ptrs[i]->obj, vbits.address (),
+				     valid_size);
+	  if (valid_size != state.ptrs[i]->size)
+	    VALGRIND_DISCARD (VALGRIND_MAKE_MEM_NOACCESS ((char *)
+							  state.ptrs[i]->obj
+							  + valid_size,
+							  state.ptrs[i]->size
+							  - valid_size));
+	}
+#endif
     }
+#if defined ENABLE_VALGRIND_CHECKING && defined VALGRIND_GET_VBITS
+  vbits.release ();
+#endif
+
   ggc_pch_finish (state.d, state.f);
   gt_pch_fixup_stringpool ();
 
-  free (state.ptrs);
+  XDELETE (state.ptrs);
+  XDELETE (this_object);
   htab_delete (saving_htab);
 }
 
--- gcc/c-family/c-pch.c.jj	2013-02-13 23:48:14.000000000 +0100
+++ gcc/c-family/c-pch.c	2013-03-01 16:29:54.001422781 +0100
@@ -141,6 +141,8 @@ pch_init (void)
 
   if (pch_ready_to_save_cpp_state)
     pch_cpp_save_state ();
+
+  XDELETE (target_validity);
 }
 
 /* Whether preprocessor state has been saved in a PCH file.  */

	Jakub

             reply	other threads:[~2013-03-01 20:16 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-01 20:16 Jakub Jelinek [this message]
2013-03-04 12:31 ` Jakub Jelinek

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=20130301201616.GG12913@tucnak.redhat.com \
    --to=jakub@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).