Closed Bug 471635 Opened 16 years ago Closed 16 years ago

JS shell print(str) is no longer traceable

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jruderman, Unassigned)

References

Details

(Keywords: regression, testcase) 

TRACEMONKEY=verbose ~/tracemonkey/js/src/debug/js -j

(function(){
  for (var i = 1; i < 20; ++i) {
    print("#");
  }
})()

abort: 6062: untraceable native
Abort recording (line 3, pc 14): call.

Print was originally made traceable in bug 461549.
Flags: blocking1.9.1?
I don't think this is a blocker, but I will take a look.
Flags: blocking1.9.1?
By itself, no, but given how it makes testing more difficult (consider using it to quickly examine an expression's value every time around a loop; storing to an array and printing all-at-once is more obfuscation and more effort to write) I think it is.
Yeah, we definitively want to fix this, but I don't think this patch must go into 1.9.1 so it should block.
I rely on print() for fuzz testing, to make sure jit and non-jit are doing the same thing.
Let's not argue about this -- it's a shell functon (unless the regresson was in jstracer.cpp) and we can and should sync js.cpp across all active trees. NPOTB (google it).

/be
Flags: wanted1.9.1?
Looking at the cause as we speak ... shouldn't take long.
Flags: wanted1.9.1? → wanted1.9.1+
Something strange is happening here. Its defined JS_TN but the flag isn't set any more at runtime??
Flags: wanted1.9.1+ → wanted1.9.1?
The first bad revision is:
changeset:   23113:55e23c647137
user:        Ted Mielczarek <ted.mielczarek@gmail.com>
date:        Wed Dec 03 08:55:27 2008 -0500
summary:     bug 462004 - JavaScript shell should provide line editing facilities. r=bsmedberg

Moving js.cpp into shell/ broke print.
Should we dup this?
No, this bug is not that one -- that one was about lack of line editing facilities since the autoconf build system landed (a regression). This is about a further regression from the editline restoration. We make the regression bug block the older bug so no one takes the patch for the older bug without seeing a dependency from it to follow, to take the blocking bug's patch too.

/be
Blocks: 462004
Ok. TM tip is back to tracing print now. sayrer merged with m-c which has the offending patch backed out for an independent bustage.
I don't really know much about the internals of the JS shell, can anyone clue me in as to why this would have broken? Did I miss a CFLAGS or DEFINES setting in the new Makefile that would have caused this bustage?
Oh, actually, I think this was just a bad merge. My patch does "hg mv js.cpp shell/", and it was done before that patch was committed, so in the merge those changes probably just got lost. When I fix the other bustage, I'll rebase to pick up those changes before the move. You'll still have to be wary of any other js.cpp changes in the meantime. I don't know what a merge of that nature looks like.
Maybe I'm looking in the wrong place, but I don't think it's a bad merge.  The revision http://hg.mozilla.org/mozilla-central/rev/55e23c647137 does contain the relevant lines, like this one:

> +    JS_TN("print",          Print,          0,0, Print_trcinfo),

It must be a DEFINES bug.  If JS_TRACER is not defined, macros like that one silently fall back on a non-jit implementation.
Ah, ok. Clearly we need to get all the necessary DEFINES out of the Makefiles and into configure.
bsmedberg's patch in bug 463172 does just that, so we'll get that in first.
Depends on: 463172
I think this is fixed, but Jesse's testcase crashes for me on Windows now, so I filed bug 473435 on that. I'll have to verify on another platform.
Ok, yes, fixed by the combination of my updated patch in bug 462004, and the patch in bug 463172.
I tested this with my patches applied, and it works.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
http://hg.mozilla.org/mozilla-central/rev/8a03e503326c
/cvsroot/mozilla/js/tests/js1_8_1/trace/regress-471635.js,v  <--  regress-471635.js
initial revision: 1.1
Flags: in-testsuite+
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.