[Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[Jia He]

Now if dwarf_getsrc_file returns by nsrcs>1 in
dwflpp::iterate_over_srcfile_lines,
The loop for (int l = lineno; ; l = l + 1) will not be continued.
But actually it is not correct in some cases.
--- systemtap-2.2.1.orig/dwflpp.cxx     2013-05-16 10:30:37.000000000 -0400
+++ systemtap-2.2.1/dwflpp.cxx  2013-06-13 01:59:52.000000000 -0400
@@ -1619,7 +1619,7 @@ dwflpp::iterate_over_srcfile_lines (char
          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)
@@ -1677,7 +1677,9 @@ dwflpp::iterate_over_srcfile_lines (char
                 advice << srcfile << ":" << hi_try;
               advice << ")";
             }
-          throw semantic_error (advice.str());
+          if (sess.verbose > 0)
+            clog<<advice.str();
+//          throw semantic_error (advice.str());
         }

       for (size_t i = 0; i < nsrcs; ++i)


test result
command:stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")'
in X86_64)
Before this patch:
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
timespec* $tu:struct timespec

After this patch:
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1602") $rqtp:struct
timespec* $rmtp:struct timespec* $tu:struct timespec
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1605") $rqtp:struct
timespec* $rmtp:struct timespec* $tu:struct timespec
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1608") $rmtp:struct
timespec* $tu:struct timespec
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1611") $rmtp:struct
timespec* $tu:struct timespec
kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
timespec* $tu:struct timespec

in ppc64)
Before:
<null result>

After:
kernel.statement("SyS_
nanosleep@kernel/hrtimer.c:1601") $rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1605") $rqtp:long int
$rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1608") $rmtp:long int
kernel.statement("SyS_nanosleep@kernel/hrtimer.c:1611") $rmtp:long int

Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[Frank Ch. Eigler]

Jia He <hejianet@gmail.com> writes:

> Now if dwarf_getsrc_file returns by nsrcs>1 in
> dwflpp::iterate_over_srcfile_lines,
> The loop for (int l = lineno; ; l = l + 1) will not be continued.
> But actually it is not correct in some cases.

Could you elaborate why you think it is incorrect?  Some of the
filtering is done deliberately, for example if the compiler debuginfo
cannot give an unambiguous starting PC-address for a source-level
statement.

> --- systemtap-2.2.1.orig/dwflpp.cxx     2013-05-16 10:30:37.000000000 -0400
> +++ systemtap-2.2.1/dwflpp.cxx  2013-06-13 01:59:52.000000000 -0400
> @@ -1619,7 +1619,7 @@ dwflpp::iterate_over_srcfile_lines (char
>           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)

For example, this change would ignore srcsp[n] for n>0, which would
need an explanation about how that could come about and why we can
ignore them.


>                  advice << srcfile << ":" << hi_try;
>                advice << ")";
>              }
> -          throw semantic_error (advice.str());
> +          if (sess.verbose > 0)
> +            clog<<advice.str();
> +//          throw semantic_error (advice.str());
>          }

What would be the purpose of this change? 


> test result
> command:stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")'
> in X86_64)
> Before this patch:
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
> timespec* $tu:struct timespec
>
> After this patch:
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1602") $rqtp:struct
> timespec* $rmtp:struct timespec* $tu:struct timespec
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1605") $rqtp:struct
> timespec* $rmtp:struct timespec* $tu:struct timespec
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1608") $rmtp:struct
> timespec* $tu:struct timespec
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1611") $rmtp:struct
> timespec* $tu:struct timespec
> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
> timespec* $tu:struct timespec

That looks good, as long as those listed probe points map to proper
addresses and give back proper context variables.


- FChE

Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[Jia He]

On Thu, Jun 13, 2013 at 11:08 PM, Frank Ch. Eigler <fche@redhat.com> wrote:
> Jia He <hejianet@gmail.com> writes:
>
>> Now if dwarf_getsrc_file returns by nsrcs>1 in
>> dwflpp::iterate_over_srcfile_lines,
>> The loop for (int l = lineno; ; l = l + 1) will not be continued.
>> But actually it is not correct in some cases.
>
> Could you elaborate why you think it is incorrect?  Some of the
> filtering is done deliberately, for example if the compiler debuginfo
> cannot give an unambiguous starting PC-address for a source-level
> statement.
Maybe I don't understand the codes correctly, please correct me if possible.
the related readelf info for hrtimer.o(ppc64) is as follows:
#readelf --debug-dump=decodedline hrtimer.o
...
hrtimer.c                                   1601              0x2040
hrtimer.c                                   1605              0x2050
hrtimer.c                                   1601              0x2054
hrtimer.c                                   1605              0x205c
hrtimer.c                                   1601              0x207c
hrtimer.c                                   1608              0x20a0
hrtimer.c                                   1611              0x20ac
hrtimer.c                                   1608              0x20b4
hrtimer.c                                   1611              0x20c8
...
the objdump info(ppc64):
0000000000002040 <.SyS_nanosleep>:
    2040:       7c 08 02 a6     mflr    r0
    2044:       fb a1 ff e8     std     r29,-24(r1)
    2048:       fb e1 ff f8     std     r31,-8(r1)
    204c:       7c 9d 23 78     mr      r29,r4
    2050:       38 a0 00 10     li      r5,16
    2054:       f8 01 00 10     std     r0,16(r1)
    2058:       f8 21 ff 61     stdu    r1,-160(r1)
    205c:       7c 64 1b 78     mr      r4,r3
    2060:       3b e1 00 70     addi    r31,r1,112
    2064:       7f e3 fb 78     mr      r3,r31
    2068:       48 00 00 01     bl      2068 <.SyS_nanosleep+0x28>
    206c:       60 00 00 00     nop
    2070:       38 00 ff f2     li      r0,-14
    2074:       2f a3 00 00     cmpdi   cr7,r3,0
    2078:       41 9e 00 28     beq-    cr7,20a0 <.SyS_nanosleep+0x60>
    207c:       38 21 00 a0     addi    r1,r1,160
    2080:       7c 03 03 78     mr      r3,r0
    2084:       e8 01 00 10     ld      r0,16(r1)
    2088:       eb a1 ff e8     ld      r29,-24(r1)
    208c:       eb e1 ff f8     ld      r31,-8(r1)
    2090:       7c 08 03 a6     mtlr    r0
    2094:       4e 80 00 20     blr
    2098:       60 00 00 00     nop
    209c:       60 00 00 00     nop
    20a0:       e8 01 00 70     ld      r0,112(r1)
    20a4:       2f a0 00 00     cmpdi   cr7,r0,0
    20a8:       40 9c 00 0c     bge-    cr7,20b4 <.SyS_nanosleep+0x74>
    20ac:       38 00 ff ea     li      r0,-22
    20b0:       4b ff ff cc     b       207c <.SyS_nanosleep+0x3c>
    20b4:       3c 00 3b 9a     lis     r0,15258
    20b8:       e9 21 00 78     ld      r9,120(r1)
    20bc:       60 00 c9 ff     ori     r0,r0,51711
    20c0:       7f a9 00 40     cmpld   cr7,r9,r0
    20c4:       41 9d ff e8     bgt+    cr7,20ac <.SyS_nanosleep+0x6c>
    20c8:       7f e3 fb 78     mr      r3,r31
    20cc:       7f a4 eb 78     mr      r4,r29
    20d0:       38 a0 00 01     li      r5,1
    20d4:       38 c0 00 01     li      r6,1
    20d8:       48 00 00 01     bl      20d8 <.SyS_nanosleep+0x98>
    20dc:       7c 60 1b 78     mr      r0,r3
    20e0:       4b ff ff 9c     b       207c <.SyS_nanosleep+0x3c>

Seems if l=1601 or 1605 or 1608 in the loop  for (int l = lineno; ; l = l + 1)
the nsrcs will return >1 after dwarf_getsrc_file().
So the old logic will regard it as sth abnormal and will not consider
it is valid
and will not insert it to lines_probed. Do u think the old logic is incorrect?
>
>> --- systemtap-2.2.1.orig/dwflpp.cxx     2013-05-16 10:30:37.000000000 -0400
>> +++ systemtap-2.2.1/dwflpp.cxx  2013-06-13 01:59:52.000000000 -0400
>> @@ -1619,7 +1619,7 @@ dwflpp::iterate_over_srcfile_lines (char
>>           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)
>
> For example, this change would ignore srcsp[n] for n>0, which would
> need an explanation about how that could come about and why we can
> ignore them.
>
>
>>                  advice << srcfile << ":" << hi_try;
>>                advice << ")";
>>              }
>> -          throw semantic_error (advice.str());
>> +          if (sess.verbose > 0)
>> +            clog<<advice.str();
>> +//          throw semantic_error (advice.str());
>>          }
>
> What would be the purpose of this change?
If it throws the error message, the loop will not be continued and the
following "l"
will not be checked.
>
>
>> test result
>> command:stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")'
>> in X86_64)
>> Before this patch:
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
>> timespec* $tu:struct timespec
>>
>> After this patch:
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1602") $rqtp:struct
>> timespec* $rmtp:struct timespec* $tu:struct timespec
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1605") $rqtp:struct
>> timespec* $rmtp:struct timespec* $tu:struct timespec
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1608") $rmtp:struct
>> timespec* $tu:struct timespec
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1611") $rmtp:struct
>> timespec* $tu:struct timespec
>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
>> timespec* $tu:struct timespec
>
> That looks good, as long as those listed probe points map to proper
> addresses and give back proper context variables.
>
>
> - FChE

Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[Jia He]

Hi Frank, Do u agree or disagree with my analysis?


On Fri, Jun 14, 2013 at 10:13 AM, Jia He <hejianet@gmail.com> wrote:
> On Thu, Jun 13, 2013 at 11:08 PM, Frank Ch. Eigler <fche@redhat.com> wrote:
>> Jia He <hejianet@gmail.com> writes:
>>
>>> Now if dwarf_getsrc_file returns by nsrcs>1 in
>>> dwflpp::iterate_over_srcfile_lines,
>>> The loop for (int l = lineno; ; l = l + 1) will not be continued.
>>> But actually it is not correct in some cases.
>>
>> Could you elaborate why you think it is incorrect?  Some of the
>> filtering is done deliberately, for example if the compiler debuginfo
>> cannot give an unambiguous starting PC-address for a source-level
>> statement.
> Maybe I don't understand the codes correctly, please correct me if possible.
> the related readelf info for hrtimer.o(ppc64) is as follows:
> #readelf --debug-dump=decodedline hrtimer.o
> ...
> hrtimer.c                                   1601              0x2040
> hrtimer.c                                   1605              0x2050
> hrtimer.c                                   1601              0x2054
> hrtimer.c                                   1605              0x205c
> hrtimer.c                                   1601              0x207c
> hrtimer.c                                   1608              0x20a0
> hrtimer.c                                   1611              0x20ac
> hrtimer.c                                   1608              0x20b4
> hrtimer.c                                   1611              0x20c8
> ...
> the objdump info(ppc64):
> 0000000000002040 <.SyS_nanosleep>:
>     2040:       7c 08 02 a6     mflr    r0
>     2044:       fb a1 ff e8     std     r29,-24(r1)
>     2048:       fb e1 ff f8     std     r31,-8(r1)
>     204c:       7c 9d 23 78     mr      r29,r4
>     2050:       38 a0 00 10     li      r5,16
>     2054:       f8 01 00 10     std     r0,16(r1)
>     2058:       f8 21 ff 61     stdu    r1,-160(r1)
>     205c:       7c 64 1b 78     mr      r4,r3
>     2060:       3b e1 00 70     addi    r31,r1,112
>     2064:       7f e3 fb 78     mr      r3,r31
>     2068:       48 00 00 01     bl      2068 <.SyS_nanosleep+0x28>
>     206c:       60 00 00 00     nop
>     2070:       38 00 ff f2     li      r0,-14
>     2074:       2f a3 00 00     cmpdi   cr7,r3,0
>     2078:       41 9e 00 28     beq-    cr7,20a0 <.SyS_nanosleep+0x60>
>     207c:       38 21 00 a0     addi    r1,r1,160
>     2080:       7c 03 03 78     mr      r3,r0
>     2084:       e8 01 00 10     ld      r0,16(r1)
>     2088:       eb a1 ff e8     ld      r29,-24(r1)
>     208c:       eb e1 ff f8     ld      r31,-8(r1)
>     2090:       7c 08 03 a6     mtlr    r0
>     2094:       4e 80 00 20     blr
>     2098:       60 00 00 00     nop
>     209c:       60 00 00 00     nop
>     20a0:       e8 01 00 70     ld      r0,112(r1)
>     20a4:       2f a0 00 00     cmpdi   cr7,r0,0
>     20a8:       40 9c 00 0c     bge-    cr7,20b4 <.SyS_nanosleep+0x74>
>     20ac:       38 00 ff ea     li      r0,-22
>     20b0:       4b ff ff cc     b       207c <.SyS_nanosleep+0x3c>
>     20b4:       3c 00 3b 9a     lis     r0,15258
>     20b8:       e9 21 00 78     ld      r9,120(r1)
>     20bc:       60 00 c9 ff     ori     r0,r0,51711
>     20c0:       7f a9 00 40     cmpld   cr7,r9,r0
>     20c4:       41 9d ff e8     bgt+    cr7,20ac <.SyS_nanosleep+0x6c>
>     20c8:       7f e3 fb 78     mr      r3,r31
>     20cc:       7f a4 eb 78     mr      r4,r29
>     20d0:       38 a0 00 01     li      r5,1
>     20d4:       38 c0 00 01     li      r6,1
>     20d8:       48 00 00 01     bl      20d8 <.SyS_nanosleep+0x98>
>     20dc:       7c 60 1b 78     mr      r0,r3
>     20e0:       4b ff ff 9c     b       207c <.SyS_nanosleep+0x3c>
>
> Seems if l=1601 or 1605 or 1608 in the loop  for (int l = lineno; ; l = l + 1)
> the nsrcs will return >1 after dwarf_getsrc_file().
> So the old logic will regard it as sth abnormal and will not consider
> it is valid
> and will not insert it to lines_probed. Do u think the old logic is incorrect?
>>
>>> --- systemtap-2.2.1.orig/dwflpp.cxx     2013-05-16 10:30:37.000000000 -0400
>>> +++ systemtap-2.2.1/dwflpp.cxx  2013-06-13 01:59:52.000000000 -0400
>>> @@ -1619,7 +1619,7 @@ dwflpp::iterate_over_srcfile_lines (char
>>>           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)
>>
>> For example, this change would ignore srcsp[n] for n>0, which would
>> need an explanation about how that could come about and why we can
>> ignore them.
>>
>>
>>>                  advice << srcfile << ":" << hi_try;
>>>                advice << ")";
>>>              }
>>> -          throw semantic_error (advice.str());
>>> +          if (sess.verbose > 0)
>>> +            clog<<advice.str();
>>> +//          throw semantic_error (advice.str());
>>>          }
>>
>> What would be the purpose of this change?
> If it throws the error message, the loop will not be continued and the
> following "l"
> will not be checked.
>>
>>
>>> test result
>>> command:stap -L 'kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")'
>>> in X86_64)
>>> Before this patch:
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
>>> timespec* $tu:struct timespec
>>>
>>> After this patch:
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1602") $rqtp:struct
>>> timespec* $rmtp:struct timespec* $tu:struct timespec
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1605") $rqtp:struct
>>> timespec* $rmtp:struct timespec* $tu:struct timespec
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1608") $rmtp:struct
>>> timespec* $tu:struct timespec
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1611") $rmtp:struct
>>> timespec* $tu:struct timespec
>>> kernel.statement("sys_nanosleep@kernel/hrtimer.c:1612") $rmtp:struct
>>> timespec* $tu:struct timespec
>>
>> That looks good, as long as those listed probe points map to proper
>> addresses and give back proper context variables.
>>
>>
>> - FChE

Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[David Smith]

