Fix inconsistency between callgrind and format spec

Bug found by, and fix based on a patch by Mark Wielaard

Callgrind format specification was inconsistent with
what Callgrind generates, and what callgrind_annotate
accepted. Now, callgrind_annotate accepts the examples
in the format specification.

* Callgrind writes 'cfi=' lines for when a call target goes
  into another source file. According to the spec, 'cfl=' is
  used for this. Change the spec to allow both, and change
  callgrind_annotate to accept both.
* The spec requires just an "events:" line as minimum header
  to render the file as correct according to the specification.
  callgrind_annotate also expected a 'cmd=' line. Fixed.
* The 'summary:' line is optional in the spec. Fixed in
  callgrind_annotate. If not provided, summary is calculated
  from all cost lines.

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

diff --git a/callgrind/callgrind_annotate.in b/callgrind/callgrind_annotate.in
index e2e2f5f2ebfd22506b0b70453349968d7e54becb..f11d5f643a13c899167e5c3f65ffd755cdf153a9 100644 (file)
--- a/callgrind/callgrind_annotate.in
+++ b/callgrind/callgrind_annotate.in
@@ -81,6 +81,7 @@ use strict;
 # Total counts for summary (an array reference).
 my $summary_CC;
 my $totals_CC;
+my $summary_calculated = 0;
 
 # Totals for each function, for overall summary.
 # hash(filename:fn_name => CC array)
@@ -440,9 +441,6 @@ sub read_input_file()
       }
     }
 
-    # Check for needed header entries
-    ($cmd ne "") or die("Line $.: missing command line\n");
-
     # Read "events:" line.  We make a temporary hash in which the Nth event's
     # value is N, which is useful for handling --show/--sort options below.
     ($events ne "") or die("Line $.: missing events line\n");
@@ -630,7 +628,7 @@ sub read_input_file()
         } elsif (s/^cob=(.*)$//) {
          $curr_cobj = uncompressed_name("ob",$1);
 
-       } elsif (s/^cfi=(.*)$//) {
+       } elsif (s/^cf[il]=(.*)$//) {
          $curr_cfile = uncompressed_name("fl",$1);
 
        } elsif (s/^cfn=(.*)$//) {
@@ -681,15 +679,6 @@ sub read_input_file()
         }
     }
 
-    if ((not defined $summary_CC) || is_zero($summary_CC)) {
-       $summary_CC = $totals_CC;
-    }
-
-    # Check if summary line was present
-    if (not defined $summary_CC) {
-        warn("WARNING: missing final summary line, no summary will be printed\n");
-    }
-
     # Finish up handling final filename/fn_name counts
     $fn_totals{"$curr_file:$curr_fn"} = $curr_fn_CC
        if (defined $curr_file && defined $curr_fn);
@@ -704,6 +693,20 @@ sub read_input_file()
     }
 
     close(INPUTFILE);
+
+    if ((not defined $summary_CC) || is_zero($summary_CC)) {
+       $summary_CC = $totals_CC;
+
+       # if neither 'summary:' nor 'totals:' line is given,
+       # calculate summary from fn_totals hash
+       if ((not defined $summary_CC) || is_zero($summary_CC)) {
+           $summary_calculated = 1;
+           $summary_CC = [];
+           foreach my $name (keys %fn_totals) {
+               add_array_a_to_b($fn_totals{$name}, $summary_CC);
+           }
+       }
+    }
 }
 
 #-----------------------------------------------------------------------------
@@ -719,6 +722,7 @@ sub print_options ()
     print($fancy);
     print($desc);
     my $target = $cmd;
