* [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
@ 2013-06-13 6:37 Jia He
2013-06-13 15:08 ` Frank Ch. Eigler
0 siblings, 1 reply; 6+ messages in thread
From: Jia He @ 2013-06-13 6:37 UTC (permalink / raw)
To: systemtap
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
2013-06-13 6:37 [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines Jia He
@ 2013-06-13 15:08 ` Frank Ch. Eigler
2013-06-14 2:13 ` Jia He
0 siblings, 1 reply; 6+ messages in thread
From: Frank Ch. Eigler @ 2013-06-13 15:08 UTC (permalink / raw)
To: Jia He; +Cc: systemtap
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
2013-06-13 15:08 ` Frank Ch. Eigler
@ 2013-06-14 2:13 ` Jia He
2013-06-18 0:31 ` Jia He
0 siblings, 1 reply; 6+ messages in thread
From: Jia He @ 2013-06-14 2:13 UTC (permalink / raw)
To: Frank Ch. Eigler; +Cc: systemtap
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
2013-06-14 2:13 ` Jia He
@ 2013-06-18 0:31 ` Jia He
2013-06-26 15:04 ` David Smith
0 siblings, 1 reply; 6+ messages in thread
From: Jia He @ 2013-06-18 0:31 UTC (permalink / raw)
To: Frank Ch. Eigler; +Cc: systemtap
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
2013-06-18 0:31 ` Jia He
@ 2013-06-26 15:04 ` David Smith
2013-06-26 16:54 ` David Smith
0 siblings, 1 reply; 6+ messages in thread
From: David Smith @ 2013-06-26 15:04 UTC (permalink / raw)
To: Jia He; +Cc: Frank Ch. Eigler, systemtap
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines
2013-06-26 15:04 ` David Smith
@ 2013-06-26 16:54 ` David Smith
0 siblings, 0 replies; 6+ messages in thread
From: David Smith @ 2013-06-26 16:54 UTC (permalink / raw)
To: Jia He; +Cc: Frank Ch. Eigler, systemtap
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)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2013-06-26 16:54 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-13 6:37 [Patch RFC] Tolerate if nsrcs>1 in iterate_over_srcfile_lines Jia He
2013-06-13 15:08 ` Frank Ch. Eigler
2013-06-14 2:13 ` Jia He
2013-06-18 0:31 ` Jia He
2013-06-26 15:04 ` David Smith
2013-06-26 16:54 ` David Smith
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).