Closed Bug 226022 Opened 17 years ago Closed 17 years ago

overflow error in PR_Poll means timeout < 1 second returns too quickly

Categories

(NSPR :: NSPR, defect)

x86
All
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: bmo, Assigned: wtc)

Details

Attachments

(1 file, 1 obsolete file)

User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b; MultiZilla v1.5.0.1) Gecko/20031007
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.5b; MultiZilla v1.5.0.1) Gecko/20031007

The following code or code similar to it is used in a number of places within NSPR.

  PRInt32 ticksPerSecond = PR_TicksPerSecond();
  tv.tv_sec = timeout / ticksPerSecond;
  tv.tv_usec = timeout - (ticksPerSecond * tv.tv_sec);
  tv.tv_usec = (PR_USEC_PER_SEC * tv.tv_usec) / ticksPerSecond;

This will overflow on a 32 bit system with times greater than 0.058 seconds.
Examples from Windows:

0.95 secs, correct = 950000 us, current = 8732
0.90 secs, correct = 900000 us, current = 17566
0.85 secs, correct = 850000 us, current = 26386
0.80 secs, correct = 800000 us, current = 35221
0.75 secs, correct = 750000 us, current = 44055
0.70 secs, correct = 700000 us, current = 52875
0.65 secs, correct = 650000 us, current = 2881
0.60 secs, correct = 600000 us, current = 11701
0.55 secs, correct = 550000 us, current = 20536
0.50 secs, correct = 500000 us, current = 29370
0.45 secs, correct = 450000 us, current = 38190
0.40 secs, correct = 400000 us, current = 47024
0.35 secs, correct = 350000 us, current = 55845
0.30 secs, correct = 300000 us, current = 5850
0.25 secs, correct = 250000 us, current = 14685
0.20 secs, correct = 200000 us, current = 23505
0.15 secs, correct = 150000 us, current = 32339
0.10 secs, correct = 100000 us, current = 41160
0.059 secs, correct = 59000 us, current = 164
0.058 secs, correct = 58000 us, current = 57993 ** correct **
0.05 secs, correct =  50000 us, current = 49994 ** correct **

One correct version of this is:

  PRInt32 ticksPerSecond = PR_TicksPerSecond();
  tv.tv_sec = timeout / ticksPerSecond;
  tv.tv_usec = timeout - (ticksPerSecond * tv.tv_sec);
  tv.tv_usec = PR_IntervalToMicroseconds( timeout % ticksPerSecond );

The files I've found this in so far are (mostly PR_Poll):

nsprpub\pr\src\md\beos\bfile.c(692)
nsprpub\pr\src\md\os2\os2poll.c(240)
nsprpub\pr\src\md\unix\uxpoll.c(404)
nsprpub\pr\src\md\windows\w32poll.c(265) 


Reproducible: Always

Steps to Reproduce:
Try the following code:

    struct timeval tv, *tvp = NULL;
    PRInt32 ticksPerSecond = PR_TicksPerSecond();
    for ( int nMult = 999; nMult > 0; --nMult )
    {
        PRIntervalTime timeout = (PRUint32) (ticksPerSecond * nMult / 1000.0);
        tv.tv_sec = timeout / ticksPerSecond;
        tv.tv_usec = timeout - (ticksPerSecond * tv.tv_sec);
        tv.tv_usec = (PR_USEC_PER_SEC * tv.tv_usec) / ticksPerSecond;
        printf( "0.%03d secs, correct = %d000 us, current = %ld\n", nMult,
nMult, tv.tv_usec );
        tv.tv_usec = (tv.tv_usec + 500) / 1000;
        NS_ABORT_IF_FALSE( nMult <= 58 || tv.tv_usec != nMult, "valid! what a
surprise!" );
        tv.tv_usec = PR_IntervalToMicroseconds( timeout % ticksPerSecond );
        tv.tv_usec = (tv.tv_usec + 500) / 1000;
        NS_ABORT_IF_FALSE( tv.tv_usec == nMult, "not valid!" );
    }
Copy and paste error with the correct version of code.

  PRInt32 ticksPerSecond = PR_TicksPerSecond();
  tv.tv_sec = timeout / ticksPerSecond;
  tv.tv_usec = PR_IntervalToMicroseconds( timeout % ticksPerSecond );

is what I meant. 
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attached patch proposed patch (obsolete) — Splinter Review
Attached patch proposed patch 2Splinter Review
Using tabs in the beos file to match the current source.

I don't have access to build systems for anything other than Windows. I have
tested it on Windows (as the NSPR library, not with Mozilla) and it works as
expected. The supplied test code could easily to used on other platforms to
test. I don't envisage problems, but as always...
Attachment #135772 - Attachment is obsolete: true
Comment on attachment 135774 [details] [diff] [review]
proposed patch 2

r=wtc.	This patch is good.
Attachment #135774 - Flags: review+
Comment on attachment 135774 [details] [diff] [review]
proposed patch 2

sr=darin
Attachment #135774 - Flags: superreview+
fixed on NSPR trunk and NSPRPUB_PRE_4_2_CLIENT_BRANCH
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Target Milestone: --- → 4.5
You need to log in before you can comment on or make changes to this bug.