Bug 482743: Fix up bytecode execution tracing. Allow tracing to file. r=igor

js_TraceOpcode: Remember the last bytecode we traced explicitly,
instead of subtracting 'len' from regs.pc, which isn't reliable.
Decline to trace values in script prologues (between 'code' and
'main').  Decline to walk off the bottom of the stack when the 'last
bytecode' is misleading.  Flush the stream after each bytecode.

Use the TRACE_OPCODE macro in both threaded and non-threaded
interpreters.  Take care to make threaded and non-threaded
interpreters produce the same traces.

In the shell's 'tracing' function, use JS_ValueToBoolean to recognize
all sorts of booleans, and treat a string as the name of a file to
write the trace to.

js/src/jscntxt.cpp

@@ -1557,7 +1557,7 @@ js_ReportValueErrorFlags(JSContext *cx, uintN flags, const uintN errorNumber,
 
 #if defined DEBUG && defined XP_UNIX
 /* For gdb usage. */
-void js_traceon(JSContext *cx)  { cx->tracefp = stderr; }
+void js_traceon(JSContext *cx)  { cx->tracefp = stderr; cx->tracePrevOp = JSOP_LIMIT; }
 void js_traceoff(JSContext *cx) { cx->tracefp = NULL; }
 #endif
 

js/src/jscntxt.h

@@ -922,6 +922,7 @@ struct JSContext {
     char                *lastMessage;
 #ifdef DEBUG
     void                *tracefp;
+    JSOp                tracePrevOp;
 #endif
 
     /* Per-context optional error reporter. */

js/src/jsinterp.cpp

@@ -2045,12 +2045,11 @@ js_DoIncDec(JSContext *cx, const JSCodeSpec *cs, jsval *vp, jsval *vp2)
 #ifdef DEBUG
 
 JS_STATIC_INTERPRET JS_REQUIRES_STACK void
-js_TraceOpcode(JSContext *cx, jsint len)
+js_TraceOpcode(JSContext *cx)
 {
     FILE *tracefp;
     JSStackFrame *fp;
     JSFrameRegs *regs;
-    JSOp prevop;
     intN ndefs, n, nuses;
     jsval *siter;
     JSString *str;
@@ -2060,10 +2059,22 @@ js_TraceOpcode(JSContext *cx, jsint len)
     JS_ASSERT(tracefp);
     fp = cx->fp;
     regs = fp->regs;
-    if (len != 0) {
+
-        prevop = (JSOp) regs->pc[-len];
+    /*
-        ndefs = js_CodeSpec[prevop].ndefs;
+     * Operations in prologues don't produce interesting values, and
-        if (ndefs != 0) {
+     * js_DecompileValueGenerator isn't set up to handle them anyway.
+     */
+    if (cx->tracePrevOp != JSOP_LIMIT && regs->pc >= fp->script->main) {
+        ndefs = js_GetStackDefs(cx, &js_CodeSpec[cx->tracePrevOp],
+                                cx->tracePrevOp, fp->script, regs->pc);
+
+        /*
+         * If there aren't that many elements on the stack, then 
+         * we have probably entered a new frame, and printing output
+         * would just be misleading.
+         */
+        if (ndefs != 0 &&
+            ndefs < regs->sp - fp->slots) {
             for (n = -ndefs; n < 0; n++) {
                 char *bytes = js_DecompileValueGenerator(cx, n, regs->sp[n],
                                                          NULL);
@@ -2108,6 +2119,10 @@ js_TraceOpcode(JSContext *cx, jsint len)
         }
         fprintf(tracefp, " @ %u\n", (uintN) (regs->sp - StackBase(fp)));
     }
+    cx->tracePrevOp = op;
+
+    /* It's nice to have complete traces when debugging a crash.  */
+    fflush(tracefp);
 }
 
 #endif /* DEBUG */
@@ -2534,6 +2549,30 @@ js_Interpret(JSContext *cx)
 # define JS_EXTENSION_(s) s
 #endif
 
+# ifdef DEBUG
+    /*
+     * We call this macro from BEGIN_CASE in threaded interpreters,
+     * and before entering the switch in non-threaded interpreters.
+     * However, reaching such points doesn't mean we've actually
+     * fetched an OP from the instruction stream: some opcodes use
+     * 'op=x; DO_OP()' to let another opcode's implementation finish
+     * their work, and many opcodes share entry points with a run of
+     * consecutive BEGIN_CASEs.
+     * 
+     * Take care to trace OP only when it is the opcode fetched from
+     * the instruction stream, so the trace matches what one would
+     * expect from looking at the code.  (We do omit POPs after SETs;
+     * unfortunate, but not worth fixing.)
+     */
+#  define TRACE_OPCODE(OP)  JS_BEGIN_MACRO                                    \
+                                if (JS_UNLIKELY(cx->tracefp != NULL) &&       \
+                                    (OP) == *regs.pc)                         \
+                                    js_TraceOpcode(cx);                       \
+                            JS_END_MACRO
+# else
+#  define TRACE_OPCODE(OP)  ((void) 0)
+# endif
+
 #if JS_THREADED_INTERP
     static void *const normalJumpTable[] = {
 # define OPDEF(op,val,name,token,length,nuses,ndefs,prec,format) \
@@ -2572,15 +2611,6 @@ js_Interpret(JSContext *cx)
                                 DO_OP();                                      \
                             JS_END_MACRO
 
-# ifdef DEBUG
-#  define TRACE_OPCODE(OP)  JS_BEGIN_MACRO                                    \
-                                if (cx->tracefp)                              \
-                                    js_TraceOpcode(cx, len);                  \
-                            JS_END_MACRO
-# else
-#  define TRACE_OPCODE(OP)  (void)0
-# endif
-
 # define BEGIN_CASE(OP)     L_##OP: TRACE_OPCODE(OP); CHECK_RECORDER();
 # define END_CASE(OP)       DO_NEXT_OP(OP##_LENGTH);
 # define END_VARLEN_CASE    DO_NEXT_OP(len);
@@ -2846,13 +2876,10 @@ js_Interpret(JSContext *cx)
       advance_pc:
         regs.pc += len;
         op = (JSOp) *regs.pc;
-# ifdef DEBUG
-        if (cx->tracefp)
-            js_TraceOpcode(cx, len);
-# endif
 
       do_op:
         CHECK_RECORDER();
+        TRACE_OPCODE(op);
         switchOp = intN(op) | switchMask;
       do_switch:
         switch (switchOp) {

js/src/jsinterp.h

@@ -611,7 +611,7 @@ js_DoIncDec(JSContext *cx, const JSCodeSpec *cs, jsval *vp, jsval *vp2);
  * previous opcode.
  */
 extern JS_REQUIRES_STACK void
-js_TraceOpcode(JSContext *cx, jsint len);
+js_TraceOpcode(JSContext *cx);
 
 /*
  * JS_OPMETER helper functions.

js/src/shell/js.cpp

@@ -1859,13 +1859,8 @@ DisassWithSrc(JSContext *cx, JSObject *obj, uintN argc, jsval *argv,
 static JSBool
 Tracing(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)
 {
-    JSBool bval;
+    FILE *file;
-    JSString *str;
 
-#if JS_THREADED_INTERP
-    JS_ReportError(cx, "tracing not supported in JS_THREADED_INTERP builds");
-    return JS_FALSE;
-#else
     if (argc == 0) {
         *rval = BOOLEAN_TO_JSVAL(cx->tracefp != 0);
         return JS_TRUE;
@@ -1873,25 +1868,40 @@ Tracing(JSContext *cx, JSObject *obj, uintN argc, jsval *argv, jsval *rval)
 
     switch (JS_TypeOfValue(cx, argv[0])) {
       case JSTYPE_NUMBER:
-        bval = JSVAL_IS_INT(argv[0])
+      case JSTYPE_BOOLEAN: {
-               ? JSVAL_TO_INT(argv[0])
+        JSBool bval;
-               : (jsint) *JSVAL_TO_DOUBLE(argv[0]);
+        if (!JS_ValueToBoolean(cx, argv[0], &bval))
+            goto bad_argument;
+        file = bval ? stderr : NULL;
         break;
-      case JSTYPE_BOOLEAN:
+      }
-        bval = JSVAL_TO_BOOLEAN(argv[0]);
+      case JSTYPE_STRING: {
+        char *name = JS_GetStringBytes(JSVAL_TO_STRING(argv[0]));
+        file = fopen(name, "w");
+        if (!file) {
+            JS_ReportError(cx, "tracing: couldn't open output file %s: %s", 
+                           name, strerror(errno));
+            return JS_FALSE;
+        }
         break;
+      }
       default:
-        str = JS_ValueToString(cx, argv[0]);
+          goto bad_argument;
+    }
+    if (cx->tracefp && cx->tracefp != stderr)
+      fclose((FILE *)cx->tracefp);
+    cx->tracefp = file;
+    cx->tracePrevOp = JSOP_LIMIT;
+    return JS_TRUE;
+
+ bad_argument:
+    JSString *str = JS_ValueToString(cx, argv[0]);
     if (!str)
         return JS_FALSE;
     JS_ReportError(cx, "tracing: illegal argument %s",
                    JS_GetStringBytes(str));
     return JS_FALSE;
 }
-    cx->tracefp = bval ? stderr : NULL;
-    return JS_TRUE;
-#endif
-}
 
 static void
 DumpScope(JSContext *cx, JSObject *obj, FILE *fp)
@@ -3676,7 +3686,8 @@ static const char *const shell_help_messages[] = {
 "dumpHeap([fileName[, start[, toFind[, maxDepth[, toIgnore]]]]])\n"
 "  Interface to JS_DumpHeap with output sent to file",
 "notes([fun])             Show source notes for functions",
-"tracing([toggle])        Turn tracing on or off",
+"tracing([true|false|filename]) Turn bytecode execution tracing on/off.\n"
+"                         With filename, send to file.\n",
 "stats([string ...])      Dump 'arena', 'atom', 'global' stats",
 #endif
 #ifdef TEST_CVTARGS