[Webkit-unassigned] [Bug 138543] Assertions in JSC::StackVisitor::Frame::existingArguments() during stack unwinding

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=138543

--- Comment #8 from Filip Pizlo <fpizlo at apple.com> ---
(In reply to comment #7)
> Comment on attachment 241252 [details]
> Proposed patch.
> 
> It feels a bit awkward to program defensively like this just to make the
> fuzzer happy. Programming like this means that we can't tell the difference
> between "Something is seriously wrong because the activation object is
> missing" and "I'm just fuzzing".
> 
> Ideally, we would teach the fuzzer not to throw in cases that otherwise
> couldn't -- for example, in the LLInt, by passing an argument to END() that
> said "ASSERT there is no exception, and do not fuzz for exceptions".
> 
> I guess this patch is an improvement for now, so it's worth doing. Note,
> though that you missed a spot: Oliver removed the original work-around for
> fuzzing, probably because he wasn't aware of this special fuzzing behavior.
> 
> You should update your comments to specify that we do this only for fuzzing,
> and also add back the code that Oliver removed in
> <http://trac.webkit.org/changeset/174226>:
> 
> Index: /trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp
> ===================================================================
> --- /trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp	(revision
> 174225)
> +++ /trunk/Source/JavaScriptCore/interpreter/Interpreter.cpp	(revision
> 174226)
> @@ -440,6 +440,4 @@
>      CallFrame* callFrame = visitor->callFrame();
>      CodeBlock* codeBlock = visitor->codeBlock();
> -    JSScope* scope = callFrame->scope();
> -
>      if (Debugger* debugger = callFrame->vmEntryGlobalObject()->debugger()) {
>          ClearExceptionScope scope(&callFrame->vm());
> @@ -456,13 +454,4 @@
>          RELEASE_ASSERT(!visitor->isInlinedFrame());
>  #endif
> -        lexicalEnvironment = callFrame->uncheckedActivation();
> -        // Protect against the lexical environment not being created, or
> the variable still being
> -        // initialized to Undefined inside op_enter.
> -        if (lexicalEnvironment && lexicalEnvironment.isCell()) {
> -            JSLexicalEnvironment* activationObject =
> jsCast<JSLexicalEnvironment*>(lexicalEnvironment);
> -            // Protect against throwing exceptions after tear-off.
> -            if (!activationObject->isTornOff())
> -                activationObject->tearOff(*scope->vm());
> -        }
>      }

For what it's worth, when we added the fuzzer we added a handful of such checks that are only to defend against the fuzzer. This involved far less infrastructure than making the fuzzer more complicated. 

Also, if we ever were wrong about our assumptions about when activations are allocated or who could throw exceptions, this defensiveness would turn a crash into something much less severe.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20141111/f2d364fc/attachment-0002.html>