* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
@ 2006-01-12 20:49 ` joshua dot i dot stone at intel dot com
2006-01-12 20:56 ` joshua dot i dot stone at intel dot com
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: joshua dot i dot stone at intel dot com @ 2006-01-12 20:49 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-12 20:49 -------
I don't want to belittle the importance of checking return values, but filled
maps could be avoided by turning on map->wrap. Perhaps we need pragmas or
something to let the user turn this on...
And BTW - the current replacement policy when wrapping is LRU. But, when
aggregating pmaps (_stp_pmap_agg), it favors data from the later cpus in the
for_each_cpu. I'm not sure of a "fair" way to correct this, except to let the
pmap->agg have more entries (MAXMAPENTRIES*NUM_CPUS).
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
2006-01-12 20:49 ` [Bug translator/2149] " joshua dot i dot stone at intel dot com
@ 2006-01-12 20:56 ` joshua dot i dot stone at intel dot com
2006-01-12 21:07 ` hunt at redhat dot com
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: joshua dot i dot stone at intel dot com @ 2006-01-12 20:56 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-12 20:56 -------
(In reply to comment #1)
> And BTW - the current replacement policy when wrapping is LRU.
Sorry, this is wrong - it's actually a FIFO. To be LRU we need to move nodes to
the tail when they're accessed as well. I think this would be preferable...
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
2006-01-12 20:49 ` [Bug translator/2149] " joshua dot i dot stone at intel dot com
2006-01-12 20:56 ` joshua dot i dot stone at intel dot com
@ 2006-01-12 21:07 ` hunt at redhat dot com
2006-01-16 15:57 ` fche at redhat dot com
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: hunt at redhat dot com @ 2006-01-12 21:07 UTC (permalink / raw)
To: systemtap
------- Additional Comments From hunt at redhat dot com 2006-01-12 21:07 -------
> Sorry, this is wrong - it's actually a FIFO. To be LRU we need to move nodes to
> the tail when they're accessed as well. I think this would be preferable...
It's a little more complicated than that. When wrapping, it removes the first
node in the node list. That would typically be the first node created, kind of
FIFO. Except you can sort the list and change the ordering. If we enable wrap
mode, it should be per-map and should be used for things like FIFOs or lists.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
` (2 preceding siblings ...)
2006-01-12 21:07 ` hunt at redhat dot com
@ 2006-01-16 15:57 ` fche at redhat dot com
2006-01-16 18:23 ` hunt at redhat dot com
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: fche at redhat dot com @ 2006-01-16 15:57 UTC (permalink / raw)
To: systemtap
------- Additional Comments From fche at redhat dot com 2006-01-16 15:57 -------
(In reply to comment #1)
> I don't want to belittle the importance of checking return values, but filled
> maps could be avoided by turning on map->wrap.
The current wrapping scheme seems too complicated to use.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
` (3 preceding siblings ...)
2006-01-16 15:57 ` fche at redhat dot com
@ 2006-01-16 18:23 ` hunt at redhat dot com
2006-04-23 4:32 ` eteo at redhat dot com
2006-04-27 16:46 ` fche at redhat dot com
6 siblings, 0 replies; 8+ messages in thread
From: hunt at redhat dot com @ 2006-01-16 18:23 UTC (permalink / raw)
To: systemtap
------- Additional Comments From hunt at redhat dot com 2006-01-16 18:23 -------
(In reply to comment #4)
> (In reply to comment #1)
> > I don't want to belittle the importance of checking return values, but filled
> > maps could be avoided by turning on map->wrap.
>
> The current wrapping scheme seems too complicated to use.
It depends what you want to use it for. Setting one flag enables it. Current
behaviour is FIFO. Adding a flag to change the behaviour to LRU would be simple.
Only gotcha is that there is only one node list and sorting will reorder the list.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
` (4 preceding siblings ...)
2006-01-16 18:23 ` hunt at redhat dot com
@ 2006-04-23 4:32 ` eteo at redhat dot com
2006-04-27 16:46 ` fche at redhat dot com
6 siblings, 0 replies; 8+ messages in thread
From: eteo at redhat dot com @ 2006-04-23 4:32 UTC (permalink / raw)
To: systemtap
------- Additional Comments From eteo at redhat dot com 2006-04-23 04:32 -------
Before fix, _stp_map_set_xx do not have their return values checked:
[eteo@eteo src]$ stap -DMAXMAPENTRIES=1 -p3 -u -e 'probe begin { a[2]="hello" ;
a[3]="a"} global a' | grep _stp_map_set
_stp_map_set_is (global_a, l->__tmp0, (l->__tmp2[0] ? l->__tmp2 : NULL));
_stp_map_set_is (global_a, l->__tmp4, (l->__tmp6[0] ? l->__tmp6 : NULL));
After applying fix:
[eteo@eteo src]$ stap -DMAXMAPENTRIES=1 -p3 -u -e 'probe begin { a[2]="hello" ;
a[3]="a"} global a' | grep _stp_map_set
{ int rc = _stp_map_set_is (global_a, l->__tmp0, (l->__tmp2[0] ? l->__tmp2
: NULL)); if (unlikely(rc)) c->last_error = "Array overflow, check
MAXMAPENTRIES"; };
{ int rc = _stp_map_set_is (global_a, l->__tmp4, (l->__tmp6[0] ? l->__tmp6
: NULL)); if (unlikely(rc)) c->last_error = "Array overflow, check
MAXMAPENTRIES"; };
[eteo@eteo src]$ stap -DMAXMAPENTRIES=1 -u -e 'probe begin { a[2]="hello" ;
a[3]="a"} global a'
ERROR: Array overflow, check MAXMAPENTRIES near identifier 'a' at <input>:1:30
WARNING: Number of errors: 1, skipped probes: 0
Here's the patch. Please feedback.
Index: ChangeLog
===================================================================
RCS file: /cvs/systemtap/src/ChangeLog,v
retrieving revision 1.360
diff -u -3 -r1.360 ChangeLog
--- ChangeLog 23 Apr 2006 03:28:45 -0000 1.360
+++ ChangeLog 23 Apr 2006 04:24:42 -0000
@@ -1,5 +1,11 @@
2006-04-23 Eugene Teo <eteo@redhat.com>
+ PR 2149
+ * translate.cxx (mapvar::set): Test _stp_map_set_xx() for
+ array overflows.
+
+2006-04-23 Eugene Teo <eteo@redhat.com>
+
* small_demos/ansi_colors.stp: Add an example of using octal
escape sequences to display all possible ansi colors.
Index: translate.cxx
===================================================================
RCS file: /cvs/systemtap/src/translate.cxx,v
retrieving revision 1.114
diff -u -3 -r1.114 translate.cxx
--- translate.cxx 22 Apr 2006 06:46:00 -0000 1.114
+++ translate.cxx 23 Apr 2006 04:24:46 -0000
@@ -581,14 +581,20 @@
string set (vector<tmpvar> const & indices, tmpvar const & val) const
{
+ string res = "{ int rc = ";
+
// impedance matching: empty strings -> NULL
if (type() == pe_string)
- return (call_prefix("set", indices)
+ res += (call_prefix("set", indices)
+ ", (" + val.qname() + "[0] ? " + val.qname() + " : NULL))");
else if (type() == pe_long)
- return (call_prefix("set", indices) + ", " + val.qname() + ")");
+ res += (call_prefix("set", indices) + ", " + val.qname() + ")");
else
throw semantic_error("setting a value of an unsupported map type");
+
+ res += "; if (unlikely(rc)) c->last_error = \"Array overflow, check
MAXMAPENTRIES\"; }";
+
+ return res;
}
string hist() const
--
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |WAITING
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug translator/2149] check return value for _stp_map_set_xx()
2006-01-12 18:19 [Bug translator/2149] New: check return value for _stp_map_set_xx() hunt at redhat dot com
` (5 preceding siblings ...)
2006-04-23 4:32 ` eteo at redhat dot com
@ 2006-04-27 16:46 ` fche at redhat dot com
6 siblings, 0 replies; 8+ messages in thread
From: fche at redhat dot com @ 2006-04-27 16:46 UTC (permalink / raw)
To: systemtap
------- Additional Comments From fche at redhat dot com 2006-04-27 16:46 -------
patch committed
--
What |Removed |Added
----------------------------------------------------------------------------
Status|WAITING |RESOLVED
Resolution| |FIXED
http://sourceware.org/bugzilla/show_bug.cgi?id=2149
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.
^ permalink raw reply [flat|nested] 8+ messages in thread