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

[inspector] Use ephemeron table for exception metadata

Browse Source 
EphemeronHashTable does not trigger interrupts when accessed
(as opposed to calling the WeakMapGet builtin), so it avoids
the use-after-free problem when reading exception metadata
triggers session disconnect while holding a reference
to the session.

Bug: chromium:1241860
Change-Id: I29264b04b8daf682e7c33a97faedf50e323d57c4
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3158326
Commit-Queue: Jaroslav Sevcik <jarin@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/main@{#76864}
This commit is contained in:
Jaroslav Sevcik 2021-09-14 21:08:03 +02:00 committed by V8 LUCI CQ
parent 9c601fbff8
commit 7994004493
10 changed files with 111 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
src/api/api.h
View File

@ -42,7 +42,7 @@ namespace debug {
class AccessorPair;
class GeneratorObject;
class Script;
class WeakMap;
class EphemeronTable;
} // namespace debug
// Constants used in the implementation of the API. The most natural thing
@ -135,7 +135,7 @@ class RegisteredExtension {
V(Proxy, JSProxy) \
V(debug::GeneratorObject, JSGeneratorObject) \
V(debug::Script, Script) \
V(debug::WeakMap, JSWeakMap) \
V(debug::EphemeronTable, EphemeronHashTable) \
V(debug::AccessorPair, AccessorPair) \
V(Promise, JSPromise) \
V(Primitive, Object) \

76
src/debug/debug-interface.cc
View File

@ -1177,61 +1177,43 @@ TypeProfile::ScriptData TypeProfile::GetScriptData(size_t i) const {
return ScriptData(i, type_profile_);
}
v8::MaybeLocal<v8::Value> WeakMap::Get(v8::Local<v8::Context> context,
v8::Local<v8::Value> key) {
PREPARE_FOR_EXECUTION(context, WeakMap, Get, Value);
auto self = Utils::OpenHandle(this);
Local<Value> result;
i::Handle<i::Object> argv[] = {Utils::OpenHandle(*key)};
has_pending_exception =
!ToLocal<Value>(i::Execution::CallBuiltin(isolate, isolate->weakmap_get(),
self, arraysize(argv), argv),
&result);
RETURN_ON_FAILED_EXECUTION(Value);
RETURN_ESCAPED(result);
MaybeLocal<v8::Value> EphemeronTable::Get(v8::Isolate* isolate,
v8::Local<v8::Value> key) {
i::Isolate* internal_isolate = reinterpret_cast<i::Isolate*>(isolate);
auto self = i::Handle<i::EphemeronHashTable>::cast(Utils::OpenHandle(this));
i::Handle<i::Object> internal_key = Utils::OpenHandle(*key);
DCHECK(internal_key->IsJSReceiver());
i::Handle<i::Object> value(self->Lookup(internal_key), internal_isolate);
if (value->IsTheHole()) return {};
return Utils::ToLocal(value);
}
v8::Maybe<bool> WeakMap::Delete(v8::Local<v8::Context> context,
v8::Local<v8::Value> key) {
PREPARE_FOR_EXECUTION_WITH_CONTEXT(context, WeakMap, Delete, Nothing<bool>(),
InternalEscapableScope, false);
auto self = Utils::OpenHandle(this);
Local<Value> result;
i::Handle<i::Object> argv[] = {Utils::OpenHandle(*key)};
has_pending_exception = !ToLocal<Value>(
i::Execution::CallBuiltin(isolate, isolate->weakmap_delete(), self,
arraysize(argv), argv),
&result);
RETURN_ON_FAILED_EXECUTION_PRIMITIVE(bool);
return Just(result->IsTrue());
Local<EphemeronTable> EphemeronTable::Set(v8::Isolate* isolate,
v8::Local<v8::Value> key,
v8::Local<v8::Value> value) {
auto self = i::Handle<i::EphemeronHashTable>::cast(Utils::OpenHandle(this));
i::Handle<i::Object> internal_key = Utils::OpenHandle(*key);
i::Handle<i::Object> internal_value = Utils::OpenHandle(*value);
DCHECK(internal_key->IsJSReceiver());
i::Handle<i::EphemeronHashTable> result(
i::EphemeronHashTable::Put(self, internal_key, internal_value));
return ToApiHandle<EphemeronTable>(result);
}
v8::MaybeLocal<WeakMap> WeakMap::Set(v8::Local<v8::Context> context,
v8::Local<v8::Value> key,
v8::Local<v8::Value> value) {
PREPARE_FOR_EXECUTION(context, WeakMap, Set, WeakMap);
auto self = Utils::OpenHandle(this);
i::Handle<i::Object> result;
i::Handle<i::Object> argv[] = {Utils::OpenHandle(*key),
Utils::OpenHandle(*value)};
has_pending_exception =
!i::Execution::CallBuiltin(isolate, isolate->weakmap_set(), self,
arraysize(argv), argv)
.ToHandle(&result);
RETURN_ON_FAILED_EXECUTION(WeakMap);
RETURN_ESCAPED(Local<WeakMap>::Cast(Utils::ToLocal(result)));
}
Local<WeakMap> WeakMap::New(v8::Isolate* isolate) {
Local<EphemeronTable> EphemeronTable::New(v8::Isolate* isolate) {
i::Isolate* i_isolate = reinterpret_cast<i::Isolate*>(isolate);
LOG_API(i_isolate, WeakMap, New);
ENTER_V8_NO_SCRIPT_NO_EXCEPTION(i_isolate);
i::Handle<i::JSWeakMap> obj = i_isolate->factory()->NewJSWeakMap();
return ToApiHandle<WeakMap>(obj);
i::Handle<i::EphemeronHashTable> table =
i::EphemeronHashTable::New(i_isolate, 0);
return ToApiHandle<EphemeronTable>(table);
}
WeakMap* WeakMap::Cast(v8::Value* value) {
return static_cast<WeakMap*>(value);
EphemeronTable* EphemeronTable::Cast(v8::Value* value) {
return static_cast<EphemeronTable*>(value);
}
Local<Value> AccessorPair::getter() {

16
src/debug/debug-interface.h
View File

@ -570,19 +570,17 @@ class V8_NODISCARD DisableBreakScope {
std::unique_ptr<i::DisableBreak> scope_;
};
class WeakMap : public v8::Object {
class EphemeronTable : public v8::Object {
public:
WeakMap() = delete;
EphemeronTable() = delete;
V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT v8::MaybeLocal<v8::Value> Get(
v8::Local<v8::Context> context, v8::Local<v8::Value> key);
V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT v8::Maybe<bool> Delete(
v8::Local<v8::Context> context, v8::Local<v8::Value> key);
V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT v8::MaybeLocal<WeakMap> Set(
v8::Local<v8::Context> context, v8::Local<v8::Value> key,
v8::Isolate* isolate, v8::Local<v8::Value> key);
V8_EXPORT_PRIVATE V8_WARN_UNUSED_RESULT v8::Local<EphemeronTable> Set(
v8::Isolate* isolate, v8::Local<v8::Value> key,
v8::Local<v8::Value> value);
V8_EXPORT_PRIVATE static Local<WeakMap> New(v8::Isolate* isolate);
V8_INLINE static WeakMap* Cast(Value* obj);
V8_EXPORT_PRIVATE static Local<EphemeronTable> New(v8::Isolate* isolate);
V8_INLINE static EphemeronTable* Cast(Value* obj);
};
/**

15
src/inspector/inspected-context.cc
View File

@ -126,12 +126,15 @@ void InspectedContext::discardInjectedScript(int sessionId) {
bool InspectedContext::addInternalObject(v8::Local<v8::Object> object,
V8InternalValueType type) {
if (m_internalObjects.IsEmpty()) {
m_internalObjects.Reset(isolate(), v8::debug::WeakMap::New(isolate()));
m_internalObjects.Reset(isolate(),
v8::debug::EphemeronTable::New(isolate()));
}
return !m_internalObjects.Get(isolate())
->Set(m_context.Get(isolate()), object,
v8::Integer::New(isolate(), static_cast<int>(type)))
.IsEmpty();
v8::Local<v8::debug::EphemeronTable> new_map =
m_internalObjects.Get(isolate())->Set(
isolate(), object,
v8::Integer::New(isolate(), static_cast<int>(type)));
m_internalObjects.Reset(isolate(), new_map);
return true;
}
V8InternalValueType InspectedContext::getInternalType(
@ -139,7 +142,7 @@ V8InternalValueType InspectedContext::getInternalType(
if (m_internalObjects.IsEmpty()) return V8InternalValueType::kNone;
v8::Local<v8::Value> typeValue;
if (!m_internalObjects.Get(isolate())
->Get(m_context.Get(isolate()), object)
->Get(isolate(), object)
.ToLocal(&typeValue) ||
!typeValue->IsUint32()) {
return V8InternalValueType::kNone;

2
src/inspector/inspected-context.h
View File

@ -77,7 +77,7 @@ class InspectedContext {
std::unordered_set<int> m_reportedSessionIds;
std::unordered_map<int, std::unique_ptr<InjectedScript>> m_injectedScripts;
WeakCallbackData* m_weakCallbackData;
v8::Global<v8::debug::WeakMap> m_internalObjects;
v8::Global<v8::debug::EphemeronTable> m_internalObjects;
};
} // namespace v8_inspector

27
src/inspector/v8-inspector-impl.cc
View File

@ -348,7 +348,14 @@ v8::MaybeLocal<v8::Context> V8InspectorImpl::regexContext() {
}
v8::MaybeLocal<v8::Context> V8InspectorImpl::exceptionMetaDataContext() {
return {};
if (m_exceptionMetaDataContext.IsEmpty()) {
m_exceptionMetaDataContext.Reset(m_isolate, v8::Context::New(m_isolate));
if (m_exceptionMetaDataContext.IsEmpty()) {
DCHECK(m_isolate->IsExecutionTerminating());
return {};
}
}
return m_exceptionMetaDataContext.Get(m_isolate);
}
void V8InspectorImpl::discardInspectedContext(int contextGroupId,
@ -479,19 +486,17 @@ bool V8InspectorImpl::associateExceptionData(v8::Local<v8::Context>,
v8::Context::Scope contextScope(context);
v8::HandleScope handles(m_isolate);
if (m_exceptionMetaData.IsEmpty())
m_exceptionMetaData.Reset(m_isolate, v8::debug::WeakMap::New(m_isolate));
m_exceptionMetaData.Reset(m_isolate,
v8::debug::EphemeronTable::New(m_isolate));
v8::Local<v8::debug::WeakMap> map = m_exceptionMetaData.Get(m_isolate);
v8::MaybeLocal<v8::Value> entry = map->Get(context, exception);
v8::Local<v8::debug::EphemeronTable> map = m_exceptionMetaData.Get(m_isolate);
v8::MaybeLocal<v8::Value> entry = map->Get(m_isolate, exception);
v8::Local<v8::Object> object;
if (entry.IsEmpty() || !entry.ToLocalChecked()->IsObject()) {
object =
v8::Object::New(m_isolate, v8::Null(m_isolate), nullptr, nullptr, 0);
v8::MaybeLocal<v8::debug::WeakMap> new_map =
map->Set(context, exception, object);
if (!new_map.IsEmpty()) {
m_exceptionMetaData.Reset(m_isolate, new_map.ToLocalChecked());
}
m_exceptionMetaData.Reset(m_isolate,
map->Set(m_isolate, exception, object));
} else {
object = entry.ToLocalChecked().As<v8::Object>();
}
@ -511,8 +516,8 @@ v8::MaybeLocal<v8::Object> V8InspectorImpl::getAssociatedExceptionData(
!exceptionMetaDataContext().ToLocal(&context)) {
return v8::MaybeLocal<v8::Object>();
}
v8::Local<v8::debug::WeakMap> map = m_exceptionMetaData.Get(m_isolate);
auto entry = map->Get(context, exception);
v8::Local<v8::debug::EphemeronTable> map = m_exceptionMetaData.Get(m_isolate);
auto entry = map->Get(m_isolate, exception);
v8::Local<v8::Value> object;
if (!entry.ToLocal(&object) || !object->IsObject())
return v8::MaybeLocal<v8::Object>();

13
src/inspector/v8-inspector-impl.h
View File

@ -56,7 +56,7 @@ class V8StackTraceImpl;
class V8InspectorImpl : public V8Inspector {
public:
V8InspectorImpl(v8::Isolate*, V8InspectorClient*);
V8_EXPORT_PRIVATE V8InspectorImpl(v8::Isolate*, V8InspectorClient*);
~V8InspectorImpl() override;
V8InspectorImpl(const V8InspectorImpl&) = delete;
V8InspectorImpl& operator=(const V8InspectorImpl&) = delete;
@ -110,10 +110,9 @@ class V8InspectorImpl : public V8Inspector {
void externalAsyncTaskStarted(const V8StackTraceId& parent) override;
void externalAsyncTaskFinished(const V8StackTraceId& parent) override;
bool associateExceptionData(v8::Local<v8::Context>,
v8::Local<v8::Value> exception,
v8::Local<v8::Name> key,
v8::Local<v8::Value> value) override;
V8_EXPORT_PRIVATE bool associateExceptionData(
v8::Local<v8::Context>, v8::Local<v8::Value> exception,
v8::Local<v8::Name> key, v8::Local<v8::Value> value) override;
unsigned nextExceptionId() { return ++m_lastExceptionId; }
void enableStackCapturingIfNeeded();
@ -134,7 +133,7 @@ class V8InspectorImpl : public V8Inspector {
int contextGroupId,
const std::function<void(V8InspectorSessionImpl*)>& callback);
int64_t generateUniqueId();
v8::MaybeLocal<v8::Object> getAssociatedExceptionData(
V8_EXPORT_PRIVATE v8::MaybeLocal<v8::Object> getAssociatedExceptionData(
v8::Local<v8::Value> exception);
class EvaluateScope {
@ -160,7 +159,7 @@ class V8InspectorImpl : public V8Inspector {
std::unique_ptr<V8Debugger> m_debugger;
v8::Global<v8::Context> m_regexContext;
v8::Global<v8::Context> m_exceptionMetaDataContext;
v8::Global<v8::debug::WeakMap> m_exceptionMetaData;
v8::Global<v8::debug::EphemeronTable> m_exceptionMetaData;
int m_capturingStackTracesCount;
unsigned m_lastExceptionId;
int m_lastContextId;

9
test/cctest/test-debug.cc
View File

@ -598,10 +598,11 @@ TEST(BreakPointApiIntrinsics) {
CHECK_EQ(2, break_point_hit_count);
break_point_hit_count = 0;
v8::Local<v8::debug::WeakMap> weakmap =
v8::debug::WeakMap::New(env->GetIsolate());
CHECK(!weakmap->Set(env.local(), weakmap, v8_num(1)).IsEmpty());
CHECK(!weakmap->Get(env.local(), weakmap).IsEmpty());
v8::Local<v8::debug::EphemeronTable> weakmap =
v8::debug::EphemeronTable::New(env->GetIsolate());
v8::Local<v8::Object> key = v8::Object::New(env->GetIsolate());
CHECK(!weakmap->Set(env->GetIsolate(), key, v8_num(1)).IsEmpty());
CHECK(!weakmap->Get(env->GetIsolate(), key).IsEmpty());
CHECK_EQ(0, break_point_hit_count);
}

36
test/cctest/test-inspector.cc
View File

@ -9,6 +9,7 @@
#include "include/v8-primitive.h"
#include "src/inspector/protocol/Runtime.h"
#include "src/inspector/string-util.h"
#include "src/inspector/v8-inspector-impl.h"
#include "test/cctest/cctest.h"
using v8_inspector::StringBuffer;
@ -169,3 +170,38 @@ TEST(BinaryBase64RoundTrip) {
CHECK_EQ(values[i], roundtrip_binary.data()[i]);
}
}
TEST(NoInterruptOnGetAssociatedData) {
LocalContext env;
v8::Isolate* isolate = env->GetIsolate();
v8::HandleScope handle_scope(isolate);
v8_inspector::V8InspectorClient default_client;
std::unique_ptr<v8_inspector::V8InspectorImpl> inspector(
new v8_inspector::V8InspectorImpl(isolate, &default_client));
v8::Local<v8::Context> context = env->GetIsolate()->GetCurrentContext();
v8::Local<v8::Value> error = v8::Exception::Error(v8_str("custom error"));
v8::Local<v8::Name> key = v8_str("key");
v8::Local<v8::Value> value = v8_str("value");
inspector->associateExceptionData(context, error, key, value);
struct InterruptRecorder {
static void handler(v8::Isolate* isolate, void* data) {
reinterpret_cast<InterruptRecorder*>(data)->WasInvoked = true;
}
bool WasInvoked = false;
} recorder;
isolate->RequestInterrupt(&InterruptRecorder::handler, &recorder);
v8::Local<v8::Object> data =
inspector->getAssociatedExceptionData(error).ToLocalChecked();
CHECK(!recorder.WasInvoked);
CHECK_EQ(data->Get(context, key).ToLocalChecked(), value);
CompileRun("0");
CHECK(recorder.WasInvoked);
}

3
test/inspector/inspector.status
View File

@ -20,9 +20,6 @@
# loop instead of properly reporting a RangeError for a stack overflow.
'regress/regress-crbug-1080638': [SKIP],
# https://crbug.com/1241860
'runtime/exception-thrown-metadata': [SKIP],
# Tests that need to run sequentially (e.g. due to memory consumption).
'runtime/console-messages-limits': [PASS, HEAVY],
'runtime/regression-732717': [PASS, HEAVY],