* [PATCH] Fix valgrind checking bootstrap with PCH (PR middle-end/56461)
@ 2013-03-01 20:16 Jakub Jelinek
2013-03-04 12:31 ` Jakub Jelinek
0 siblings, 1 reply; 2+ messages in thread
From: Jakub Jelinek @ 2013-03-01 20:16 UTC (permalink / raw)
To: gcc-patches
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
^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH] Fix valgrind checking bootstrap with PCH (PR middle-end/56461)
2013-03-01 20:16 [PATCH] Fix valgrind checking bootstrap with PCH (PR middle-end/56461) Jakub Jelinek
@ 2013-03-04 12:31 ` Jakub Jelinek
0 siblings, 0 replies; 2+ messages in thread
From: Jakub Jelinek @ 2013-03-04 12:31 UTC (permalink / raw)
To: gcc-patches
On Fri, Mar 01, 2013 at 09:16:16PM +0100, Jakub Jelinek wrote:
> 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).
FYI, with this patch (first time I remember) the valgrind checking
bootstrap actually succeeded on x86_64-linux.
../configure --enable-languages=all,obj-c++,lto --enable-checking=yes,valgrind
make -j16 STAGE1_CFLAGS='-g -O2' STAGE1_CXXFLAGS='-g -O2' > LOG 2>&1 && make -j16 -k check > LOGC 2>&1; ../contrib/test_summary > LOGT 2>&1
Bootstrap took a few minutes under 24 hours, regtest a few minutes under 40
hours.
During regtesting some failures were seen (smaller amount of them
timeouts because I haven't increased the timeouts, bigger amounts because of
various valgrind errors, I'll go through them soon).
Compared to non-valgrind checking bootstrap the difference was:
=== gcc Summary ===
-# of expected passes 95837
-# of unexpected failures 32
+# of expected passes 94093
+# of unexpected failures 287
# of unexpected successes 44
# of expected failures 266
+# of unresolved testcases 26
# of unsupported tests 1398
=== gfortran Summary ===
-# of expected passes 43273
-# of unexpected failures 6
+# of expected passes 43227
+# of unexpected failures 48
# of unexpected successes 2
# of expected failures 48
-# of unresolved testcases 6
+# of unresolved testcases 10
# of unsupported tests 61
=== g++ Summary ===
-# of expected passes 53896
+# of expected passes 52584
+# of unexpected failures 84
# of expected failures 290
+# of unresolved testcases 1
# of unsupported tests 832
=== objc Summary ===
-# of expected passes 2988
+# of expected passes 2985
+# of unexpected failures 3
# of expected failures 6
# of unsupported tests 74
=== libstdc++ Summary ===
-# of expected passes 9246
+# of expected passes 9237
+# of unexpected failures 7
# of expected failures 45
+# of unresolved testcases 2
# of unsupported tests 212
> 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.
Jakub
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2013-03-04 12:31 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-01 20:16 [PATCH] Fix valgrind checking bootstrap with PCH (PR middle-end/56461) Jakub Jelinek
2013-03-04 12:31 ` Jakub Jelinek
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).