Bug 117373

Summary: REGRESSION: ASSERT failures in stack walking code on FTL branch
Product: WebKit Reporter: Mark Lam <mark.lam>
Component: JavaScriptCoreAssignee: Mark Lam <mark.lam>
Status: RESOLVED FIXED    
Severity: Normal CC: fpizlo, ggaren, mhahnenberg, msaboff, oliver
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
the patch. oliver: review+

Mark Lam
Reported 2013-06-08 13:20:44 PDT
The first assertion, in CallFrame::setLocationAsBytecodeOffset(), wrongly assumes that the incoming offset argument is in units of bytes. This is not true. It is in units of Instruction*. Hence, the assertion which checks for the low 2 bits to be clear can fail. The fix is to remove the assertion.
Attachments
the patch. (1.44 KB, patch)
2013-06-08 13:23 PDT, Mark Lam
oliver: review+
Mark Lam
Comment 1 2013-06-08 13:23:42 PDT
Created attachment 204095 [details] the patch.
Oliver Hunt
Comment 2 2013-06-08 13:27:50 PDT
Comment on attachment 204095 [details] the patch. Can we have an enum type so distinguish whether an offset should be bytes or instructions? e.g. enum InstructionOffset {}; enum ByteOffset {}; ByteOffset byteOffsetFromInstruction(unsigned i) { return i * sizeof Instruction } , etc
Mark Lam
Comment 3 2013-06-08 13:31:15 PDT
Mark Lam
Comment 4 2013-06-08 13:37:10 PDT
(In reply to comment #2) > (From update of attachment 204095 [details]) > Can we have an enum type so distinguish whether an offset should be bytes or instructions? > > e.g. enum InstructionOffset {}; enum ByteOffset {}; ByteOffset byteOffsetFromInstruction(unsigned i) { return i * sizeof Instruction } , etc I'm not sure that will work. My gut feeling is that it will result in a lot of casting, which would defeat the type safety checks (which is why I think you want the enums, right?). Thanks for the review. Patch landed in r151347: <http://trac.webkit.org>.
Mark Lam
Comment 5 2013-06-08 13:40:15 PDT
Note You need to log in before you can comment on or make changes to this bug.