Closed Bug 59652 Opened 24 years ago Closed 13 years ago

Numerous places where uninitialized variables might be being used before being set.

Categories

(Core Graveyard :: Tracking, defect, P3)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: rich.burridge, Assigned: atulagrwl)

References

(Blocks 1 open bug)

Details

(Keywords: meta)

Attachments

(4 files, 9 obsolete files)

(deleted), text/plain
Details
(deleted), text/plain
Details
(deleted), patch
Details | Diff | Splinter Review
(deleted), text/plain
Details
We have seen a couple of bugs (58698 and 59617) where crashes have resulted because variables have been used without being previously initialized. By building Mozilla (with mail/news) with the -Wuninitialized flag and with optimization turned on, I was able to find 192 occurances of this in total (in 109 files). These problems could be the cause of many "random" crashes on platforms which don't automatically initialize local variables to zero (Solaris being one of them). See the first attachment, which contains the compile transcript for all of these 109 files. Even though Netscape 6 RTM is almost out the door, we would like to fix up these problems for our Solaris Netscape 6 RTM/FCS release, so if diffs could be attached to this bug for the various fixes (note the bugs 58698 and 59617 already contain two such sets of diffs), then it would be very much appreciated. Thanks.
The best would be if you could split the log between different modules and file a bug for each. My experience of bugs like this that covers many components is that noone takes it on.
I've open a load of individual bugs in various modules for this problem. Hopefully that will inspire the module owners to create diffs for the files that they own. These are: 59657 - XPCOM 59658 - XPCOM / Registry 59659 - JavaScript Engine 59661 - Networking 59664 - Security / General 59666 - Internationalization 59667 - Imagelib 59670 - Installer 59668 - GTX Embedding 59669 - HTML Element 59673 - Mail / News 59675 - Layout 59676 - XPToolkit / Widgets 59677 - DOM 59678 - Viewer 58698 - Editor The following lines still need to have individual module bugs filed against them. I'll do this tomorrow. If anybody can tell me the modules, I'd be very grateful. /db/mork/src/morkAtom.cpp morkAtom.cpp:363: warning: `const mork_u1 * body' might be used uninitialized in this function morkAtom.cpp:407: warning: `mork_size size' might be used uninitialized in this morkAtom.cpp:408: warning: `mork_cscode form' might be used uninitialized in this function morkAtom.cpp:432: warning: `mork_size thisSize' might be used uninitialized in this function morkAtom.cpp:433: warning: `mork_cscode thisForm' might be used uninitialized in this function ---- /rdf/base/src/nsRDFContentSink.cpp nsRDFContentSink.cpp:538: warning: `nsresult rv' might be used uninitialized in ---- /rdf/content/src/nsXULSortService.cpp nsXULSortService.cpp:1416: warning: `nsresult rv' might be used uninitialized in this function ---- /docshell/base/nsDocShell.cpp nsDocShell.cpp:2114: warning: `PRInt32 x' might be used uninitialized in this function nsDocShell.cpp:2115: warning: `PRInt32 y' might be used uninitialized in this function ---- /xpfe/appshell/src/nsWindowMediator.cpp nsWindowMediator.cpp:676: warning: `struct nsWindowInfo * info' might be used uninitialized in this function nsWindowMediator.cpp:678: warning: `PRBool found' might be used uninitialized in this function nsWindowMediator.cpp:805: warning: `struct nsWindowInfo * belowInfo' might be used uninitialized in this function ---- /xpfe/components/search/src/nsInternetSearchService.cpp nsInternetSearchService.cpp:4082: warning: `PRUnichar quoteChar' might be used uninitialized in this function ../../../../dist/include/nsAReadableString.h:1085: warning: `int whichString' might be used uninitialized in this function ---- /extensions/cookie/nsCookie.cpp nsCookie.cpp:397: warning: `PRInt32 typeIndex' might be used uninitialized in this function nsCookie.cpp:1251: warning: `struct permission_HostStruct * hostStruct' might be used uninitialized in this function ---- /extensions/wallet/src/singsign.cpp singsign.cpp:1900: warning: `class si_SignonDataStruct * data' might be used uninitialized in this function ----
I'm not sure about modules, but you should assign the db/mork stuff to bienvenu extensions/wallet to morse docshell to adamlock
[tracking] bug
Summary: Numerous places where uninitialized variables must be being used before being set. → [TRACKING] Numerous places where uninitialized variables must be being used before being set.
Thanks Seth. I've added bienvenu, morse and adamlock to the CC: of this bug. I've already opened up enough others bugs for this one. Guys, with reference to the previous comment by Seth, could you please attach diifs (assuming they are needed), to fix the uninitialized variables for the files that you own? Thanks.
Besides the extensions/wallet/singsign.cpp cases, I'm also responsible for the one in extensions/cookie/nsCookie.cpp. I just checked the two cases in singsign and the one case in nsCookie and none are a problem. Here's why (same reason in all three cases). It's true that the variables in question are not initialized when declared but rather are initialized inside a loop. They are then used after the loop terminates. So the compiler would have no way of knowing if the loop was ever executed and therefore considers these as potentially uninitialized. However, inside the loop another variable changes value. And after the loop, a test is made for that variable changing value and the routine is exited if it didn't happen. Therefore the place that the questionable variable is used will never get executed if the loop had never been entered. Bottom line -- there is no problem here in the nsCookie or singsign files.
Correction: Above I said the two cases in singsign and one in nsCookie. That should read the other way around -- one case in singsign and two in nsCookie.
morse: you should still fix the warnings to get the off the radar.
Assignee: asa → chofmann
Component: Browser-General → Tracking
Keywords: meta
QA Contact: doronr → chofmann
Summary: [TRACKING] Numerous places where uninitialized variables must be being used before being set. → Numerous places where uninitialized variables must be being used before being set.
-> tracking
Added attachment that contains comments regarding the warnings.
Attached file comments on warnings (deleted) —
Changed summary to reflect what the warnings really say.
Summary: Numerous places where uninitialized variables must be being used before being set. → Numerous places where uninitialized variables might be being used before being set.
Cc'ing mang for advice on the fdlibm warnings and scary-looking unbraced but indented as if written in Python code. From my attachment: e_asin.c:110: warning: `t' might be used uninitialized in this function - will get assigned before use. *** Agreed, but the indentation without bracing at lines 125-129 looks very *** wrong. This fdlibm code originated from sun.com, could you please see *** whether a bug needs to be filed there? Thanks. e_exp.c:143: warning: `hi' might be used uninitialized in this function e_exp.c:143: warning: `lo' might be used uninitialized in this function e_exp.c:144: warning: `k' might be used uninitialized in this function - might need to be init. possible to get used without first setting. *** agreed, these are bugs. This fdlibm code originated from sun.com, can *** you see whether a bug needs to be filed there? Thanks. e_rem_pio2.c:124: warning: `z' might be used uninitialized in this function - may get used at line 199 before getting assigned. need to be init/set! e_sqrt.c:132: warning: `z' might be used uninitialized in this function - may get used at line 224 before getting assigned. need to be init/set! k_cos.c:105: warning: `qx' might be used uninitialized in this function - same as the above two, hmm ... are these relying on some runtime stack values? it seems pretty dangerous not to set/init these, but on the other hand, it seems like they are intended. *** The use of qx on line 120 in k_cos.c is clearly wrong, as the next two *** statements completely overwrite u.d. This fdlibm code originated from *** sun.com, could you please see whether a bug needs to be filed there? *** In any case, the JS engine's copy should be fixed to strike line 120. *** Same for the e_sqrt.c and e_rem_pio2.c uses. Who at Sun can follow up with fdlibm authors? George? /be
Hi Brendan: I don't know what fdlibm is, but I'd be happy to try to track it down. What is it? Also, can you tell me some of its history? Maybe I can go from there and track down the authors. Or if you even know a Sun name (probably too much to ask, but I gotta try), I can go from there.
fdlibm is the "freely distributable libm". It provides software implementations of the math functions found in libm. We use fdlibm in the JavaScript engine when a system-provided math function doesn't conform to the IEEE 754 standard. I forget exactly where the fdlibm in mozilla CVS came from. It looks like version 5.2, which was developed by Sun and released in 1995(!) fdlibm is currently distributed as part of netlib, so that might be the right place to send upstream patches (www.netlib.org). NetBSD also maintains a copy of fdlibm, based off version 5.1. They've done some patches for endian-ness problems, and possibly others. I'll have a look (RSN) to see how their current version of our problem files compare. The readme (from 1995) for fdlibm 5.2 (http://www.netlib.org/fdlibm/readme) says: "6. PROBLEMS ? -------------- Please send comments and bug report to: fdlibm-comments@sunpro.eng.sun.com" Might be worth a shot. Another tidbit. The Java 1.2 API spec says that conformant JVMs should have the same results for math operations as fdlibm: http://java.sun.com/products/jdk/1.2/docs/api/java/lang/Math.html
Status: NEW → ASSIGNED
- bug 81851 was caused by an uninitialized nsresult. - bug 84477 "nsCachePrefObserver::Install can return an uninitialized nsresult" - bug 84498 "xpidl_process_idl may return unitialized value" - bug 84508 "Tinderbox should compile with -O on Linux to produce -Wuninitialized warnings." - attachment 37644 [details] to bug 84508 gives a listing of _243_ uninitialized variables in an up-to-date build. This means that the number more than doubled since this bug was filed!
Depends on: 81851, 84477, 84498, 84508
Another good practice is for each engineer to turn warnings all they way up and do a build of his or her area. I've often find numerous problems in code by doing this in this past. There's plenty of noise, but often there are valid warnings that point to bugs.
bug 94135 "nsPrefBranch::SetComplexValue : rv may be returned uninitialized" bug 94141 "modules/libimg/src/if.cpp : IL_StreamWrite return value undefined if len==0."
Depends on: 94135, 94141
bug 94151 "nsBidiPresUtils::FormatUnicodeText can return uninitialized nsresult"
Depends on: 94151
Bug 53593 "uninitialized variables in jsj.c" I filed a few new NSS bugs: Bug 95982 "rsa.c: RSA_PrivateKeyOp will return uninitialized SECStatus on success" Bug 95983 "jarver.c: uninitialized variables might be being used before being set" Bug 95989 "certdb/genname.c: cert_CompareNameWithConstraints handles certURI case incorrectly" Bug 95990 "rijndael.c: encrypt/decrypt use c2 and c3 without ever initializing" Unfortunatelly, there are much more of these in NSS...
Depends on: 53593, 95982, 95983, 95989, 95990
Attached file 2001100515: 278 "may be used uninitialized" warnings (obsolete) (deleted) —
The number of these warnings seems to be finally going down (http://tinderbox.mozilla.org/SeaMonkey/warn1010614620.16740.html has 269 of them), but very slowly. It's very likely that some of those 269 are real bugs and IMHO it would make sence to try to make sure that do not make it into 1.0
Keywords: mozilla1.0
Attached file 267 "may be uninitialized" warnings from todays log (obsolete) (deleted) —
These are the "may be uninitialized" warnings listed in http://tinderbox.mozilla.org/SeaMonkey/warn1010629440.4199.html
Attachment #19017 - Attachment is obsolete: true
Attachment #19018 - Attachment is obsolete: true
Attachment #38857 - Attachment is obsolete: true
Attachment #44946 - Attachment is obsolete: true
Attachment #52326 - Attachment is obsolete: true
Depends on: 119220
Depends on: 5602, 14733, 14737
Depends on: 119376
Depends on: 119481
Depends on: 99324
Comment on attachment 64819 [details] [diff] [review] fix for warnings in cookie and wallet code sr=alecf
Attachment #64819 - Flags: superreview+
cookie and wallet fixes are now checked in. Removing myself from cc list.
Depends on: 117418
Depends on: 122904
These are the warnings from http://tinderbox.mozilla.org/SeaMonkey/warn1013026020.23864.html (Wed, 06 Feb 2002 15:07 EST). The NSS3.4 ladning got rid of a huge number of warnings, so now we only have 190 of them left!
Attachment #64250 - Attachment is obsolete: true
Depends on: 124618
Depends on: 124619
Depends on: 119330
Depends on: 124614
Depends on: 125795
Depends on: 126457
Depends on: 126460
Depends on: 126462
Depends on: 126463
Depends on: 126473
Depends on: 126475
Depends on: 126484
Depends on: 126485
Depends on: 126488
Distribution by toplevel directory: 46 layout 34 js 16 nsprpub 13 mailnews 10 security 10 extensions 10 content 8 xpfe 6 widget 5 xpinstall 4 xpcom 4 modules 2 string 2 gfx 2 db 1 uriloader 1 rdf 1 intl 1 embedding
Attachment #68209 - Attachment is obsolete: true
Depends on: 127489
Depends on: 127490
Depends on: 127592
Depends on: 127419
Depends on: 131347
Depends on: 132141
Depends on: 132145
Depends on: 132148
Depends on: 132161
Depends on: 132163
26 down, 150 to go...
Attachment #70722 - Attachment is obsolete: true
Depends on: 134556
Depends on: 136878
Depends on: 141552
Depends on: 127289
Depends on: 145029
Depends on: 145031
Depends on: 145947
Depends on: 152700
Depends on: 161417
Depends on: 155652
Depends on: 165908
Distribution of the lines with warnings, by toplevel dir: 31 layout 28 js 14 content 13 nsprpub 11 directory 4 xpcom 4 widget 4 modules 3 security 3 mailnews 3 gfx 2 tools 2 string 2 extensions 1 xpfe 1 rdf 1 profile 1 intl
Attachment #75073 - Attachment is obsolete: true
Depends on: 155539
Blocks: buildwarning
Depends on: 188124
Depends on: 188640
Depends on: 189563
Depends on: 191717
Depends on: 191712
Depends on: 202594
Depends on: 208868
Depends on: 209335
No longer depends on: 134556
Depends on: 223702
No longer depends on: 131347
No longer depends on: 188124
No longer depends on: 127489
No longer depends on: 126475
No longer depends on: 132145
No longer depends on: 155539
Depends on: 230397
No longer depends on: 202594
Depends on: 230001
No longer depends on: 230397
No longer depends on: 208868
*** Bug 132145 has been marked as a duplicate of this bug. ***
No longer depends on: 132141
*** Bug 132141 has been marked as a duplicate of this bug. ***
All the dependent bugs has been closed. Hence closing the meta bug as well.
Assignee: chofmann → atulagrwl
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Product: Core → Core Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: