You can subscribe to this list here.
| 2002 |
Jan
|
Feb
|
Mar
|
Apr
|
May
|
Jun
|
Jul
|
Aug
|
Sep
(1) |
Oct
(122) |
Nov
(152) |
Dec
(69) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| 2003 |
Jan
(6) |
Feb
(25) |
Mar
(73) |
Apr
(82) |
May
(24) |
Jun
(25) |
Jul
(10) |
Aug
(11) |
Sep
(10) |
Oct
(54) |
Nov
(203) |
Dec
(182) |
| 2004 |
Jan
(307) |
Feb
(305) |
Mar
(430) |
Apr
(312) |
May
(187) |
Jun
(342) |
Jul
(487) |
Aug
(637) |
Sep
(336) |
Oct
(373) |
Nov
(441) |
Dec
(210) |
| 2005 |
Jan
(385) |
Feb
(480) |
Mar
(636) |
Apr
(544) |
May
(679) |
Jun
(625) |
Jul
(810) |
Aug
(838) |
Sep
(634) |
Oct
(521) |
Nov
(965) |
Dec
(543) |
| 2006 |
Jan
(494) |
Feb
(431) |
Mar
(546) |
Apr
(411) |
May
(406) |
Jun
(322) |
Jul
(256) |
Aug
(401) |
Sep
(345) |
Oct
(542) |
Nov
(308) |
Dec
(481) |
| 2007 |
Jan
(427) |
Feb
(326) |
Mar
(367) |
Apr
(255) |
May
(244) |
Jun
(204) |
Jul
(223) |
Aug
(231) |
Sep
(354) |
Oct
(374) |
Nov
(497) |
Dec
(362) |
| 2008 |
Jan
(322) |
Feb
(482) |
Mar
(658) |
Apr
(422) |
May
(476) |
Jun
(396) |
Jul
(455) |
Aug
(267) |
Sep
(280) |
Oct
(253) |
Nov
(232) |
Dec
(304) |
| 2009 |
Jan
(486) |
Feb
(470) |
Mar
(458) |
Apr
(423) |
May
(696) |
Jun
(461) |
Jul
(551) |
Aug
(575) |
Sep
(134) |
Oct
(110) |
Nov
(157) |
Dec
(102) |
| 2010 |
Jan
(226) |
Feb
(86) |
Mar
(147) |
Apr
(117) |
May
(107) |
Jun
(203) |
Jul
(193) |
Aug
(238) |
Sep
(300) |
Oct
(246) |
Nov
(23) |
Dec
(75) |
| 2011 |
Jan
(133) |
Feb
(195) |
Mar
(315) |
Apr
(200) |
May
(267) |
Jun
(293) |
Jul
(353) |
Aug
(237) |
Sep
(278) |
Oct
(611) |
Nov
(274) |
Dec
(260) |
| 2012 |
Jan
(303) |
Feb
(391) |
Mar
(417) |
Apr
(441) |
May
(488) |
Jun
(655) |
Jul
(590) |
Aug
(610) |
Sep
(526) |
Oct
(478) |
Nov
(359) |
Dec
(372) |
| 2013 |
Jan
(467) |
Feb
(226) |
Mar
(391) |
Apr
(281) |
May
(299) |
Jun
(252) |
Jul
(311) |
Aug
(352) |
Sep
(481) |
Oct
(571) |
Nov
(222) |
Dec
(231) |
| 2014 |
Jan
(185) |
Feb
(329) |
Mar
(245) |
Apr
(238) |
May
(281) |
Jun
(399) |
Jul
(382) |
Aug
(500) |
Sep
(579) |
Oct
(435) |
Nov
(487) |
Dec
(256) |
| 2015 |
Jan
(338) |
Feb
(357) |
Mar
(330) |
Apr
(294) |
May
(191) |
Jun
(108) |
Jul
(142) |
Aug
(261) |
Sep
(190) |
Oct
(54) |
Nov
(83) |
Dec
(22) |
| 2016 |
Jan
(49) |
Feb
(89) |
Mar
(33) |
Apr
(50) |
May
(27) |
Jun
(34) |
Jul
(53) |
Aug
(53) |
Sep
(98) |
Oct
(206) |
Nov
(93) |
Dec
(53) |
| 2017 |
Jan
(65) |
Feb
(82) |
Mar
(102) |
Apr
(86) |
May
(187) |
Jun
(67) |
Jul
(23) |
Aug
(93) |
Sep
(65) |
Oct
(45) |
Nov
(35) |
Dec
(17) |
| 2018 |
Jan
(26) |
Feb
(35) |
Mar
(38) |
Apr
(32) |
May
(8) |
Jun
(43) |
Jul
(27) |
Aug
(30) |
Sep
(43) |
Oct
(42) |
Nov
(38) |
Dec
(67) |
| 2019 |
Jan
(32) |
Feb
(37) |
Mar
(53) |
Apr
(64) |
May
(49) |
Jun
(18) |
Jul
(14) |
Aug
(53) |
Sep
(25) |
Oct
(30) |
Nov
(49) |
Dec
(31) |
| 2020 |
Jan
(87) |
Feb
(45) |
Mar
(37) |
Apr
(51) |
May
(99) |
Jun
(36) |
Jul
(11) |
Aug
(14) |
Sep
(20) |
Oct
(24) |
Nov
(40) |
Dec
(23) |
| 2021 |
Jan
(14) |
Feb
(53) |
Mar
(85) |
Apr
(15) |
May
(19) |
Jun
(3) |
Jul
(14) |
Aug
(1) |
Sep
(57) |
Oct
(73) |
Nov
(56) |
Dec
(22) |
| 2022 |
Jan
(3) |
Feb
(22) |
Mar
(6) |
Apr
(55) |
May
(46) |
Jun
(39) |
Jul
(15) |
Aug
(9) |
Sep
(11) |
Oct
(34) |
Nov
(20) |
Dec
(36) |
| 2023 |
Jan
(79) |
Feb
(41) |
Mar
(99) |
Apr
(169) |
May
(48) |
Jun
(16) |
Jul
(16) |
Aug
(57) |
Sep
(19) |
Oct
|
Nov
|
Dec
|
| S | M | T | W | T | F | S |
|---|---|---|---|---|---|---|
|
1
(21) |
2
(18) |
3
(19) |
4
(16) |
5
(20) |
6
(22) |
7
(17) |
|
8
(13) |
9
(1) |
10
(3) |
11
(28) |
12
(13) |
13
(12) |
14
(25) |
|
15
(15) |
16
(29) |
17
(19) |
18
(15) |
19
(27) |
20
(29) |
21
(21) |
|
22
(16) |
23
(24) |
24
(18) |
25
(26) |
26
(27) |
27
(21) |
28
(30) |
|
29
(23) |
30
(3) |
31
(19) |
|
|
|
|
|
From: <sv...@va...> - 2012-07-18 23:01:09
|
philippe 2012-07-19 00:01:02 +0100 (Thu, 19 Jul 2012)
New Revision: 12758
Log:
Fix 303624 segmentation fault on Android 4.1 (e.g. on android emulator or Galaxy Nexus OMAP)
Valgrind was crashing systematically on Android 4.1.
This crash is caused by AT_IGNORE-ing AT_BASE.
This AT_IGNORE was needed to have breakpoints in shared libs
be handled properly (not very clear what is the problem
in the interaction between Valgrind GDBSERVER, AT_BASE and GDB).
Waiting to better understand all this, as a temporary bypass,
this patch ensures we do not ignore the AT_BASE on android.
The possible consequence is that breakpoints might be inserted
by the Valgrind gdbserver at wrong addresses in shared lib.
(any feedback on that is welcome).
Valgrind was build and then "proved" to work on Android emulator 4.0
and emulator 4.1, by using memcheck on one executable.
Modified files:
trunk/NEWS
trunk/README.android
trunk/README.android_emulator
trunk/coregrind/m_initimg/initimg-linux.c
Modified: trunk/README.android_emulator (+0 -2)
===================================================================
--- trunk/README.android_emulator 2012-07-18 23:36:37 +01:00 (rev 12757)
+++ trunk/README.android_emulator 2012-07-19 00:01:02 -23:00 (rev 12758)
@@ -42,8 +42,6 @@
# Android sdk 20
# Android platform tools 12
-# Android 4.1 (API 16) does not work.
-
# then define a virtual device:
Tools -> Manage AVDs...
# I define an AVD Name with 64 Mb SD Card, (4.0.3, api 15)
Modified: trunk/coregrind/m_initimg/initimg-linux.c (+6 -1)
===================================================================
--- trunk/coregrind/m_initimg/initimg-linux.c 2012-07-18 23:36:37 +01:00 (rev 12757)
+++ trunk/coregrind/m_initimg/initimg-linux.c 2012-07-19 00:01:02 -23:00 (rev 12758)
@@ -668,8 +668,13 @@
/* When gdbserver sends the auxv to gdb, the AT_BASE has
to be ignored, as otherwise gdb adds this offset
to loaded shared libs, causing wrong address
- relocation e.g. when inserting breaks. */
+ relocation e.g. when inserting breaks.
+ However, ignoring AT_BASE makes V crash on Android 4.1.
+ So, keep the AT_BASE on android for now.
+ ??? Need to dig in depth about AT_BASE/GDB interaction */
+# if !defined(VGPV_arm_linux_android)
auxv->a_type = AT_IGNORE;
+# endif
auxv->u.a_val = info->interp_base;
break;
Modified: trunk/README.android (+4 -1)
===================================================================
--- trunk/README.android 2012-07-18 23:36:37 +01:00 (rev 12757)
+++ trunk/README.android 2012-07-19 00:01:02 -23:00 (rev 12758)
@@ -6,10 +6,13 @@
Android 4.0.3 running on a (rooted, AOSP build) Nexus S.
Android 4.0.3 running on Motorola Xoom.
Android 4.0.3 running on android emulator.
+ Android 4.1 running on android emulator.
Android 2.3.4 on Nexus S worked at some time in the past.
-It is known not to work on Android 4.1 running on android emulator.
+On android, GDBserver might insert breaks at wrong addresses.
+Feedback on this welcome.
+
Other configurations and toolchains might work, but haven't been tested.
Feedback is welcome.
Modified: trunk/NEWS (+1 -1)
===================================================================
--- trunk/NEWS 2012-07-18 23:36:37 +01:00 (rev 12757)
+++ trunk/NEWS 2012-07-19 00:01:02 -23:00 (rev 12758)
@@ -251,8 +251,8 @@
303127 Power test suite fixes for frsqrte, vrefp, and vrsqrtefp instructions.
303250 "Assertion `instrs_in->arr_used <= 10000' failed" on
OpenSSL with --track-origins=yes
+303624 segmentation fault on Android 4.1 (e.g. on android emulator or Galaxy Nexus OMAP)
-
Release 3.7.0 (5 November 2011)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
3.7.0 is a feature release with many significant improvements and the
|
|
From: Julian S. <js...@ac...> - 2012-07-18 22:46:30
|
> attached are three patches [...] These look OK to me. > I've reg-tested the patches on x86-64, ppc64, and s390x with no new > regressions. Thanks for the extensive testing. Sorry for slow review. J |
|
From: <sv...@va...> - 2012-07-18 22:36:43
|
philippe 2012-07-18 23:36:37 +0100 (Wed, 18 Jul 2012)
New Revision: 12757
Log:
Be slightly less precise on the leak check perf. improvement in NEWS
(as this 40% is only a measurement of one perf program on amd64).
Modified files:
trunk/NEWS
Modified: trunk/NEWS (+1 -1)
===================================================================
--- trunk/NEWS 2012-07-18 23:26:51 +01:00 (rev 12756)
+++ trunk/NEWS 2012-07-18 23:36:37 +01:00 (rev 12757)
@@ -43,7 +43,7 @@
a program using statically linked malloc or using alternative
malloc libraries (such as tcmalloc).
- - Performance of leak check has been improved by 40%.
+ - Performance of leak check has been improved.
* DRD:
|
|
From: <sv...@va...> - 2012-07-18 22:26:59
|
philippe 2012-07-18 23:26:51 +0100 (Wed, 18 Jul 2012)
New Revision: 12756
Log:
patch that improves the speed of the leak search by up to 40% (on amd64)
Scanning 1GB of random values made of 200_000 malloc-ed block is
(on amd64) changing from (about) 17 seconds to (about) 10 seconds.
On x86, it goes from 153 seconds to 129 seconds.
(this huge difference between x86 and amd64 leak search time
for this random test is because a random value has about one chance
on 4 to be in the addressable memory on x86 and so the dichotomic
search in the list of malloc-ed blocks is called for a lot more
values than on amd64).
Basically, there are 3 optimisations:
1. call MC_(is_within_valid_secondary) only at the beginning of a
secondary map (and not for each Word).
2. call SETJMP only at the beginning of a page (and not for each Word)
3. Validate an aligned word using get_vabits8 rather than get_vabits2.
Each of the above optimisation more or less improves by 2 seconds.
(to go from 17 seconds to 10 seconds).
Modified files:
trunk/NEWS
trunk/memcheck/mc_leakcheck.c
trunk/memcheck/mc_main.c
Modified: trunk/memcheck/mc_main.c (+8 -4)
===================================================================
--- trunk/memcheck/mc_main.c 2012-07-18 21:33:40 +01:00 (rev 12755)
+++ trunk/memcheck/mc_main.c 2012-07-18 23:26:51 +01:00 (rev 12756)
@@ -4659,12 +4659,16 @@
{
tl_assert(sizeof(UWord) == 4 || sizeof(UWord) == 8);
tl_assert(VG_IS_WORD_ALIGNED(a));
- if (is_mem_defined( a, sizeof(UWord), NULL, NULL) == MC_Ok
- && !MC_(in_ignored_range)(a)) {
- return True;
- } else {
+ if (get_vabits8_for_aligned_word32 (a) != VA_BITS8_DEFINED)
return False;
+ if (sizeof(UWord) == 8) {
+ if (get_vabits8_for_aligned_word32 (a + 4) != VA_BITS8_DEFINED)
+ return False;
}
+ if (UNLIKELY(MC_(in_ignored_range)(a)))
+ return False;
+ else
+ return True;
}
Modified: trunk/memcheck/mc_leakcheck.c (+85 -34)
===================================================================
--- trunk/memcheck/mc_leakcheck.c 2012-07-18 21:33:40 +01:00 (rev 12755)
+++ trunk/memcheck/mc_leakcheck.c 2012-07-18 23:26:51 +01:00 (rev 12756)
@@ -502,7 +502,10 @@
MC_Chunk* ch;
LC_Extra* ex;
- // Quick filter.
+ // Quick filter. Note: implemented with am, not with get_vabits2
+ // as ptr might be random data pointing anywhere. On 64 bit
+ // platforms, getting va bits for random data can be quite costly
+ // due to the secondary map.
if (!VG_(am_is_valid_for_client)(ptr, 1, VKI_PROT_READ)) {
return False;
} else {
@@ -705,6 +708,11 @@
lc_scan_memory(Addr start, SizeT len, Bool is_prior_definite, Int clique, Int cur_clique,
Addr searched, SizeT szB)
{
+ /* memory scan is based on the assumption that valid pointers are aligned
+ on a multiple of sizeof(Addr). So, we can (and must) skip the begin and
+ end portions of the block if they are not aligned on sizeof(Addr):
+ These cannot be a valid pointer, and calls to MC_(is_valid_aligned_word)
+ will assert for a non aligned address. */
Addr ptr = VG_ROUNDUP(start, sizeof(Addr));
Addr end = VG_ROUNDDN(start+len, sizeof(Addr));
vki_sigset_t sigmask;
@@ -715,57 +723,90 @@
VG_(sigprocmask)(VKI_SIG_SETMASK, NULL, &sigmask);
VG_(set_fault_catcher)(scan_all_valid_memory_catcher);
- // We might be in the middle of a page. Do a cheap check to see if
- // it's valid; if not, skip onto the next page.
- if (!VG_(am_is_valid_for_client)(ptr, sizeof(Addr), VKI_PROT_READ))
+ /* Optimisation: the loop below will check for each begin
+ of SM chunk if the chunk is fully unaddressable. The idea is to
+ skip efficiently such fully unaddressable SM chunks.
+ So, we preferrably start the loop on a chunk boundary.
+ If the chunk is not fully unaddressable, we might be in
+ an unaddressable page. Again, the idea is to skip efficiently
+ such unaddressable page : this is the "else" part.
+ We use an "else" so that two consecutive fully unaddressable
+ SM chunks will be skipped efficiently: first one is skipped
+ by this piece of code. The next SM chunk will be skipped inside
+ the loop. */
+ if ( ! MC_(is_within_valid_secondary)(ptr) ) {
+ // Skip an invalid SM chunk till the beginning of the next SM Chunk.
+ ptr = VG_ROUNDUP(ptr+1, SM_SIZE);
+ } else if (!VG_(am_is_valid_for_client)(ptr, sizeof(Addr), VKI_PROT_READ)) {
+ // else we are in a (at least partially) valid SM chunk.
+ // We might be in the middle of an unreadable page.
+ // Do a cheap check to see if it's valid;
+ // if not, skip onto the next page.
ptr = VG_PGROUNDUP(ptr+1); // First page is bad - skip it.
+ }
+ /* This optimisation and below loop is based on some relationships between
+ VKI_PAGE_SIZE, SM_SIZE and sizeof(Addr) which are asserted in
+ MC_(detect_memory_leaks). */
while (ptr < end) {
Addr addr;
// Skip invalid chunks.
- if ( ! MC_(is_within_valid_secondary)(ptr) ) {
- ptr = VG_ROUNDUP(ptr+1, SM_SIZE);
- continue;
+ if (UNLIKELY((ptr % SM_SIZE) == 0)) {
+ if (! MC_(is_within_valid_secondary)(ptr) ) {
+ ptr = VG_ROUNDUP(ptr+1, SM_SIZE);
+ continue;
+ }
}
// Look to see if this page seems reasonable.
- if ((ptr % VKI_PAGE_SIZE) == 0) {
+ if (UNLIKELY((ptr % VKI_PAGE_SIZE) == 0)) {
if (!VG_(am_is_valid_for_client)(ptr, sizeof(Addr), VKI_PROT_READ)) {
ptr += VKI_PAGE_SIZE; // Bad page - skip it.
continue;
}
+ // aspacemgr indicates the page is readable and belongs to client.
+ // We still probe the page explicitely in case aspacemgr is
+ // desynchronised with the real page mappings.
+ // Such a desynchronisation can happen due to an aspacemgr bug.
+ // Note that if the application is using mprotect(NONE), then
+ // a page can be unreadable but have addressable and defined
+ // VA bits (see mc_main.c function mc_new_mem_mprotect).
+ if (VG_MINIMAL_SETJMP(memscan_jmpbuf) == 0) {
+ // Try a read in the beginning of the page ...
+ Addr test = *(volatile Addr *)ptr;
+ __asm__ __volatile__("": :"r"(test) : "cc","memory");
+ } else {
+ // Catch read error ...
+ // We need to restore the signal mask, because we were
+ // longjmped out of a signal handler.
+ VG_(sigprocmask)(VKI_SIG_SETMASK, &sigmask, NULL);
+ ptr += VKI_PAGE_SIZE; // Bad page - skip it.
+ continue;
+ }
}
- if (VG_MINIMAL_SETJMP(memscan_jmpbuf) == 0) {
- if ( MC_(is_valid_aligned_word)(ptr) ) {
- lc_scanned_szB += sizeof(Addr);
- addr = *(Addr *)ptr;
- // If we get here, the scanned word is in valid memory. Now
- // let's see if its contents point to a chunk.
- if (searched) {
- if (addr >= searched && addr < searched + szB) {
- if (addr == searched)
- VG_(umsg)("*%#lx points at %#lx\n", ptr, searched);
- else
- VG_(umsg)("*%#lx interior points at %lu bytes inside %#lx\n",
- ptr, (long unsigned) addr - searched, searched);
- MC_(pp_describe_addr) (ptr);
- }
- } else {
- lc_push_if_a_chunk_ptr(addr, clique, cur_clique, is_prior_definite);
+ if ( MC_(is_valid_aligned_word)(ptr) ) {
+ lc_scanned_szB += sizeof(Addr);
+ addr = *(Addr *)ptr;
+ // If we get here, the scanned word is in valid memory. Now
+ // let's see if its contents point to a chunk.
+ if (UNLIKELY(searched)) {
+ if (addr >= searched && addr < searched + szB) {
+ if (addr == searched)
+ VG_(umsg)("*%#lx points at %#lx\n", ptr, searched);
+ else
+ VG_(umsg)("*%#lx interior points at %lu bytes inside %#lx\n",
+ ptr, (long unsigned) addr - searched, searched);
+ MC_(pp_describe_addr) (ptr);
}
- } else if (0 && VG_DEBUG_LEAKCHECK) {
- VG_(printf)("%#lx not valid\n", ptr);
+ } else {
+ lc_push_if_a_chunk_ptr(addr, clique, cur_clique, is_prior_definite);
}
- ptr += sizeof(Addr);
- } else {
- // We need to restore the signal mask, because we were
- // longjmped out of a signal handler.
- VG_(sigprocmask)(VKI_SIG_SETMASK, &sigmask, NULL);
-
- ptr = VG_PGROUNDUP(ptr+1); // Bad page - skip it.
+ } else if (0 && VG_DEBUG_LEAKCHECK) {
+ VG_(printf)("%#lx not valid\n", ptr);
}
+ ptr += sizeof(Addr);
}
VG_(sigprocmask)(VKI_SIG_SETMASK, &sigmask, NULL);
@@ -1290,6 +1331,16 @@
tl_assert(lcp->mode != LC_Off);
+ // Verify some assertions which are used in lc_scan_memory.
+ tl_assert((VKI_PAGE_SIZE % sizeof(Addr)) == 0);
+ tl_assert((SM_SIZE % sizeof(Addr)) == 0);
+ // Above two assertions are critical, while below assertion
+ // ensures that the optimisation in the loop is done in the
+ // correct order : the loop checks for (big) SM chunk skipping
+ // before checking for (smaller) page skipping.
+ tl_assert((SM_SIZE % VKI_PAGE_SIZE) == 0);
+
+
MC_(detect_memory_leaks_last_delta_mode) = lcp->deltamode;
// Get the chunks, stop if there were none.
Modified: trunk/NEWS (+2 -0)
===================================================================
--- trunk/NEWS 2012-07-18 21:33:40 +01:00 (rev 12755)
+++ trunk/NEWS 2012-07-18 23:26:51 +01:00 (rev 12756)
@@ -43,6 +43,8 @@
a program using statically linked malloc or using alternative
malloc libraries (such as tcmalloc).
+ - Performance of leak check has been improved by 40%.
+
* DRD:
- Fixed a subtle bug that could cause false positive data race reports.
|
|
From: <sv...@va...> - 2012-07-18 20:33:48
|
philippe 2012-07-18 21:33:40 +0100 (Wed, 18 Jul 2012)
New Revision: 12755
Log:
Small cleanup: use VG_TRACK (when possible) to call tool tracking functions
(spotted by Julian)
Note: there is a second occurence of call to track_post_mem_write in the
same file; but this second occurence is better done with an "if".
Modified files:
trunk/coregrind/m_gdbserver/target.c
Modified: trunk/coregrind/m_gdbserver/target.c (+2 -4)
===================================================================
--- trunk/coregrind/m_gdbserver/target.c 2012-07-18 11:47:38 +01:00 (rev 12754)
+++ trunk/coregrind/m_gdbserver/target.c 2012-07-18 21:33:40 +01:00 (rev 12755)
@@ -393,10 +393,8 @@
delta);
VG_TRACK( new_mem_stack_w_ECU, new_SP, -delta, 0 );
VG_TRACK( new_mem_stack, new_SP, -delta );
- if (VG_(tdict).track_post_mem_write) {
- VG_(tdict).track_post_mem_write( Vg_CoreClientReq, tid,
- new_SP, -delta);
- }
+ VG_TRACK( post_mem_write, Vg_CoreClientReq, tid,
+ new_SP, -delta);
}
}
}
|
|
From: Tom H. <to...@co...> - 2012-07-18 13:54:50
|
On 18/07/12 14:48, Petar Jovanovic wrote: > I do not think we need to analyze the kernel code in this case. > Existence of parameter is valid, but glibc does not use it. I was suggesting that you look at the kernel because it is not clear to me from that comment in glibc exactly what combination of flags to the futex call cause the kernel not to look at the last argument. Looking at it again I suspect that the answer is that although we have asked to wait, there is no timeout given, which presumably means it doesn't actually wait and therefore doesn't need to access the bitset? But as say, the best way to be sure we handle it correctly, and for all clients, not just glibc, is to work exactly what the kernel API is and under what circumstances the kernel will look at the final argument. Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Petar J. <mip...@gm...> - 2012-07-18 13:48:33
|
I do not think we need to analyze the kernel code in this case. Existence of parameter is valid, but glibc does not use it. Petar On Wed, Jul 18, 2012 at 2:24 PM, Tom Hughes <to...@co...> wrote: > On 18/07/12 13:10, Petar Jovanovic wrote: > > My worries have been if there could be a side effect if some other code >> actually makes the system call with VKI_FUTEX_WAIT_BITSET and with the >> 6th parameter. Guess we can not know that now. >> > > As always you need to analyse the kernel code and see under what > conditions the kernel looks at the 6th argument and then reflect that in > valgrind's checks. > > I will grant you that understanding the kernel futex code is not the > easiest thing ;-) > > Tom > > -- > Tom Hughes (to...@co...) > http://compton.nu/ > > > |
|
From: Tom H. <to...@co...> - 2012-07-18 12:24:54
|
On 18/07/12 13:10, Petar Jovanovic wrote: > My worries have been if there could be a side effect if some other code > actually makes the system call with VKI_FUTEX_WAIT_BITSET and with the > 6th parameter. Guess we can not know that now. As always you need to analyse the kernel code and see under what conditions the kernel looks at the 6th argument and then reflect that in valgrind's checks. I will grant you that understanding the kernel futex code is not the easiest thing ;-) Tom -- Tom Hughes (to...@co...) http://compton.nu/ |
|
From: Petar J. <mip...@gm...> - 2012-07-18 12:10:27
|
Yes, it is stack allocated. Here is a snippet from the log with
--track-origins=yes.
==25520== Syscall param futex(val3) contains uninitialised byte(s)
==25520== at 0x48617B8: __pthread_initialize_minimal (nptl-init.c:340)
==25520== by 0x4861328: ??? (in
/home/dejan/eglibc-exec/lib/libpthread-2.13.so)
==25520== Uninitialised value was created by a stack allocation
==25520== at 0x48617A8: __pthread_initialize_minimal (nptl-init.c:340)
==25520==
PRE_REG_READ6 is already under VKI_FUTEX_WAIT_BITSET case, so I guess that
we can change it for all platforms
from:
case VKI_FUTEX_WAIT_BITSET:
PRE_REG_READ6(long, "futex",
vki_u32 *, futex, int, op, int, val,
struct timespec *, utime, int, dummy, int, val3);
to:
case VKI_FUTEX_WAIT_BITSET:
PRE_REG_READ5(long, "futex",
vki_u32 *, futex, int, op, int, val,
struct timespec *, utime, int, val3);
My worries have been if there could be a side effect if some other code
actually makes the system call with VKI_FUTEX_WAIT_BITSET and with the 6th
parameter. Guess we can not know that now.
On Wed, Jul 18, 2012 at 1:31 PM, Julian Seward <js...@ac...> wrote:
> On Wednesday, July 18, 2012, Julian Seward wrote:
> > > if the glibc kludge you report applied to all linux targets, change to
> > > PRE_REG_READ5 for all linux targets. If the glibc kludge applies only
> > > to mips-linux, leave the PRE_REG_READ6 in place but put a _READ5
> variant
> > > which is guarded by #if defined(VGO_mips32_linux).
> >
> > On second thoughts, just change it for MIPS. To change it for all
> > platforms assumes that it is always going to be called by glibc in this
> > particular way (with garbage 6th argument), which is not necessarily
> true.
>
> Urr. Ignore all that.
>
> The problem exists because Valgrind's model of what the kernel does
> with calls to sys_futex is inaccurate -- assuming the libc comment
> is accurate. I think the right fix is, for all platforms,
> check the first 5 args (PRE_REG_READ5). Then, if this is a
> non-FUTEX_WAIT_BITSET call, check the 6th arg. That makes the checking
> sync with the glibc comment.
>
> If you do this, please also add a comment in the code explaining why
> this change has happened.
>
> J
>
|
|
From: <sv...@va...> - 2012-07-18 11:48:35
|
sewardj 2012-07-18 12:48:23 +0100 (Wed, 18 Jul 2012)
New Revision: 2438
Log:
eqIRConst: handle Ico_V256.
Modified files:
trunk/priv/ir_defs.c
Modified: trunk/priv/ir_defs.c (+1 -0)
===================================================================
--- trunk/priv/ir_defs.c 2012-07-16 15:25:05 +01:00 (rev 2437)
+++ trunk/priv/ir_defs.c 2012-07-18 12:48:23 +01:00 (rev 2438)
@@ -3876,6 +3876,7 @@
case Ico_F64: return toBool( c1->Ico.F64 == c2->Ico.F64 );
case Ico_F64i: return toBool( c1->Ico.F64i == c2->Ico.F64i );
case Ico_V128: return toBool( c1->Ico.V128 == c2->Ico.V128 );
+ case Ico_V256: return toBool( c1->Ico.V256 == c2->Ico.V256 );
default: vpanic("eqIRConst");
}
}
|
|
From: Julian S. <js...@ac...> - 2012-07-18 11:35:43
|
On Wednesday, July 18, 2012, Julian Seward wrote: > > if the glibc kludge you report applied to all linux targets, change to > > PRE_REG_READ5 for all linux targets. If the glibc kludge applies only > > to mips-linux, leave the PRE_REG_READ6 in place but put a _READ5 variant > > which is guarded by #if defined(VGO_mips32_linux). > > On second thoughts, just change it for MIPS. To change it for all > platforms assumes that it is always going to be called by glibc in this > particular way (with garbage 6th argument), which is not necessarily true. Urr. Ignore all that. The problem exists because Valgrind's model of what the kernel does with calls to sys_futex is inaccurate -- assuming the libc comment is accurate. I think the right fix is, for all platforms, check the first 5 args (PRE_REG_READ5). Then, if this is a non-FUTEX_WAIT_BITSET call, check the 6th arg. That makes the checking sync with the glibc comment. If you do this, please also add a comment in the code explaining why this change has happened. J |
|
From: Julian S. <js...@ac...> - 2012-07-18 11:22:01
|
> if the glibc kludge you report applied to all linux targets, change to > PRE_REG_READ5 for all linux targets. If the glibc kludge applies only > to mips-linux, leave the PRE_REG_READ6 in place but put a _READ5 variant > which is guarded by #if defined(VGO_mips32_linux). On second thoughts, just change it for MIPS. To change it for all platforms assumes that it is always going to be called by glibc in this particular way (with garbage 6th argument), which is not necessarily true. J |
|
From: Julian S. <js...@ac...> - 2012-07-18 11:16:18
|
> Now, "it causes no harm" may not be the case for Valgrind, as V sees 6th > parameter as an undefined value (well, for MIPS arch for sure). We handle > this in generic syscall wrapper in syswrap-linux.c as: mips32-linux is the only Linux target we support, in which the 6th syscall arg is passed in memory. It would be good if you could rerun your test case with --track-origins=yes and verify that the uninitialised value is stack-allocated. I am wondering why this doesn't happen on other targets. My guess is that it is effectively a false negative from Memcheck -- the register in which the 6th parameter is passed, is statistically speaking almost always going to marked as initialised, due to whatever value was previously in it, Memcheck almost never reports an error for that arg. > Anybody has a neat idea how to handle this situation? if the glibc kludge you report applied to all linux targets, change to PRE_REG_READ5 for all linux targets. If the glibc kludge applies only to mips-linux, leave the PRE_REG_READ6 in place but put a _READ5 variant which is guarded by #if defined(VGO_mips32_linux). J |
|
From: Petar J. <mip...@gm...> - 2012-07-18 10:54:05
|
Hi everyone,
V may have an issue with glibc for futex system call with
FUTEX_WAIT_BITSET. In short, glibc uses 5 parameters and V expects 6
parameters.
In more detail, in glibc, when syscall with FUTEX_WAIT_BITSET is invoked
(glibc/nptl/nptl-init.c), 5 parameters are given, and they comment this:
* /* NB: the syscall actually takes six parameters. The last is the
bit mask. But since we will not actually wait at all the value
is irrelevant. Given that passing six parameters is difficult
on some architectures we just pass whatever random value the
calling convention calls for to the kernel. It causes no harm. */
word = INTERNAL_SYSCALL (futex, err, 5, &word,
FUTEX_WAIT_BITSET | FUTEX_CLOCK_REALTIME
| FUTEX_PRIVATE_FLAG, 1, NULL, 0);*
Now, "it causes no harm" may not be the case for Valgrind, as V sees 6th
parameter as an undefined value (well, for MIPS arch for sure). We handle
this in generic syscall wrapper in syswrap-linux.c as:
* case VKI_FUTEX_WAIT_BITSET:
PRE_REG_READ6(long, "futex",
vki_u32 *, futex, int, op, int, val,
struct timespec *, utime, int, dummy, int, val3);
break;*
So, V will report something like:
*"Syscall param futex(val3) contains uninitialised byte(s)"*
Anybody has a neat idea how to handle this situation?
Petar
|
|
From: <sv...@va...> - 2012-07-18 10:47:49
|
sewardj 2012-07-18 11:47:38 +0100 (Wed, 18 Jul 2012)
New Revision: 12754
Log:
Un-break the build on MacOS, following r12742 (initial support for DWZ
compressed debuginfo).
Modified files:
trunk/coregrind/m_debuginfo/readmacho.c
Modified: trunk/coregrind/m_debuginfo/readmacho.c (+7 -2)
===================================================================
--- trunk/coregrind/m_debuginfo/readmacho.c 2012-07-16 23:39:24 +01:00 (rev 12753)
+++ trunk/coregrind/m_debuginfo/readmacho.c 2012-07-18 11:47:38 +01:00 (rev 12754)
@@ -1087,7 +1087,8 @@
NULL, 0,
debug_abbv_img, debug_abbv_sz,
debug_line_img, debug_line_sz,
- debug_str_img, debug_str_sz );
+ debug_str_img, debug_str_sz,
+ NULL, 0 /* ALT .debug_str */ );
/* The new reader: read the DIEs in .debug_info to acquire
information on variable types and locations. But only if
@@ -1102,7 +1103,11 @@
debug_line_img, debug_line_sz,
debug_str_img, debug_str_sz,
debug_ranges_img, debug_ranges_sz,
- debug_loc_img, debug_loc_sz
+ debug_loc_img, debug_loc_sz,
+ NULL, 0, /* ALT .debug_info */
+ NULL, 0, /* ALT .debug_abbv */
+ NULL, 0, /* ALT .debug_line */
+ NULL, 0 /* ALT .debug_str */
);
}
}
|