https://bugzilla.novell.com/show_bug.cgi?id=223718 matz@novell.com changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mhopf@novell.com, matz@novell.com AssignedTo|matz@novell.com |sndirsch@novell.com Status|ASSIGNED |NEW ------- Comment #24 from matz@novell.com 2006-11-27 14:34 MST ------- Hmm, okay, maybe I've missing something in my analysis as r11 really is zero here. So [r28] is zero, but as I said we should be only able to get here after the check much earlier that [r28] is in fact not zero. I wonder if something was able to somehow overwrite or reset that memory. The load of r28 is some instructions before the call to GetTimeInMillis(), as is also a compare of that place with zero. The code reads like so in the .s file: lwz 28,.LC30-.LCTOC1(30) LVL150: .loc 1 200 0 li 0,0 stw 0,8(1) LVL151: .loc 1 201 0 lwz 9,0(28) cmpwi 7,9,0 beq 7,.L188 .loc 1 203 0 bl GetTimeInMillis+32768@plt You should be able to locate it and set a breakpoint onto the "lwz r9,0(28)" instruction. There you can see if the [r28] memory is zero or not. Also set a breakpoint to the load on 0x1018d2d4 (from comment #22), there it's loaded the second time. It should still be nonzero. If not then something in between the first and the second check did change the variable. .. Hmm, actually that might happen indeed. The first check is before the call to CheckAllTimers (later inlined), which calls TimerForce() which can change the static "timers" variable. So, actually the code is wrong. Again, the initial code looks like: if (timers) { now = GetTimeInMillis(); timeout = timers->expires - now; if (timeout > 0 && timeout > timers->delta + 250) { /* time has rewound. reset the timers. */ CheckAllTimers(now); timeout = timers->expires - now; } if (timeout < 0) timeout = 0; waittime.tv_sec = timeout / MILLI_PER_SECOND; waittime.tv_usec = (timeout % MILLI_PER_SECOND) * (1000000 / MILLI_PER_SECOND); wt = &waittime; } So it checks "timers" for NULL, then calls CheckAllTimers, and then tries to access timers->expires after that call again, without further checking. But CheckAllTimers calls TimerForce, which in turn calls DoTimer() which overwrites it's given "previous timer" argument, which might be &timers. So the call to CheckAllTimers might make "timers" zero, so the access after that call really needs to check again for nullness. As this is sensitive code it needs probably some carefull thought. I think it's required that if "timers" is zero at that point that "wt" is set to NULL and not to &waittime. The timers list is sorted per wakeup time, so if there's no timer then the process does not need to wakeup at all. I think this should work (sometimes it does a little too much work, but otherwise it's a bit ugly to implement this without goto's): if (timers) { now = GetTimeInMillis(); timeout = timers->expires - now; if (timeout > 0 && timeout > timers->delta + 250) { /* time has rewound. reset the timers. */ CheckAllTimers(now); } /* timers might have changed by CheckAllTimers */ if (timers) { timeout = timers->expires - now; if (timeout < 0) timeout = 0; waittime.tv_sec = timeout / MILLI_PER_SECOND; waittime.tv_usec = (timeout % MILLI_PER_SECOND) * (1000000 / MILLI_PER_SECOND); wt = &waittime; } else { wt = NULL; } } -- Configure bugmail: https://bugzilla.novell.com/userprefs.cgi?tab=email ------- You are receiving this mail because: ------- You are on the CC list for the bug, or are watching someone who is.