AuroraMiddleware/v8
1
0
Fork 0
Code Issues 2 Pull Requests Releases Wiki Activity

Revert "[debug] CHECK that a function's context is always available"

Browse Source 
This reverts commit 911c7170dc.

Reason for revert: Reverting because of the revert in https://crrev.com/c/3867311

Original change's description:
> [debug] CHECK that a function's context is always available
>
> After https://crrev.com/c/3854501 has landed, we no longer have to
> handle the case that we do not find a function's context in the
> scope iterator even though the function requires one.
>
> This CL renames `NeedsAndHasContext` to `NeedsContext` since we
> always find a scope's context now. Additionally we turn this
> assumption into a dedicated check.
>
> R=​bmeurer@chromium.org
>
> Bug: chromium:1246907
> Change-Id: I6458df76689c0bfa6d6b2f8c421f9ce481855547
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3865153
> Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
> Commit-Queue: Simon Zünd <szuend@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#82848}

Bug: chromium:1246907
Change-Id: I1c8849ce60533f5c6da99f432bf1902ade47bb8b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3866174
Auto-Submit: Leszek Swirski <leszeks@chromium.org>
Commit-Queue: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Leszek Swirski <leszeks@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82858}
This commit is contained in:
Leszek Swirski 2022-08-31 10:34:10 +00:00 committed by V8 LUCI CQ
parent 7a61dad0db
commit 8293459504
2 changed files with 28 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
src/debug/debug-scopes.cc
View File

@ -376,27 +376,21 @@ bool ScopeIterator::DeclaresLocals(Mode mode) const {
}
bool ScopeIterator::HasContext() const {
return !InInnerScope() || NeedsContext();
return !InInnerScope() || NeedsAndHasContext();
}
bool ScopeIterator::NeedsContext() const {
const bool needs_context = current_scope_->NeedsContext();
// We try very hard to ensure that a function's context is already
// available when we pause right at the beginning of that function.
// This can be tricky when we pause via stack check or via
// `BreakOnNextFunctionCall`, which happens normally in the middle of frame
// construction and we have to "step into" the function first.
//
// We check this by ensuring that the current context is not the closure
// context should the function need one. In that case the function has already
// pushed the context and we are good.
CHECK_IMPLIES(needs_context && current_scope_ == closure_scope_ &&
current_scope_->is_function_scope() &&
!function_->is_null(),
function_->context() != *context_);
return needs_context;
bool ScopeIterator::NeedsAndHasContext() const {
if (!current_scope_->NeedsContext()) return false;
// Generally, if a scope needs a context, then we can assume that it has a
// context. However, the stack check during function entry happens before the
// function has a chance to create and push its own context, so we must check
// for the case where the function is executing in its parent context. This
// case is only possible in function scopes; top-level code (modules and
// non-module scripts) begin execution in the context they need and don't have
// a separate step to push the correct context.
return !(current_scope_ == closure_scope_ &&
current_scope_->is_function_scope() && !function_.is_null() &&
function_->context() == *context_);
}
bool ScopeIterator::AdvanceOneScope() {
@ -421,7 +415,7 @@ void ScopeIterator::AdvanceScope() {
DCHECK(InInnerScope());
do {
if (NeedsContext()) {
if (NeedsAndHasContext()) {
// current_scope_ needs a context so moving one scope up requires us to
// also move up one context.
AdvanceOneContext();
@ -438,7 +432,7 @@ void ScopeIterator::AdvanceContext() {
// scope, but until we hit the next scope that actually requires
// a context. All the locals collected along the way build the
// blocklist for debug-evaluate for this context.
while (AdvanceOneScope() && !NeedsContext()) {
while (AdvanceOneScope() && !NeedsAndHasContext()) {
}
}
@ -476,7 +470,7 @@ void ScopeIterator::Next() {
// and collect the blocklist along the way until we find the scope
// that should match `context_`.
// But only do this if we have complete scope information.
while (!NeedsContext() && AdvanceOneScope()) {
while (!NeedsAndHasContext() && AdvanceOneScope()) {
}
}
}
@ -492,28 +486,29 @@ ScopeIterator::ScopeType ScopeIterator::Type() const {
if (InInnerScope()) {
switch (current_scope_->scope_type()) {
case FUNCTION_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsFunctionContext() ||
context_->IsDebugEvaluateContext());
DCHECK_IMPLIES(NeedsAndHasContext(),
context_->IsFunctionContext() ||
context_->IsDebugEvaluateContext());
return ScopeTypeLocal;
case MODULE_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsModuleContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsModuleContext());
return ScopeTypeModule;
case SCRIPT_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsScriptContext() ||
context_->IsNativeContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsScriptContext() ||
context_->IsNativeContext());
return ScopeTypeScript;
case WITH_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsWithContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsWithContext());
return ScopeTypeWith;
case CATCH_SCOPE:
DCHECK(context_->IsCatchContext());
return ScopeTypeCatch;
case BLOCK_SCOPE:
case CLASS_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsBlockContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsBlockContext());
return ScopeTypeBlock;
case EVAL_SCOPE:
DCHECK_IMPLIES(NeedsContext(), context_->IsEvalContext());
DCHECK_IMPLIES(NeedsAndHasContext(), context_->IsEvalContext());
return ScopeTypeEval;
}
UNREACHABLE();
@ -647,7 +642,7 @@ bool ScopeIterator::SetVariableValue(Handle<String> name,
DCHECK_EQ(ScopeTypeLocal, Type());
if (SetLocalVariableValue(name, value)) return true;
// There may not be an associated context since we're InInnerScope().
if (!NeedsContext()) return false;
if (!NeedsAndHasContext()) return false;
} else {
DCHECK_EQ(ScopeTypeClosure, Type());
if (SetContextVariableValue(name, value)) return true;
@ -693,7 +688,7 @@ void ScopeIterator::DebugPrint() {
case ScopeIterator::ScopeTypeLocal: {
os << "Local:\n";
if (NeedsContext()) {
if (NeedsAndHasContext()) {
context_->Print(os);
if (context_->has_extension()) {
Handle<HeapObject> extension(context_->extension(), isolate_);

2
src/debug/debug-scopes.h
View File

@ -101,7 +101,7 @@ class ScopeIterator {
bool InInnerScope() const { return !function_.is_null(); }
bool HasContext() const;
bool NeedsContext() const;
bool NeedsAndHasContext() const;
Handle<Context> CurrentContext() const {
DCHECK(HasContext());
return context_;