Closed Bug 548583 Opened 15 years ago Closed 14 years ago

JIT can cause VerifyError when attempting to early bind to a super property

Categories

(Tamarin Graveyard :: Baseline JIT (CodegenLIR), defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
Q3 11 - Serrano

People

(Reporter: edwsmith, Assigned: edwsmith)

References

Details

Attachments

(1 file)

Throws runtime error with interp, VerifyError with JIT: package { class A { function f() { print("f") } } class B extends A { function g() { super.f(1) } } new B().g() } The JIT should not early bind if the argument count doesn't match. (need to check in OP_callsuper and OP_callsupervoid). This is also a problem with OP_getsuper and OP_setsuper, if the getter and setter functions don't have 0 and 1 argument, respectively. There are no verify checks for that -- the runtime call should fail.
any all from the jit to verifier->readBinding() can also caused indirect verify errors.
Assignee: nobody → edwsmith
Flags: flashplayer-qrb+
Priority: -- → P2
Target Milestone: --- → flash10.1
(In reply to comment #1) > any all from the jit to verifier->readBinding() can also caused indirect verify > errors. Probably not true, actually. Logic: - if we're verifying method M, M is resolved - if M is resolved, M->declaringTraits is - if M->declaringTraits is resolved, so are all base classes ergo, cannot get a verify error in resolveTraits at this point If this is true, we should factor the code so it's plainly obvious instead of calling a generic routine (read that *could* through even though it will not. The original bug description is still valid as the test case shows. The fix is to not early bind in the jit, and not throw a verify error.
Target Milestone: flash10.1 → flash10.2
Component: Virtual Machine → JIT Compiler (NanoJIT)
Blocks: 538181
There is small a risk here of suppressing a verify error where we once had one. However, with asc -strict, the problem is detected at compile time. No bugs have been reported on this verify error, where many other verify error bugs have been reported. A brightspot scan found no occurances of this verifier error as well. The patch fixes it by simply falling back to the late bound case, which aligns behavior with the interpreters. It is wrong for the JIT to report a verify error; either the verifier must do it regardless of execution mode, or the JIT must gracefully degrade.
Attachment #454919 - Flags: review?(lhansen)
Any issue with constructsuper?
Attachment #454919 - Flags: review?(lhansen) → review+
OP_constructsuper uses the verifier helper emitCoerceArgs(), which throws a verify error if the arg count is not ok. (regardless of execution mode). in this case its the verifier reporting failure, which is okay. The other opcodes that use emitCoerceArgs() and thus fail with bad arg counts are: * OP_getproperty/getlex, invoking a getter. (implicit argc == 0, won't fail). * OP_set/initproperty, when invoking a setter. (argc is implicit, so this can't fail). * OP_callstatic * OP_callproperty/callproplex/callpropvoid. One could argue that constructsuper and callproperty should not trigger a verifyError either, if there is an argc mismatch after early binding (instead, just don't early bind). But, since the failure is independent of execution mode, its not my current problem (scope creep). another argument would be that we should *tighten* OP_call/get/setsuper, to fail independently of execution mode, like callproperty and constructsuper do. But, adding new verify failure modes is bad mojo (too risky). Thoughts?
Status: NEW → RESOLVED
Closed: 14 years ago
Flags: in-testsuite?
Resolution: --- → FIXED
Flags: flashplayer-bug+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: