The default bug view has changed. See this FAQ.

Assertion failure: map->vector && i < map->length, at jsatom.c:919

VERIFIED FIXED

Status

()

Core
JavaScript Engine
--
critical
VERIFIED FIXED
10 years ago
10 years ago

People

(Reporter: Waldo, Assigned: Igor Bukanov)

Tracking

({verified1.8.1.2})

Trunk
x86
Linux
verified1.8.1.2
Points:
---
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 2 obsolete attachments)

js> dis(function() {
  XML.prototype.function::toString = function() { return "foo"; };
})
flags: LAMBDA INTERPRETED
main:
00000:  name "XML"
00003:  getprop "prototype"
00006:  anonfunobj (function () {return "foo";})
00009:  setmethod "toString"
00012:  pop
00013:  stop

Source notes:
  0:     0 [   0] newline
  1:     3 [   3] pcbase   offset 3
Assertion failure: map->vector && i < map->length, at jsatom.c:919
(Assignee)

Comment 1

10 years ago
Most likely this is a regression from bug 331966.
Assignee: general → igor.bukanov
(Assignee)

Comment 2

10 years ago
(In reply to comment #1)
> Most likely this is a regression from bug 331966.

It is not, but still I will look into it.
(In reply to comment #2)
> It is not, but still I will look into it.

Bug 355258 comment 5 may be of interest here, then.
(Assignee)

Comment 4

10 years ago
Created attachment 249185 [details] [diff] [review]
Fix for jsshell

The patch updates the source note printout code in jsshell to distinguish LABEL from METHODBASE based on the bytecode the code points to.
Attachment #249185 - Flags: review?(brendan)
Comment on attachment 249185 [details] [diff] [review]
Fix for jsshell

>+/*
>+ * SrcNotes assumes that SRC_METHODBASE should be distinguished from SRC_LABEL
>+ * using the bytecode the source note points to.
>+ */
>+JS_STATIC_ASSERT(SRC_LABEL == SRC_METHODBASE);
>+

In the rush for Firefox 2 I failed to catch this.  I had a plan to extend js_SrcNoteSpec with lists of bytecodes (initialized static arrays, or perhaps C strings) for which the aliases were valid.  Comments?

> static void
> SrcNotes(JSContext *cx, JSScript *script)
> {
>+    const char NAME_PATTERN[] = "%-8s";

I'm superstitious about old C compilers, so if you could make this static, move it down to bottom of declarations, and use FORMAT or FMT instead of PATTERN, I'd be grateful.

>     uintN offset, delta, caseOff;
>     jssrcnote *notes, *sn;
>     JSSrcNoteType type;
>+    uintN op;

Why not JSOp?  Helps in gdb.

>+        if (type == SRC_LABEL) {
>+            op = script->code[offset];
>+            if (op == JSOP_GETMETHOD || op == JSOP_SETMETHOD) {
>+                /* This is SRC_METHODBASE. */
>+                fprintf(gOutFile, NAME_PATTERN, "methodbase");
>+                goto print_offset;
>+            }
>+            JS_ASSERT(op == JSOP_NOP ||
>+                      op == JSOP_TABLESWITCH ||
>+                      op == JSOP_TABLESWITCHX ||
>+                      op == JSOP_LOOKUPSWITCH ||
>+                      op == JSOP_LOOKUPSWITCHX);

The SRC_LABEL notes emitted for the switch opcodes do not apply to JSOP_*SWITCH*, rather to case expressions that have had constants propagated to replace names:

js> eval("const A=1, B=2, C=3; function f(x){switch(x){case A:return 1;case B:return 2;case C:return 3}}")
js> f
function f(x) {
    switch (x) {
      case A:
        return 1;
      case B:
        return 2;
      case C:
        return 3;
      default:;
    }
}
js> dis(f)
main:
00000:  getarg 0
00003:  tableswitch defaultOffset 23 low 1 high 3
        1: 13
        2: 15
        3: 19
00016:  one
00017:  return
00018:  uint16 2
00021:  return
00022:  uint16 3
00025:  return
00026:  stop

Source notes:
  0:     3 [   3] switch   length 23
  3:     9 [   6] label    atom 0 (A)
  5:    11 [   2] label    atom 1 (B)
  7:    13 [   2] label    atom 2 (C)

In this example, which requires eval in order for the optimization to be considered safe, the A SRC_LABEL applies to the constant 1 whose two-byte jump offset is at bytecode 10; to address this using the standard decoding, subtract -1 to get 9. Etc. for 2/12 and 3/14.  See the JSOP_TABLESWITCH* and JSOP_LOOKUPSWITCH* cases in Decompile, note the pc2 variable passed to js_GetSrcNote.

So the upshot is that you can't load script->code[offset] and test for a known bytecode -- you need to remember that you're in a switch of the right kind.  Yuck.

/be
(Assignee)

Comment 6

10 years ago
Created attachment 249240 [details] [diff] [review]
Fix for jsshell v2

This version detects the switch and reads script->code[offset] only when the label note is outside the switch tables.

I also changed the code to print "case", not "label" for the switch case notes.
Attachment #249185 - Attachment is obsolete: true
Attachment #249240 - Flags: review?(brendan)
Attachment #249185 - Flags: review?(brendan)
(Assignee)

Comment 7

10 years ago
Created attachment 249258 [details] [diff] [review]
Fix for jsshell v3

In this version I removed +1/-1 game in GetSwitchTableBounds which was a leftover from the inirial prototype.
Attachment #249240 - Attachment is obsolete: true
Attachment #249258 - Flags: review?(brendan)
Attachment #249240 - Flags: review?(brendan)
(Assignee)

Comment 8

10 years ago
(In reply to comment #5)
> I had a plan to extend js_SrcNoteSpec with lists of bytecodes (initialized static arrays, or perhaps C strings) for which the aliases were valid.  Comments?

But why aliases are neccessery? The total number of all source notes is 28 AFAICS which is less then 1<<5. 
 

(In reply to comment #8)
> But why aliases are neccessery? The total number of all source notes is 28
> AFAICS which is less then 1<<5. 

We would like more than 28 source note *names*, in order to make decompiler code intelligible given the new JS1.7 features.  But as you note we do not need more than 28 distinct codes.  So the trick is to decompile the appropriate name in context of the bytecode it annotates.

/be
Comment on attachment 249258 [details] [diff] [review]
Fix for jsshell v3

Looks great.

I should take a followup to make getting the name part of the jsemit.c machinery, instead of a shell helper.

/be
Attachment #249258 - Flags: review?(brendan) → review+
(Assignee)

Comment 11

10 years ago
I committed the patch from comment 7 to the trunk:

Checking in js.c;
/cvsroot/mozilla/js/src/js.c,v  <--  js.c
new revision: 3.128; previous revision: 3.127
done
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
(Assignee)

Comment 12

10 years ago
Comment on attachment 249258 [details] [diff] [review]
Fix for jsshell v3

Is the approval is necessary for 1.8.1 branch given that the file is not a part of browser's build?
Attachment #249258 - Flags: approval1.8.1.2?
(In reply to comment #12)
> (From update of attachment 249258 [details] [diff] [review] [edit])
> Is the approval is necessary for 1.8.1 branch given that the file is not a part
> of browser's build?

Not in my book.

/be
(Assignee)

Comment 14

10 years ago
Comment on attachment 249258 [details] [diff] [review]
Fix for jsshell v3

Given the previous comment the approval is not necessary for 1.8.1
Attachment #249258 - Flags: approval1.8.1.2?
(Assignee)

Comment 15

10 years ago
I committed the patch from comment 7 to MOZILLA_1_8_BRANCH: 

Checking in js.c;
/cvsroot/mozilla/js/src/js.c,v  <--  js.c
new revision: 3.93.2.14; previous revision: 3.93.2.13
done
Keywords: fixed1.8.1.2

Comment 16

10 years ago
/cvsroot/mozilla/js/tests/e4x/Regress/regress-364017.js,v  <--  regress-364017.js
initial revision: 1.1
Flags: in-testsuite+

Comment 17

10 years ago
verified fixed 1.8.1, 1.9.0 2007-01-23 win/mac*/linux
Status: RESOLVED → VERIFIED
Keywords: fixed1.8.1.2 → verified1.8.1.2
You need to log in before you can comment on or make changes to this bug.