2007-05-06 Frank Ch. Eigler <fche@elastic.org>

	PR 1305.
	* tapsets.cxx (iterate_over_srcfile_lines): Exclude approximate
	file:lineno matches optimistically returned by dwarf_getsrc_file.
	(query_srcfile_line): Use Dwarf_Line line number for constructing
	derived_probe location string.

2007-05-06  Frank Ch. Eigler  <fche@elastic.org>

	* semko/thirtynine.stp: New test.

6 files changed, 57 insertions, 5 deletions

diff --git a/ChangeLog b/ChangeLog
index 505bac06..3a2f7171 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2007-05-06  Frank Ch. Eigler  <fche@elastic.org>
+
+	PR 1305.
+	* tapsets.cxx (iterate_over_srcfile_lines): Exclude approximate 
+	file:lineno matches optimistically returned by dwarf_getsrc_file.
+	(query_srcfile_line): Use Dwarf_Line line number for constructing
+	derived_probe location string.
+
 2007-05-02  David Smith  <dsmith@redhat.com>
 
 	* translate.cxx (c_tmpcounter::visit_functioncall): Updated
diff --git a/tapsets.cxx b/tapsets.cxx
index 029b4dec..df91f9ab 100644
--- a/tapsets.cxx
+++ b/tapsets.cxx
@@ -932,8 +932,38 @@ struct dwflpp
 		  dwarf_getsrc_file (module_dwarf,
 				     srcfile, lineno, 0,
 				     &srcsp, &nsrcs));
+    // BUT:
 
-    if (need_single_match && nsrcs > 1)
+    // dwarf_getsrc_file gets one *near hits* for line numbers, not
+    // exact matches.  For example, an existing file but a nonexistent
+    // line number will be rounded up to the next definition in that
+    // file.  This may be similar to the GDB breakpoint algorithm, but
+    // we don't want to be so fuzzy in systemtap land.  So we filter.
+
+    // XXX: the code also fails to match e.g.  inline function
+    // definitions when the srcfile is a header file rather than the
+    // CU name.
+
+    size_t remaining_nsrcs = nsrcs;
+    for (size_t i = 0; i < nsrcs; ++i)
+      {
+        int l_no;
+        Dwarf_Line* l = srcsp[i];
+        dwarf_assert ("dwarf_lineno", dwarf_lineno (l, & l_no));
+        if (l_no != lineno)
+          {
+            if (sess.verbose > 3)
+	      clog << "skiping approximate line number match "
+                   << "(" << l_no << " vs " << lineno << ")" 
+                   << " in file '" << srcfile << "'"
+                   << "\n";
+            srcsp[i] = 0;
+            remaining_nsrcs --;
+          }
+      }
+
+
+    if (need_single_match && remaining_nsrcs > 1)
       {
 	// We wanted a single line record (a unique address for the
 	// line) and we got a bunch of line records. We're going to
@@ -976,7 +1006,8 @@ struct dwflpp
       {
 	for (size_t i = 0; i < nsrcs; ++i)
 	  {
-	    callback (srcsp[i], data);
+            if (srcsp [i]) // skip over mismatched lines
+              callback (srcsp[i], data);
 	  }
       }
     catch (...)
@@ -2579,6 +2610,9 @@ query_srcfile_line (Dwarf_Line * line, void * arg)
   Dwarf_Addr addr;
   dwarf_lineaddr(line, &addr);
 
+  int lineno;
+  dwarf_lineno (line, &lineno);
+
   for (map<Dwarf_Addr, func_info>::iterator i = q->filtered_functions.begin();
        i != q->filtered_functions.end(); ++i)
     {
@@ -2588,7 +2622,8 @@ query_srcfile_line (Dwarf_Line * line, void * arg)
 	    clog << "function DIE lands on srcfile\n";
 	  if (q->has_statement_str)
 	    query_statement (i->second.name, i->second.decl_file,
-			     q->line, NULL, addr, q);
+			     lineno, // NB: not q->line !
+                             NULL, addr, q);
 	  else
 	    query_func_info (i->first, i->second, q);
 	}
diff --git a/testsuite/ChangeLog b/testsuite/ChangeLog
index 00a30a3d..79a6cdd5 100644
--- a/testsuite/ChangeLog
+++ b/testsuite/ChangeLog
@@ -1,3 +1,7 @@
+2007-05-06  Frank Ch. Eigler  <fche@elastic.org>
+
+	* semko/thirtynine.stp: New test.
+
 2007-05-01  Martin Hunt  <hunt@redhat.com>
 
 	* systemtap.samples/system_func.*: New test.
diff --git a/testsuite/semko/thirtyeight.stp b/testsuite/semko/thirtyeight.stp
index 6b7f1be4..749b1125 100755
--- a/testsuite/semko/thirtyeight.stp
+++ b/testsuite/semko/thirtyeight.stp
@@ -1,3 +1,3 @@
-#! stap -p3
+#! stap -p2
 
 probe kernel.statement(0).absolute {} /* needs guru mode */
diff --git a/testsuite/semko/thirtynine.stp b/testsuite/semko/thirtynine.stp
new file mode 100755
index 00000000..d8cd56f5
--- /dev/null
+++ b/testsuite/semko/thirtynine.stp
@@ -0,0 +1,5 @@
+#! stap -p2
+
+# bug 1305
+probe kernel.function("*@fs/open.c:1") {
+}
diff --git a/testsuite/semko/thirtyseven.stp b/testsuite/semko/thirtyseven.stp
index b0c8225e..77637ff4 100755
--- a/testsuite/semko/thirtyseven.stp
+++ b/testsuite/semko/thirtyseven.stp
@@ -1,3 +1,3 @@
-#! stap -gp3
+#! stap -gp2
 
 probe kernel.statement(0).absolute { print ($any_variable) }