199901 - OS/2 - Cookies don't work (PR_snprintf problem)

Status: RESOLVED FIXED

(NSPR :: NSPR, defect)

Description

[Felix Miata]

OS/2 trunk builds. Works OK as of 2003031312. Broken in 2003033012 going back at
least 6 days.

%d is written in place of integer for expire time in cookies.txt, causing
corruption of file and loss of all cookies saved in prior sessions.

Comment 1

[[e-mail]]

-> mkaply

confirmed - we spent a while debugging this one. os/2 only, works on other
platforms...

looks like the recent cookie rewrite has exposed a problem in PR_snprintf. we
have these two datapoints:

0312 build, the following line produces a correctly formatted integer:

  PR_snprintf(dateString, sizeof(dateString), "%lu", some_unsigned_long);

0330 build, the following line produces "%d":

  PR_snprintf(dateString, sizeof(dateString), "%lld", some_PRInt64);

so what we know for sure is that os/2 handling of "%lld"/PRInt64's is busted in
0330, but whether this was always the case or is due to some recent os/2
checkins i'm unsure. the symptom of this is that cookies don't persist across
sessions, since the cookie file is corrupted.

mkaply?

Comment 2

[Felix Miata]

Further testing results:
2003032209 is broken
2003032112 is OK

Comment 3

[[e-mail]]

great! so this confirms it was most probably just the change to "%lld" from
"%lu" that broke things - so PR_snprintf has probably been broken on os/2 for
some time.

mkaply: should this be a blocker?

Comment 4

[[e-mail]]

oh, one random note:

PR_sscanf works fine with %lld on the affected builds:

  PRInt32 numInts = PR_sscanf(dateString, "%lld", &some_PRInt64);

Comment 5

[Mike Kaply [:mkaply]]

Where are the PR_snprintfs in question?

On OS/2, we have a major issue where time_t is represented as a double and must
be case to a long for this stuff.

I'm wondering if this is the cause (or something related)

PR_snprintf is a totally NSPR call, so if %lld should be handled by NSPR.

Honestly, at this point it looks like the VACPP build is suddenly falling apart.
I can't even compile on my machine anymore (brings down the kernel)

Comment 6

[[e-mail]]

mkaply: thanks for the quick response. to answer...

according to prprf.h, %lld should be supported. see
http://lxr.mozilla.org/seamonkey/source/nsprpub/pr/include/prprf.h#39

and the PR_snprintf in question is
http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookies.cpp#1799

this doesn't involve time_t, it's purely PRInt64 arithmetic, so i'm not quite
sure what you meant.

Comment 7

[[e-mail]]

uhh, sorry... second link should be
http://lxr.mozilla.org/seamonkey/source/extensions/cookie/nsCookies.cpp#1979

and, best of luck with VACPP. ;)

Comment 8

[Mike Kaply [:mkaply]]

Optmizer bug in NSPR.

I was able to fix it by unoptimizing prprf.c.

Dang.

Comment 9

[Mike Kaply [:mkaply]]

Moving to NSPR

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Turn off optimization for VACPP

I give up.

We're turning off optimization for VACPP for NSPR.

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 118965 [details] [diff] [review]
Turn off optimization for VACPP

OS/2 builds haven't had cookies working for a week now we discovered (since the
cookie landing)

Comment 12

[hacker formerly known as [e-mail]]

Comment on attachment 118965 [details] [diff] [review]
Turn off optimization for VACPP

Turning optimizations off for the entire NSPR tree seems a bit exccessive but I
guess it's not going to be around that much longer anyway.

Comment 13

[Wan-Teh Chang]

Comment on attachment 118965 [details] [diff] [review]
Turn off optimization for VACPP

Ideally you should identify the files and only turn off
optimization on those files.  Have you given up on that,
too?

Comment 14

[Mike Kaply [:mkaply]]

There is no easy method to turn off optimization on individual files.

If you have a really good suggestion for that, that would be great.

I'm just getting tired of optimizer bugs.

Comment 15

[Wan-Teh Chang]

mozilla/nsprpub/pr/src/misc/Makefile.in, rev. 1.15 has an
example of turning off optimization on a specific file.

# This suppresses optimization for this single compilation unit.
ifeq ($(OS_ARCH), AIX)
$(OBJDIR)/prdtoa.o: prdtoa.c
        @$(MAKE_OBJDIR)
        $(CC) -o $@ -c $(filter-out -O, $(CFLAGS)) $<
endif

In comment 8, you said you can fix it by unoptimizing prprf.c.
I suggest you apply a similar makefile patch to
mozilla/nsprpub/pr/src/io/Makefile.in.

Comment 16

[Mike Kaply [:mkaply]]

Attachment: Just turn off opt for prprf.c

Comment 17

[Wan-Teh Chang]

Comment on attachment 118993 [details] [diff] [review]
Just turn off opt for prprf.c

r=wtc.

Requesting mozilla 1.4a approval.  This is an OS/2-only makefile change.

Comment 18

[Asa Dotzler [:asa]]

Comment on attachment 118993 [details] [diff] [review]
Just turn off opt for prprf.c

a=asa (on behalf of drivers) for checkin to 1.4a. Time is short so this needs
to land quickly.

Comment 19

[hacker formerly known as [e-mail]]

The patch has been checked in on the client branch & the NSPR trunk.