|
From: <sv...@va...> - 2008-05-29 23:09:46
|
Author: njn
Date: 2008-05-30 00:09:52 +0100 (Fri, 30 May 2008)
New Revision: 8154
Log:
Fix a bug in Massif and Cachegrind, whereby if program's forked, the child
wrote into the parent's output file even if %p was specified.
Josef, I think Callgrind does not have this bug, but you might want to say
something about forking in the manual, as I have done for Massif and
Cachegrind.
Modified:
trunk/cachegrind/cg_main.c
trunk/cachegrind/docs/cg-manual.xml
trunk/docs/xml/manual-core.xml
trunk/massif/docs/ms-manual.xml
trunk/massif/ms_main.c
Modified: trunk/cachegrind/cg_main.c
===================================================================
--- trunk/cachegrind/cg_main.c 2008-05-29 23:02:49 UTC (rev 8153)
+++ trunk/cachegrind/cg_main.c 2008-05-29 23:09:52 UTC (rev 8154)
@@ -1256,9 +1256,6 @@
static BranchCC Bc_total;
static BranchCC Bi_total;
-// The output file name. Controlled by --cachegrind-out-file.
-static Char* cachegrind_out_file = NULL;
-
static void fprint_CC_table_and_calc_totals(void)
{
Int i, fd;
@@ -1266,6 +1263,14 @@
Char buf[512], *currFile = NULL, *currFn = NULL;
LineCC* lineCC;
+ // Setup output filename. Nb: it's important to do this now, ie. as late
+ // as possible. If we do it at start-up and the program forks and the
+ // output file format string contains a %p (pid) specifier, both the
+ // parent and child will incorrectly write to the same file; this
+ // happened in 3.3.0.
+ Char* cachegrind_out_file =
+ VG_(expand_file_name)("--cachegrind-out-file", clo_cachegrind_out_file);
+
sres = VG_(open)(cachegrind_out_file, VKI_O_CREAT|VKI_O_TRUNC|VKI_O_WRONLY,
VKI_S_IRUSR|VKI_S_IWUSR);
if (sres.isError) {
@@ -1276,9 +1281,11 @@
cachegrind_out_file );
VG_(message)(Vg_UserMsg,
" ... so simulation results will be missing.");
+ VG_(free)(cachegrind_out_file);
return;
} else {
fd = sres.res;
+ VG_(free)(cachegrind_out_file);
}
// "desc:" lines (giving I1/D1/L2 cache configuration). The spaces after
@@ -1752,10 +1759,6 @@
VG_(exit)(2);
}
- // Setup output filename.
- cachegrind_out_file =
- VG_(expand_file_name)("--cachegrind-out-file", clo_cachegrind_out_file);
-
CC_table =
VG_(OSetGen_Create)(offsetof(LineCC, loc),
cmp_CodeLoc_LineCC,
Modified: trunk/cachegrind/docs/cg-manual.xml
===================================================================
--- trunk/cachegrind/docs/cg-manual.xml 2008-05-29 23:02:49 UTC (rev 8153)
+++ trunk/cachegrind/docs/cg-manual.xml 2008-05-29 23:09:52 UTC (rev 8154)
@@ -820,6 +820,19 @@
</sect2>
+<sect2 id="ms-manual.forkingprograms" xreflabel="Forking Programs">
+<title>Forking Programs</title>
+<para>If your program forks, the child will inherit all the profiling data that
+has been gathered for the parent.</para>
+
+<para>If the output file format string (controlled by
+<option>--cachegrind-out-file</option>) does not contain <option>%p</option>,
+then the outputs from the parent and child will be intermingled in a single
+output file, which will almost certainly make it unreadable by
+cg_annotate.</para>
+</sect2>
+
+
</sect1>
Modified: trunk/docs/xml/manual-core.xml
===================================================================
--- trunk/docs/xml/manual-core.xml 2008-05-29 23:02:49 UTC (rev 8153)
+++ trunk/docs/xml/manual-core.xml 2008-05-29 23:09:52 UTC (rev 8154)
@@ -714,7 +714,8 @@
<para><option>%p</option> is replaced with the current process ID.
This is very useful for program that invoke multiple processes.
WARNING: If you use <option>--trace-children=yes</option> and your
- program invokes multiple processes and you don't use this specifier
+ program invokes multiple processes OR your program forks without
+ calling exec afterwards, and you don't use this specifier
(or the <option>%q</option> specifier below), the Valgrind output from
all those processes will go into one file, possibly jumbled up, and
possibly incomplete.</para>
Modified: trunk/massif/docs/ms-manual.xml
===================================================================
--- trunk/massif/docs/ms-manual.xml 2008-05-29 23:02:49 UTC (rev 8153)
+++ trunk/massif/docs/ms-manual.xml 2008-05-29 23:09:52 UTC (rev 8154)
@@ -494,12 +494,23 @@
only prints the details for code locations responsible for more than 1%.
The entries that do not meet this threshold are aggregated. This avoids
filling up the output with large numbers of unimportant entries. The
-thresholds threshold can be changed with the
+thresholds can be changed with the
<computeroutput>--threshold</computeroutput> option that both Massif and
ms_print support.</para>
</sect2>
+<sect2 id="ms-manual.forkingprograms" xreflabel="Forking Programs">
+<title>Forking Programs</title>
+<para>If your program forks, the child will inherit all the profiling data that
+has been gathered for the parent.</para>
+
+<para>If the output file format string (controlled by
+<option>--massif-out-file</option>) does not contain <option>%p</option>, then
+the outputs from the parent and child will be intermingled in a single output
+file, which will almost certainly make it unreadable by ms_print.</para>
+</sect2>
+
</sect1>
Modified: trunk/massif/ms_main.c
===================================================================
--- trunk/massif/ms_main.c 2008-05-29 23:02:49 UTC (rev 8153)
+++ trunk/massif/ms_main.c 2008-05-29 23:09:52 UTC (rev 8154)
@@ -1866,9 +1866,6 @@
//--- Writing snapshots ---//
//------------------------------------------------------------//
-// The output file name. Controlled by --massif-out-file.
-static Char* massif_out_file = NULL;
-
Char FP_buf[BUF_LEN];
// XXX: implement f{,n}printf in m_libcprint.c eventually, and use it here.
@@ -2041,6 +2038,14 @@
Int i, fd;
SysRes sres;
+ // Setup output filename. Nb: it's important to do this now, ie. as late
+ // as possible. If we do it at start-up and the program forks and the
+ // output file format string contains a %p (pid) specifier, both the
+ // parent and child will incorrectly write to the same file; this
+ // happened in 3.3.0.
+ Char* massif_out_file =
+ VG_(expand_file_name)("--massif-out-file", clo_massif_out_file);
+
sres = VG_(open)(massif_out_file, VKI_O_CREAT|VKI_O_TRUNC|VKI_O_WRONLY,
VKI_S_IRUSR|VKI_S_IWUSR);
if (sres.isError) {
@@ -2050,9 +2055,11 @@
"error: can't open output file '%s'", massif_out_file );
VG_(message)(Vg_UserMsg,
" ... so profiling results will be missing.");
+ VG_(free)(massif_out_file);
return;
} else {
fd = sres.res;
+ VG_(free)(massif_out_file);
}
// Print massif-specific options that were used.
@@ -2184,10 +2191,6 @@
clear_snapshot( & snapshots[i], /*do_sanity_check*/False );
}
sanity_check_snapshots_array();
-
- // Setup output filename.
- massif_out_file =
- VG_(expand_file_name)("--massif-out-file", clo_massif_out_file);
}
static void ms_pre_clo_init(void)
|
|
From: Josef W. <Jos...@gm...> - 2008-05-30 12:58:26
|
On Friday 30 May 2008, sv...@va... wrote: > Author: njn > Date: 2008-05-30 00:09:52 +0100 (Fri, 30 May 2008) > New Revision: 8154 > > Log: > Fix a bug in Massif and Cachegrind, whereby if program's forked, the child > wrote into the parent's output file even if %p was specified. > > Josef, I think Callgrind does not have this bug, Hmm.. Callgrind seems to have this bug, too: VG_(expand_file_name) is called at initialization time. This bug may be a regression :( As I use the output filename early to enable interaction with the GUI, I will fix the bug by resetting the name when a PID change (fork) is detected. Patch about to come. > but you might want to say > something about forking in the manual, as I have done for Massif and > Cachegrind. Yes. Thanks, Josef |