On 06/17/2013 07:31 PM, Jia He wrote:
> Hi Frank, Do u agree or disagree with my analysis?

I'm not Frank, but I've looked at your patch. The proposed 'stap -L'
output look ok, but the following does not (the two "multiple addresses"
messages).

====
stap -p4 -ve 'probe kernel.statement("sys_nanosleep@kernel/hrtimer.c:*")
{ printf("here\n") }'
Pass 1: parsed user script and 94 library script(s) using
209488virt/27280res/3016shr/24568data kb, in 170usr/70sys/232real ms.
multiple addresses for kernel/hrtimer.c:1644 [man error::dwarf] (try
kernel/hrtimer.c:1643 or kernel/hrtimer.c:1645)
multiple addresses for kernel/hrtimer.c:1650 [man error::dwarf] (try
kernel/hrtimer.c:1649 or kernel/hrtimer.c:1651)
Pass 2: analyzed script: 7 probe(s), 0 function(s), 0 embed(s), 0
global(s) using 428896virt/103496res/22564shr/83872data kb, in
850usr/120sys/972real ms.
Pass 3: translated to C into
"/tmp/stap5k0Jxw/stap_8585646b49e1a5e52739bef89619426d_2021_src.c" using
428896virt/103788res/22856shr/83872data kb, in 10usr/0sys/10real ms.
/home/dsmith/.systemtap/cache/85/stap_8585646b49e1a5e52739bef89619426d_2021.ko
Pass 4: compiled C into "stap_8585646b49e1a5e52739bef89619426d_2021.ko"
in 2600usr/2700sys/8669real ms.
====

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.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines

[David Smith]

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)