+    if ($target eq "") { $target = "(unknown)"; }
     if ($pid ne "") {
       $target .= " (PID $pid";
       if ($part ne "") { $target .= ", part $part"; }
@@ -855,8 +859,11 @@ sub print_summary_and_fn_totals ()
     print("\n");
     print($fancy);
     print_CC($summary_CC, $summary_CC_col_widths);
-    print(" PROGRAM TOTALS\n");
-    print("\n");
+    print(" PROGRAM TOTALS");
+    if ($summary_calculated) {
+       print(" (calculated)");
+    }
+    print("\n\n");
 
     # Header for functions
     print($fancy);
@@ -921,7 +928,8 @@ sub print_summary_and_fn_totals ()
         print_CC($fn_CC, $fn_CC_col_widths);
        if ($tree_caller || $tree_calling) { print " * "; }
         print(" $fn_name");
-       if (defined $obj_name{$fn_name}) {
+       if ((defined $obj_name{$fn_name}) &&
+           ($obj_name{$fn_name} ne "")) {
          print " [$obj_name{$fn_name}]";
        }
        print "\n";

diff --git a/callgrind/docs/cl-format.xml b/callgrind/docs/cl-format.xml
index f6199e005a65d2809e976b0775838a0605305a63..4426b6ac7c3ac5887b7a8c4aa8343e2468e4452f 100644 (file)
--- a/callgrind/docs/cl-format.xml
+++ b/callgrind/docs/cl-format.xml
@@ -30,7 +30,9 @@ For detailed syntax, look at the format reference.</para>
 <title>Basic Structure</title>
 
 <para>Each file has a header part of an arbitrary number of lines of the
-format "key: value". The lines with key "positions" and "events" define
+format "key: value". After the header, lines specifying profile costs
+follow. Everywhere, comments on own lines starting with '#' are allowed.
+The header lines with keys "positions" and "events" define
 the meaning of cost lines in the second part of the file: the value of
 "positions" is a list of subpositions, and the value of "events" is a list
 of event type names. Cost lines consist of subpositions followed by 64-bit
@@ -98,8 +100,8 @@ called: profile data only contains sums.</para>
 ability to specify call relationship among functions. More generally, you
 specify associations among positions. For this, the second part of the
 file also can contain association specifications. These look similar to
-position specifications, but consist of 2 lines. For calls, the format
-looks like 
+position specifications, but consist of two lines. For calls, the format
+looks like
 <screen>
  calls=(Call Count) (Destination position)
  (Source position) (Inclusive cost of call)
@@ -108,8 +110,10 @@ looks like
 <para>The destination only specifies subpositions like line number. Therefore,
 to be able to specify a call to another function in another source file, you
 have to precede the above lines with a "cfn=" specification for the name of the
-called function, and a "cfl=" specification if the function is in another
-source file. The 2nd line looks like a regular cost line with the difference
+called function, and optionally a "cfi=" specification if the function is in
+another source file ("cfl=" is an alternative specification for "cfi=" because
+of historical reasons, and both should be supported by format readers).
+The second line looks like a regular cost line with the difference
 that inclusive cost spent inside of the function call has to be specified.</para> 
 
 <para>Other associations are for example (conditional) jumps. See the
@@ -134,14 +138,14 @@ fn=main
 cfn=func1
 calls=1 50
 16 400
-cfl=file2.c
+cfi=file2.c
 cfn=func2
 calls=3 20
 16 400
 
 fn=func1
 51 100
-cfl=file2.c
+cfi=file2.c
 cfn=func2
 calls=2 20
 51 300
@@ -158,7 +162,7 @@ and 400 as sum for the three calls to <function>func2</function>.</para>
 
 <para>Function <function>func1</function> is located in
 <filename>file1.c</filename>, the same as <function>main</function>.
-Therefore, a "cfl=" specification for the call to <function>func1</function>
+Therefore, a "cfi=" specification for the call to <function>func1</function>
 is not needed. The function <function>func1</function> only consists of code
 at line 51 of <filename>file1.c</filename>, where <function>func2</function>
 is called.</para>
@@ -191,14 +195,14 @@ fn=(1) main
 cfn=(2) func1
 calls=1 50
 16 400
-cfl=(2) file2.c
+cfi=(2) file2.c
 cfn=(3) func2
 calls=3 20
 16 400
 
 fn=(2)
 51 100
-cfl=(2)
+cfi=(2)
 cfn=(3)
 calls=2 20
 51 300
@@ -286,8 +290,8 @@ relocatable shared objects, often a load offset has to be subtracted.</para>
 cost of the full run has to be known. Usually, it is assumed that this is the
 sum of all cost lines in a file. But sometimes, this is not correct. Thus, you
 can specify a "summary:" line in the header giving the full cost for the
-profile run. This has another effect: a import filter can show a progress bar
-while loading a large data file if he knows to cost sum in advance.</para>
+profile run. An import filter may use this to show a progress bar
+while loading a large data file.</para>
 
 </sect3>
 
@@ -356,7 +360,7 @@ for "Ir and "Dr".</para>
 <screen>PositionSpecification := Position "=" Space* PositionName</screen>
 <screen>Position := CostPosition | CalledPosition</screen>
 <screen>CostPosition := "ob" | "fl" | "fi" | "fe" | "fn"</screen>
-<screen>CalledPosition := " "cob" | "cfl" | "cfn"</screen>
+<screen>CalledPosition := " "cob" | "cfi" | "cfl" | "cfn"</screen>
 <screen>PositionName := ( "(" Number ")" )? (Space* NoNewLineChar* )?</screen>
 <screen>AssociationSpecification := CallSpecification
   | JumpSpecification</screen>
@@ -388,24 +392,24 @@ for "Ir and "Dr".</para>
     <para>This is used to distinguish future profile data formats.  A 
     major version of 0 or 1 is supposed to be upwards compatible with 
     Cachegrind's format.  It is optional; if not appearing, version 1 
-    is supposed.  Otherwise, this has to be the first header line.</para>
+    is assumed.  Otherwise, this has to be the first header line.</para>
   </listitem>
 
   <listitem>
     <para><computeroutput>pid: process id</computeroutput> [Callgrind]</para>
-    <para>This specifies the process ID of the supervised application 
+    <para>Optional. This specifies the process ID of the supervised application 
     for which this profile was generated.</para>
   </listitem>
 
   <listitem>
     <para><computeroutput>cmd: program name + args</computeroutput> [Cachegrind]</para>
-    <para>This specifies the full command line of the supervised
+    <para>Optional. This specifies the full command line of the supervised
     application for which this profile was generated.</para>
   </listitem>
 
   <listitem>
     <para><computeroutput>part: number</computeroutput> [Callgrind]</para>
-    <para>This specifies a sequentially incremented number for each dump 
+    <para>Optional. This specifies a sequentially incremented number for each dump 
     generated, starting at 1.</para>
   </listitem>
 
@@ -468,13 +472,16 @@ for "Ir and "Dr".</para>
 
   <listitem>
     <para><computeroutput>summary: costs</computeroutput> [Callgrind]</para>
+    <para>Optional. This header line specifies a summary cost, which should be
+    equal or larger than a total over all self costs. It may be larger as
+    the cost lines may not represent all cost of the program run.</para>
+  </listitem>
+
+  <listitem>
     <para><computeroutput>totals: costs</computeroutput> [Cachegrind]</para>
-    <para>The value or the total number of events covered by this trace
-    file.  Both keys have the same meaning, but the "totals:" line 
-    happens to be at the end of the file, while "summary:" appears in 
-    the header.  This was added to allow postprocessing tools to know
-    in advance to total cost. The two lines always give the same cost 
-    counts.</para>
+    <para>Optional. Should appear at the end of the file (although
+    looking like a header line). Must give the total of all cost lines,
+    to allow for a consistency check.</para>
   </listitem>
 
 </itemizedlist>
@@ -534,11 +541,17 @@ optional.</para>
   </listitem>
 
   <listitem>
-    <para><computeroutput>cfl=</computeroutput> [Callgrind]</para>
+    <para><computeroutput>cfi=</computeroutput> [Callgrind]</para>
     <para>The source file including the code of the target of the
     next call cost lines.</para>
   </listitem>
 
+  <listitem>
+    <para><computeroutput>cfl=</computeroutput> [Callgrind]</para>
+    <para>Alternative spelling for <computeroutput>cfi=</computeroutput>
+    specification (because of historical reasons).</para>
+  </listitem>
+
   <listitem>
     <para><computeroutput>cfn=</computeroutput> [Callgrind]</para>
     <para>The name of the target function of the next call cost