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.



git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8154

cachegrind/cg_main.c

@@ -1256,9 +1256,6 @@ static CacheCC  Dw_total;
 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 @@ static void fprint_CC_table_and_calc_totals(void)
    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 @@ static void fprint_CC_table_and_calc_totals(void)
          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 @@ static void cg_post_clo_init(void)
       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,

cachegrind/docs/cg-manual.xml

@@ -820,6 +820,19 @@ way as for C/C++ programs.</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>--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>
 
 

docs/xml/manual-core.xml

@@ -714,7 +714,8 @@ categories.</para>
       <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>

massif/docs/ms-manual.xml

@@ -494,12 +494,23 @@ responsible for more than 1% of useful memory bytes, and ms_print likewise
 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>
 
 

massif/ms_main.c

@@ -1866,9 +1866,6 @@ IRSB* ms_instrument ( VgCallbackClosure* closure,
 //--- 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 @@ static void write_snapshots_to_file(void)
    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 @@ static void write_snapshots_to_file(void)
          "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 @@ static void ms_post_clo_init(void)
       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)