This is the mail archive of the
systemtap@sourceware.org
mailing list for the systemtap project.
Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
- From: David Smith <dsmith at redhat dot com>
- To: Jia He <hejianet at gmail dot com>
- Cc: "Frank Ch. Eigler" <fche at redhat dot com>, systemtap at sourceware dot org
- Date: Wed, 26 Jun 2013 11:54:47 -0500
- Subject: Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
- References: <CAL+6Mkdtb6=0E0GrSk6fWWbO8TxRg51T9orWtYguKs=+ucamUQ at mail dot gmail dot com> <y0m7ghym4jv dot fsf at fche dot csb> <CAL+6Mkf+erf4Av7v5zck-z-uZ9N-AifoJhXMhPLT7LZHqb3VBQ at mail dot gmail dot com> <CAL+6Mkf4_KBZenAvTs9q2ZEWaNeKx+sQKg7sQcKYVKdz4=-rfg at mail dot gmail dot com> <51CB02FA dot 7060205 at redhat dot com>
On 06/26/2013 10:04 AM, David Smith wrote:
> This certainly isn't my area of expertise, but I'm beginning to think
> that both approaches are wrong. Rejecting everthing when nsrcs>1 seems
> to restrictive, but accepting everything when nsrcs>1 seems too broad.
> Perhaps there is a middle ground where if nsrcs>1 we go through each one
> and validate it. We'd only give an error if none were usable.
> To do this the existing code would probably need to be rearranged a bit.
> However I'm still unsure about what to do if nsrcs>1 and more than one
> validate correctly, assuming that is possible.
Here's a patch, but it isn't right either.
diff --git a/dwflpp.cxx b/dwflpp.cxx
index bb0d34a..320fcf9 100644
--- a/dwflpp.cxx
+++ b/dwflpp.cxx
@@ -1623,11 +1623,24 @@ dwflpp::iterate_over_srcfile_lines (char const *
srcfile,
if (line_type == RANGE && lineno > lines[1])
break;
line_probed = lines_probed.insert(lineno);
- if (lineno != l || line_probed.second == false || nsrcs > 1)
+ if (lineno != l || line_probed.second == false)
continue;
- dwarf_lineaddr (srcsp [0], &line_addr);
- if (!function_name_matches(func_pattern) && dwarf_haspc
(function, line_addr) != 1)
- break;
+
+ size_t valid_srcs = 0;
+ for (size_t s = 0; s < nsrcs; ++s)
+ {
+ dwarf_lineaddr (srcsp [s], &line_addr);
+ if (!function_name_matches(func_pattern)
+ && dwarf_haspc (function, line_addr) != 1)
+ break;
+ ++valid_srcs;
+ }
+ nsrcs = valid_srcs;
+ if (nsrcs == 0)
+ break;
+
+ // FIXME: if nsrcs > 1, and the a src didn't validate
+ // above, we don't try the rest.
}
// NB: Formerly, we used to filter, because:
@@ -1646,7 +1659,10 @@ dwflpp::iterate_over_srcfile_lines (char const *
srcfile,
size_t remaining_nsrcs = nsrcs;
- if (need_single_match && remaining_nsrcs > 1)
+ // FIXME: this still isn't right. Statement probes on a single
+ // line that have >1 src get rejected.
+ if (need_single_match && remaining_nsrcs > 1
+ && line_type != WILDCARD && line_type != RANGE)
{
// We wanted a single line record (a unique address for the
// line) and we got a bunch of line records. We're going to
Here's what's wrong with this patch. A statement probe with a wildcard
looks reasonable now:
# stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")'
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1644") $rqtp:long int
$rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1649") $rqtp:long int
$rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1650") $rqtp:long int
$rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1653") $rqtp:long int
$rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1655") $rqtp:long int
$rmtp:long int
On my kernel (3.10.0-0.rc6.git0.4.fc20.x86_64), lines 1644 and 1650 have
multiple line information. The problem is that probing individual lines
fails:
# stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:1644")'
# stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:1649")'
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1649") $rqtp:long int
$rmtp:long int
# stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:1650")'
# stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:1653")'
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1653") $rqtp:long int
$rmtp:long int
As I said before, this area of the code is really outside my knowledge,
but I'm wondering if we need to rethink the 'need_single_match' section.
'need_single_match' is really 'this_is_a_statement_probe'. Perhaps we
could do the same validation we do in the wildcard section when nsrcs>1
before the rejection.
--
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)