* [PATCH] sim: fix signed 32bit time overflow with long delayed events
@ 2010-04-20 11:16 Mike Frysinger
2010-04-23 2:30 ` Mike Frysinger
2010-12-31 23:01 ` [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows Mike Frysinger
0 siblings, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2010-04-20 11:16 UTC (permalink / raw)
To: gdb-patches
The sim-events code jumps through some hoops to avoid using 64bit math
to manage the current time. One fundamental assumption here is that by
constantly scheduling the sim poll event a short time into the future,
the 64bit difference will always fall into a signed 32bit value. This
does work most of the time, except for when processing the sim poll event
itself.
Normally, sim_events_process() will dequeue the sim poll event, update
the current time (time_from_event) according to the next pending event,
process the sim poll event (which will then requeue the sim poll event),
and then continue on.
The problem here of course is that the current time is updated in that
small window before the sim poll event gets a chance to reschedule itself.
So if the 64bit difference between the current time and the next event
does not fit into the signed 32bit value, time_from_event overflows, and
the internal assert at the end of update_time_from_event() triggers.
This was noticed when simulating Blackfin Das U-Boot. The simulated core
timer is given the max unsigned timeout value possible on a 32bit processor
(0xffffffff). This timeout value is used directly to schedule a hw event
in the sim future (the IRQ firing). Once the sim poll event is kicked off,
the next pending event is the core timer event which is more than 2^31
ticks in the future, and the sim aborts with:
sim-events.c:435: assertion failed - current_time == sim_events_time (sd)
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
---
2010-04-20 Mike Frysinger <vapier@gentoo.org>
* sim-events.c (sim_events_process): Delay update_time_from_event()
till after the call to handler().
sim/common/sim-events.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)
diff --git a/sim/common/sim-events.c b/sim/common/sim-events.c
index e02fb76..3fa5433 100644
--- a/sim/common/sim-events.c
+++ b/sim/common/sim-events.c
@@ -1172,7 +1172,6 @@ sim_events_process (SIM_DESC sd)
sim_event_handler *handler = to_do->handler;
void *data = to_do->data;
events->queue = to_do->next;
- update_time_from_event (sd);
ETRACE((_ETRACE,
"event issued at %ld - tag 0x%lx - handler 0x%lx, data 0x%lx%s%s\n",
(long) event_time,
@@ -1183,6 +1182,10 @@ sim_events_process (SIM_DESC sd)
(to_do->trace != NULL) ? to_do->trace : ""));
sim_events_free (sd, to_do);
handler (sd, data);
+ /* Update time *after* the handler in case it inserted an event itself.
+ Like in the case of the persistent sim poll event. Otherwise, our
+ 32bit math assumption may be violated and overflow. */
+ update_time_from_event (sd);
}
/* put things back where they belong ready for the next iteration */
--
1.7.0.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sim: fix signed 32bit time overflow with long delayed events
2010-04-20 11:16 [PATCH] sim: fix signed 32bit time overflow with long delayed events Mike Frysinger
@ 2010-04-23 2:30 ` Mike Frysinger
2010-12-31 23:01 ` [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows Mike Frysinger
1 sibling, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2010-04-23 2:30 UTC (permalink / raw)
To: gdb-patches
[-- Attachment #1: Type: Text/Plain, Size: 242 bytes --]
NAK this patch for now. it sometimes causes a different assert to trigger.
i'll have to read this events code a bit more to figure out a way to avoid
both issues. unless of course someone who knows this code has a solution ;).
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows
2010-04-20 11:16 [PATCH] sim: fix signed 32bit time overflow with long delayed events Mike Frysinger
2010-04-23 2:30 ` Mike Frysinger
@ 2010-12-31 23:01 ` Mike Frysinger
2011-02-07 6:10 ` Mike Frysinger
2011-02-14 4:09 ` Joel Brobecker
1 sibling, 2 replies; 6+ messages in thread
From: Mike Frysinger @ 2010-12-31 23:01 UTC (permalink / raw)
To: gdb-patches; +Cc: toolchain-devel
The sim-events code jumps through some hoops to avoid using 64bit math
to manage the current time. One fundamental assumption here is that by
constantly scheduling the sim poll event a short time into the future,
the 64bit difference will always fall into a signed 32bit value. This
does work most of the time, except for when processing the sim poll event
itself.
Normally, sim_events_process() will dequeue the sim poll event, update
the current time (time_from_event) according to the next pending event,
process the sim poll event (which will then requeue the sim poll event),
and then continue on.
The problem here of course is that the current time is updated in that
small window before the sim poll event gets a chance to reschedule itself.
So if the 64bit difference between the current time and the next event
does not fit into the signed 32bit value, time_from_event overflows, and
the internal assert at the end of update_time_from_event() triggers.
Since attempts at tweaking sim_events_process() logic introduced other
subtle bugs (due to tangled assumptions between most pieces of the sim
time keeping code), change the time_from_event to a real 64bit value.
Tests on my system between a 32bit ELF and a 64bit ELF show no practical
difference (it's all lost in the system noise). Basically, I booted a
Linux kernel to userspace and then paniced it; this gave me a constant
sample size of about 18 million insns.
This was noticed when simulating Blackfin Das U-Boot. The simulated core
timer is given the max unsigned timeout value possible on a 32bit processor
(0xffffffff). This timeout value is used directly to schedule a hw event
in the sim future (the IRQ firing). Once the sim poll event is kicked off,
the next pending event is the core timer event which is more than 2^31
ticks in the future, and the sim aborts with:
sim-events.c:435: assertion failed - current_time == sim_events_time (sd)
Signed-off-by: Mike Frysinger <vapier@gentoo.org>
2010-12-31 Mike Frysinger <vapier@gentoo.org>
* sim-events.h (_sim_events.time_from_event): Change type to signed64.
---
sim/common/sim-events.h | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sim/common/sim-events.h b/sim/common/sim-events.h
index cc22439..e86dc18 100644
--- a/sim/common/sim-events.h
+++ b/sim/common/sim-events.h
@@ -93,7 +93,7 @@ struct _sim_events {
unsigned long elapsed_wallclock;
SIM_ELAPSED_TIME resume_wallclock;
signed64 time_of_event;
- int time_from_event;
+ signed64 time_from_event;
int trace;
};
--
1.7.3.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows
2010-12-31 23:01 ` [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows Mike Frysinger
@ 2011-02-07 6:10 ` Mike Frysinger
2011-02-14 4:09 ` Joel Brobecker
1 sibling, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2011-02-07 6:10 UTC (permalink / raw)
To: gdb-patches; +Cc: toolchain-devel
[-- Attachment #1: Type: Text/Plain, Size: 2104 bytes --]
On Friday, December 31, 2010 17:24:26 Mike Frysinger wrote:
> The sim-events code jumps through some hoops to avoid using 64bit math
> to manage the current time. One fundamental assumption here is that by
> constantly scheduling the sim poll event a short time into the future,
> the 64bit difference will always fall into a signed 32bit value. This
> does work most of the time, except for when processing the sim poll event
> itself.
>
> Normally, sim_events_process() will dequeue the sim poll event, update
> the current time (time_from_event) according to the next pending event,
> process the sim poll event (which will then requeue the sim poll event),
> and then continue on.
>
> The problem here of course is that the current time is updated in that
> small window before the sim poll event gets a chance to reschedule itself.
> So if the 64bit difference between the current time and the next event
> does not fit into the signed 32bit value, time_from_event overflows, and
> the internal assert at the end of update_time_from_event() triggers.
>
> Since attempts at tweaking sim_events_process() logic introduced other
> subtle bugs (due to tangled assumptions between most pieces of the sim
> time keeping code), change the time_from_event to a real 64bit value.
> Tests on my system between a 32bit ELF and a 64bit ELF show no practical
> difference (it's all lost in the system noise). Basically, I booted a
> Linux kernel to userspace and then paniced it; this gave me a constant
> sample size of about 18 million insns.
>
> This was noticed when simulating Blackfin Das U-Boot. The simulated core
> timer is given the max unsigned timeout value possible on a 32bit processor
> (0xffffffff). This timeout value is used directly to schedule a hw event
> in the sim future (the IRQ firing). Once the sim poll event is kicked off,
> the next pending event is the core timer event which is more than 2^31
> ticks in the future, and the sim aborts with:
> sim-events.c:435: assertion failed - current_time == sim_events_time (sd)
ping ...
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows
2010-12-31 23:01 ` [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows Mike Frysinger
2011-02-07 6:10 ` Mike Frysinger
@ 2011-02-14 4:09 ` Joel Brobecker
2011-02-14 5:15 ` Mike Frysinger
1 sibling, 1 reply; 6+ messages in thread
From: Joel Brobecker @ 2011-02-14 4:09 UTC (permalink / raw)
To: Mike Frysinger; +Cc: gdb-patches, toolchain-devel
> 2010-12-31 Mike Frysinger <vapier@gentoo.org>
>
> * sim-events.h (_sim_events.time_from_event): Change type to signed64.
This is OK.
--
Joel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows
2011-02-14 4:09 ` Joel Brobecker
@ 2011-02-14 5:15 ` Mike Frysinger
0 siblings, 0 replies; 6+ messages in thread
From: Mike Frysinger @ 2011-02-14 5:15 UTC (permalink / raw)
To: Joel Brobecker; +Cc: gdb-patches, toolchain-devel
[-- Attachment #1: Type: Text/Plain, Size: 246 bytes --]
On Sunday, February 13, 2011 23:08:46 Joel Brobecker wrote:
> > 2010-12-31 Mike Frysinger <vapier@gentoo.org>
> >
> > * sim-events.h (_sim_events.time_from_event): Change type to signed64.
>
> This is OK.
thanks, committed !
-mike
[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-02-14 5:15 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-04-20 11:16 [PATCH] sim: fix signed 32bit time overflow with long delayed events Mike Frysinger
2010-04-23 2:30 ` Mike Frysinger
2010-12-31 23:01 ` [PATCH] sim: change to 64bit time keeping to avoid 32bit overflows Mike Frysinger
2011-02-07 6:10 ` Mike Frysinger
2011-02-14 4:09 ` Joel Brobecker
2011-02-14 5:15 ` Mike Frysinger
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).