Closed Bug 107138 Opened 23 years ago Closed 23 years ago

Strange javascript behavior with literal value of "1"

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Windows NT
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla0.9.6

People

(Reporter: morse, Assigned: brendan)

Details

(Keywords: js1.5, Whiteboard: [QA note: verify interactively; see below])

Attachments

(2 files, 1 obsolete file)

real fix (last patch papers over thread safety bug)
1.91 KB, patch
brendan
: review+
jband_mozilla
: superreview+
Details | Diff | Splinter Review
diff -wu version of last patch
1.41 KB, patch
Details | Diff | Splinter Review 
Consider the following content:

   <html>
     <head>
       <script>
         var value = ["zero", "one", "two"];
         var index = "0,1,2".split(",");

         dump("index[0]="+index[0]+"\n");
         dump("index[1]="+index[1]+"\n");
         dump("index[2]="+index[2]+"\n");
         dump("\n");

         dump("value[0]="+value[0]+"\n");
         dump("value[index[0]]="+value[index[0]]+"\n");
         dump("\n");

         dump("value[1]="+value[1]+"\n");
         dump("value[index[1]]="+value[index[1]]+"\n");
         dump("\n");

         dump("value[2]="+value[2]+"\n");
         dump("value[index[2]]="+value[index[2]]+"\n");

   //    var dummy = "1";

       </script>
     <body>
        Note that value[index[1]] comes out to be undefined instead of "one"<br>
        Commenting out the last line makes it work correctly
     </body>
   </html>

The expected dumps on the console should be

   index[0]=0
   index[1]=1
   index[2]=2

   value[0]=zero
   value[index[0]]=zero

   value[1]=one
   value[index[1]]=one         // this one is coming up as undefined

   value[2]=two
   value[index[2]]=two

The third line from the bottom is coming out as undefined instead of one.  All 
other lines are coming up as expected.

Uncommenting out the assignment of "1" fixes the problem.
If the assignment were anything other than "1", it does not fix the problem.

I must admit that this is one of the strangest bug reports that I've ever filed, 
and I still can't believe the results!
This bug was causing the problem in bug 106914
This bug is also responsible for the problem in bug 106866.

Since both 106866 and 106914 just started occuring (I know that those dialogs 
used to work), I am assuming that this js bug is a recent regression.
Of course adding a parseInt also solves the problem.  That is the 
work-around that I attached to bug 106866 and bug 106914.

I think I'd rather see this bug fixed than those work-arounds being checked in.
This bug is pretty serious because it breaks at least two dialogs that we know 
of and possibly others that we haven't discovered yet.

Is this assigned to the right component?  Are there others that should be cc'd 
on it?
Severity: normal → critical
Adding nice informative alert() at the head of the test code that says it all.  
In version 4.x the alert displays "zero one two three" whereas in mozilla it 
displays "zero undefined two three".

<html>
  <head>
    <script>
      var value = ["zero", "one", "two", "three"];
      var index = "0,1,2,3".split(",");

      alert(value[index[0]]+" "+value[index[1]]+" "+
            value[index[2]]+" "+value[index[3]]);

      dump("...index[0]="+index[0]+"\n");
      dump("...index[1]="+index[1]+"\n");
      dump("...index[2]="+index[2]+"\n");
      dump("...index[3]="+index[3]+"\n");
      dump("...\n");

      dump("...value[0]="+value[0]+"\n");
      dump("...value[index[0]]="+value[index[0]]+"\n");
      dump("...\n");

      dump("...value[1]="+value[1]+"\n");
      dump("...value[index[1]]="+value[index[1]]+"\n");
      dump("...\n");

      dump("...value[2]="+value[2]+"\n");
      dump("...value[index[2]]="+value[index[2]]+"\n");
      dump("...\n");

      dump("...value[3]="+value[3]+"\n");
      dump("...value[index[3]]="+value[index[3]]+"\n");
      dump("...\n");

//    var dummy = "1";

    </script>
  <body>
     Note that value[index[1]] comes out to be undefined instead of "one"<br>
     Commenting out the last line makes it work correctly
  </body>
</html>

And to make it even simpler we can get rid of all the dumps and just focus on 
the alert:

<html>
  <head>
    <script>
      var value = ["zero", "one", "two", "three"];
      var index = "0,1,2,3".split(",");
      alert(value[index[0]]+" "+value[index[1]]+" "+
            value[index[2]]+" "+value[index[3]]);
    </script>
  <body>
     alert should display "zero one two three"
     but instead it is displaying "zero undefined two three"
  </body>
</html>

This bug is also responsible for bug 107351.

In summary, three dialogs are now broken because of this bug:
   bug 107351: cookie manager
   bug 106914: image manager
   but 106866: password manager

cc'ing brendan, jband, and shaver.  Do any of you have any idea what is causing 
this problem?  And is it assigned to the right component/person?
The code in CHECK_FOR_FUNNY_INDEX calls ATOM_TO_STRING which is returning a 
dependent string, but proceeds to use the chars field, which is bogus. I don't 
know why "0" & "1" are dependent but not "2" and "3" - (note for me, running 
under the shell I get undefined for 0 & 1, probably there's a random memory 
reference). 
switching to use JSSTRING_CHARS & JSSTRING_LENGTH fixes things. I guess we just 
missed this case - but i'll leave it to Brendan to say.
In the JS shell, with the testcase reduced to:

var value = ["zero", "one", "two", "three"];

print(value[0]);
print(value["0"]);

print(value[1]);
print(value["1"]);

print(value[2]);
print(value["2"]);

print(value[3]);
print(value["3"]);


I often got this output, showing the bug:

zero
zero

one
undefined

two
two

three
three


But sometimes the bug did not appear - 
Testcase added to JS testsuite:

         mozilla/js/tests/js1_5/Array/regress-107138.js

Very frustrating - it never fails in the current shell! Must be 
the print() and alert()functions do something to expose the bug.
The testcase must capture the array values for comparison:

                    actual = value["1"];
                    expect = "one";

This is not failing for me - ever!
Whiteboard: [QA note: verify interactively; see below]
IIRC, brendan didn't overlook this case: he explicitly changed his patch because
CHECK_FOR_FUNNY_INDEX couldn't be called on a dependent string.  Are we exposing
a different bug here?

Argh.  Fix coming right up.

/be
Assignee: rogerl → brendan
Keywords: js1.5, mozilla0.9.6
Target Milestone: --- → mozilla0.9.6
Please review ASAP.  Thanks,

/be
Status: NEW → ASSIGNED
Attachment #55638 - Attachment is obsolete: true
Comment on attachment 55676 [details] [diff] [review]
real fix (last patch papers over thread safety bug)

r/sr=jband
Attachment #55676 - Flags: superreview+
Comment on attachment 55676 [details] [diff] [review]
real fix (last patch papers over thread safety bug)

r=jag, he says (over my shoulder; I've babbled to him about the 56940 patch for weeks, so he's hip to the technique).

/be
Attachment #55676 - Flags: review+
Fix is in, sorry about that.

/be
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Verified interactively in debug/optimized JS shells built
today, 2001-10-30, on WinNT, Linux, and Mac 9.1
                                                                              
morse@netscape.com: thank you for catching this.
Please reopen if your results don't match mine -
Status: RESOLVED → VERIFIED
Flags: testcase+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: