* [Bug translator/2156] New: check return value for _stp_pmap_agg()
@ 2006-01-16 8:35 hunt at redhat dot com
2006-01-18 2:10 ` [Bug translator/2156] " joshua dot i dot stone at intel dot com
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: hunt at redhat dot com @ 2006-01-16 8:35 UTC (permalink / raw)
To: systemtap
When _stp_pmap_agg() returns NULL, something has gone wrong. Typically this is
because the map size was too small.
--
Summary: check return value for _stp_pmap_agg()
Product: systemtap
Version: unspecified
Status: NEW
Severity: normal
Priority: P2
Component: translator
AssignedTo: systemtap at sources dot redhat dot com
ReportedBy: hunt at redhat dot com
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
@ 2006-01-18 2:10 ` joshua dot i dot stone at intel dot com
2006-01-18 3:05 ` fche at redhat 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-18 2:10 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-18 02:10 -------
patch committed
translate.cxx 1.96
--
What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution| |FIXED
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
2006-01-18 2:10 ` [Bug translator/2156] " joshua dot i dot stone at intel dot com
@ 2006-01-18 3:05 ` fche at redhat dot com
2006-01-18 3:35 ` joshua dot i dot stone at intel dot com
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: fche at redhat dot com @ 2006-01-18 3:05 UTC (permalink / raw)
To: systemtap
------- Additional Comments From fche at redhat dot com 2006-01-18 03:05 -------
(In reply to comment #1)
> patch committed
> translate.cxx 1.96
Is there a test case for this scenario?
The new code looks suspicious for the unsorted case.
--
What |Removed |Added
----------------------------------------------------------------------------
GCC build triplet| |joshua.i.stone@intel.com
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
2006-01-18 2:10 ` [Bug translator/2156] " joshua dot i dot stone at intel dot com
2006-01-18 3:05 ` fche at redhat dot com
@ 2006-01-18 3:35 ` joshua dot i dot stone at intel dot com
2006-01-18 3:45 ` fche at redhat dot com
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: joshua dot i dot stone at intel dot com @ 2006-01-18 3:35 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-18 03:35 -------
(In reply to comment #2)
> Is there a test case for this scenario?
> The new code looks suspicious for the unsorted case.
buildok/pmap_foreach.stp tests the unsorted case - I just modified it to test
the sorted case as well.
What looks suspicious to you?
--
What |Removed |Added
----------------------------------------------------------------------------
CC| |joshua dot i dot stone at
| |intel dot com
GCC build triplet|joshua.i.stone@intel.com |
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
` (2 preceding siblings ...)
2006-01-18 3:35 ` joshua dot i dot stone at intel dot com
@ 2006-01-18 3:45 ` fche at redhat dot com
2006-01-18 18:06 ` hunt at redhat dot com
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: fche at redhat dot com @ 2006-01-18 3:45 UTC (permalink / raw)
To: systemtap
------- Additional Comments From fche at redhat dot com 2006-01-18 03:45 -------
> buildok/pmap_foreach.stp tests the unsorted case - I just modified it to test
> the sorted case as well.
OK, but that tests only buildability. Is there a run-time test?
> What looks suspicious to you?
The conditional emission of an "else" (only in the sorted case) is a tip.
Looking at the generated code in both cases, stp_pmap_get_agg() can still
be called and the result used, in the stp_map_start call.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
` (3 preceding siblings ...)
2006-01-18 3:45 ` fche at redhat dot com
@ 2006-01-18 18:06 ` hunt at redhat dot com
2006-01-18 18:54 ` joshua dot i dot stone at intel dot com
2006-01-18 21:00 ` joshua dot i dot stone at intel dot com
6 siblings, 0 replies; 8+ messages in thread
From: hunt at redhat dot com @ 2006-01-18 18:06 UTC (permalink / raw)
To: systemtap
------- Additional Comments From hunt at redhat dot com 2006-01-18 18:06 -------
Looking at the latest generated code, it does this
if (unlikely(NULL == _stp_pmap_agg (global_foo)))
c->last_error = "unknown error while aggregating global_foo";
write_unlock (& global_foo_lock);
post_unlock_3: ;
read_lock (& global_foo_lock);
l->__tmp19 = _stp_map_start (_stp_pmap_get_agg(global_foo));
[...]
_stp_pmap_agg() will return NULL if the map pointer is invalid, the map type in
unknown, or it overflows. The first two should be impossible with code from the
translator, so I suggest changing the error message to indicate an overflow
happened and suggest the map size needs increased.
_stp_pmap_get_agg() always returns a valid pointer but in the case above the
aggragated map will not be complete. So printing it anyway is safe, but
potentially misleading.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
` (4 preceding siblings ...)
2006-01-18 18:06 ` hunt at redhat dot com
@ 2006-01-18 18:54 ` joshua dot i dot stone at intel dot com
2006-01-18 21:00 ` joshua dot i dot stone at intel dot com
6 siblings, 0 replies; 8+ messages in thread
From: joshua dot i dot stone at intel dot com @ 2006-01-18 18:54 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-18 18:54 -------
(In reply to comment #4)
> > buildok/pmap_foreach.stp tests the unsorted case - I just modified it to test
> > the sorted case as well.
>
> OK, but that tests only buildability. Is there a run-time test?
I will make sure that there's a runtime test. If possible, I will also try to
create tests of the failure case as well.
(In reply to comment #5)
> _stp_pmap_agg() will return NULL if the map pointer is invalid, the map type in
> unknown, or it overflows. The first two should be impossible with code from the
> translator, so I suggest changing the error message to indicate an overflow
> happened and suggest the map size needs increased.
Ok, I will change the message as suggested.
> _stp_pmap_get_agg() always returns a valid pointer but in the case above the
> aggragated map will not be complete. So printing it anyway is safe, but
> potentially misleading.
There won't be anything misleading, because the first statement within the
foreach (maybe a print, or anything) will check c->last_error before doing
anything. Thus the error is delayed a little bit, but as long as the operations
between the error and its detection are safe, there's no problem. I see no
reason to optimize for the error case and try to catch it sooner.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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/2156] check return value for _stp_pmap_agg()
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
` (5 preceding siblings ...)
2006-01-18 18:54 ` joshua dot i dot stone at intel dot com
@ 2006-01-18 21:00 ` joshua dot i dot stone at intel dot com
6 siblings, 0 replies; 8+ messages in thread
From: joshua dot i dot stone at intel dot com @ 2006-01-18 21:00 UTC (permalink / raw)
To: systemtap
------- Additional Comments From joshua dot i dot stone at intel dot com 2006-01-18 21:00 -------
Runtime tests are now in CVS
tests/testsuite/systemtap.maps/ix.*
Modified to test successful pmap aggregation for sorted and unsorted cases.
tests/testsuite/systemtap.maps/pmap_agg_overflow.*
New test to verify the failing case is handled properly.
--
http://sourceware.org/bugzilla/show_bug.cgi?id=2156
------- 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
end of thread, other threads:[~2006-01-18 21:00 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2006-01-16 8:35 [Bug translator/2156] New: check return value for _stp_pmap_agg() hunt at redhat dot com
2006-01-18 2:10 ` [Bug translator/2156] " joshua dot i dot stone at intel dot com
2006-01-18 3:05 ` fche at redhat dot com
2006-01-18 3:35 ` joshua dot i dot stone at intel dot com
2006-01-18 3:45 ` fche at redhat dot com
2006-01-18 18:06 ` hunt at redhat dot com
2006-01-18 18:54 ` joshua dot i dot stone at intel dot com
2006-01-18 21:00 ` joshua dot i dot stone at intel dot com
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).