Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: start annotating native code side effect #21458

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/cares_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2107,8 +2107,8 @@ void Initialize(Local<Object> target,

env->SetMethod(target, "getaddrinfo", GetAddrInfo);
env->SetMethod(target, "getnameinfo", GetNameInfo);
env->SetMethod(target, "isIPv6", IsIPv6);
env->SetMethod(target, "canonicalizeIP", CanonicalizeIP);
env->SetSafeMethod(target, "isIPv6", IsIPv6);
env->SetSafeMethod(target, "canonicalizeIP", CanonicalizeIP);

env->SetMethod(target, "strerror", StrError);

Expand Down Expand Up @@ -2165,7 +2165,7 @@ void Initialize(Local<Object> target,
env->SetProtoMethod(channel_wrap, "querySoa", Query<QuerySoaWrap>);
env->SetProtoMethod(channel_wrap, "getHostByAddr", Query<GetHostByAddrWrap>);

env->SetProtoMethod(channel_wrap, "getServers", GetServers);
env->SetSafeProtoMethod(channel_wrap, "getServers", GetServers);
env->SetProtoMethod(channel_wrap, "setServers", SetServers);
env->SetProtoMethod(channel_wrap, "cancel", Cancel);

Expand Down
69 changes: 64 additions & 5 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -675,17 +675,41 @@ inline void Environment::ThrowUVException(int errorno,
inline v8::Local<v8::FunctionTemplate>
Environment::NewFunctionTemplate(v8::FunctionCallback callback,
v8::Local<v8::Signature> signature,
v8::ConstructorBehavior behavior) {
v8::ConstructorBehavior behavior,
v8::SideEffectType side_effect_type) {
v8::Local<v8::External> external = as_external();
return v8::FunctionTemplate::New(isolate(), callback, external,
signature, 0, behavior);
signature, 0, behavior, side_effect_type);
}

inline void Environment::SetMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Function> function =
NewFunctionTemplate(callback)->GetFunction();
NewFunctionTemplate(callback,
v8::Local<v8::Signature>(),
// TODO(TimothyGu): Investigate if SetMethod is ever
// used for constructors.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's easy to check, isn't it? Or do you suspect make test will miss instances?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather leave that to a separate PR is what I meant.

v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasSideEffect)->GetFunction();
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->Set(name_string, function);
function->SetName(name_string); // NODE_SET_METHOD() compatibility.
}

inline void Environment::SetSafeMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Function> function =
NewFunctionTemplate(callback,
v8::Local<v8::Signature>(),
// TODO(TimothyGu): Investigate if SetMethod is ever
// used for constructors.
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasNoSideEffect)->GetFunction();
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand All @@ -699,7 +723,23 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
v8::FunctionCallback callback) {
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow);
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
v8::SideEffectType::kHasSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->PrototypeTemplate()->Set(name_string, t);
t->SetClassName(name_string); // NODE_SET_PROTOTYPE_METHOD() compatibility.
}

inline void Environment::SetSafeProtoMethod(
v8::Local<v8::FunctionTemplate> that, const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
v8::SideEffectType::kHasNoSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand All @@ -711,7 +751,26 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
inline void Environment::SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::FunctionTemplate> t = NewFunctionTemplate(callback);
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
that->Set(name_string, t);
t->SetClassName(name_string); // NODE_SET_METHOD() compatibility.
}

inline void Environment::SetSafeTemplateMethod(
v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback) {
v8::Local<v8::FunctionTemplate> t =
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
v8::ConstructorBehavior::kAllow,
v8::SideEffectType::kHasNoSideEffect);
// kInternalized strings are created in the old space.
const v8::NewStringType type = v8::NewStringType::kInternalized;
v8::Local<v8::String> name_string =
Expand Down
16 changes: 15 additions & 1 deletion src/env.h
Original file line number Diff line number Diff line change
Expand Up @@ -752,19 +752,33 @@ class Environment {
v8::Local<v8::Signature> signature =
v8::Local<v8::Signature>(),
v8::ConstructorBehavior behavior =
v8::ConstructorBehavior::kAllow);
v8::ConstructorBehavior::kAllow,
v8::SideEffectType side_effect =
v8::SideEffectType::kHasSideEffect);

// Convenience methods for NewFunctionTemplate().
inline void SetMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback);

inline void SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);
inline void SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);

// Safe variants denote the function has no side effects.
inline void SetSafeMethod(v8::Local<v8::Object> that,
const char* name,
v8::FunctionCallback callback);
inline void SetSafeProtoMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);
inline void SetSafeTemplateMethod(v8::Local<v8::FunctionTemplate> that,
const char* name,
v8::FunctionCallback callback);

void BeforeExit(void (*cb)(void* arg), void* arg);
void RunBeforeExitCallbacks();
void AtExit(void (*cb)(void* arg), void* arg);
Expand Down
4 changes: 2 additions & 2 deletions src/inspector_js_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
if (agent->IsWaitingForConnect())
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
env->SetMethod(target, "open", Open);
env->SetMethod(target, "url", Url);
env->SetSafeMethod(target, "url", Url);

env->SetMethod(target, "asyncTaskScheduled", AsyncTaskScheduledWrapper);
env->SetMethod(target, "asyncTaskCanceled",
Expand All @@ -300,7 +300,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
InvokeAsyncTaskFnWithId<&Agent::AsyncTaskFinished>);

env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
env->SetMethod(target, "isEnabled", IsEnabled);
env->SetSafeMethod(target, "isEnabled", IsEnabled);

auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
Local<FunctionTemplate> tmpl =
Expand Down
10 changes: 5 additions & 5 deletions src/module_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -789,11 +789,11 @@ void ModuleWrap::Initialize(Local<Object> target,
env->SetProtoMethod(tpl, "link", Link);
env->SetProtoMethod(tpl, "instantiate", Instantiate);
env->SetProtoMethod(tpl, "evaluate", Evaluate);
env->SetProtoMethod(tpl, "namespace", Namespace);
env->SetProtoMethod(tpl, "getStatus", GetStatus);
env->SetProtoMethod(tpl, "getError", GetError);
env->SetProtoMethod(tpl, "getStaticDependencySpecifiers",
GetStaticDependencySpecifiers);
env->SetSafeProtoMethod(tpl, "namespace", Namespace);
env->SetSafeProtoMethod(tpl, "getStatus", GetStatus);
env->SetSafeProtoMethod(tpl, "getError", GetError);
env->SetSafeProtoMethod(tpl, "getStaticDependencySpecifiers",
GetStaticDependencySpecifiers);

target->Set(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction());
env->SetMethod(target, "resolve", Resolve);
Expand Down
20 changes: 12 additions & 8 deletions src/node.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ using v8::Promise;
using v8::PropertyCallbackInfo;
using v8::ScriptOrigin;
using v8::SealHandleScope;
using v8::SideEffectType;
using v8::String;
using v8::TryCatch;
using v8::Undefined;
Expand Down Expand Up @@ -1947,7 +1948,10 @@ void SetupProcessObject(Environment* env,
title_string,
ProcessTitleGetter,
env->is_main_thread() ? ProcessTitleSetter : nullptr,
env->as_external()).FromJust());
env->as_external(),
v8::DEFAULT,
v8::None,
SideEffectType::kHasNoSideEffect).FromJust());

// process.version
READONLY_PROPERTY(process,
Expand Down Expand Up @@ -2252,17 +2256,17 @@ void SetupProcessObject(Environment* env,
env->SetMethod(process, "_getActiveHandles", GetActiveHandles);
env->SetMethod(process, "_kill", Kill);

env->SetMethod(process, "cwd", Cwd);
env->SetSafeMethod(process, "cwd", Cwd);
env->SetMethod(process, "dlopen", DLOpen);
env->SetMethod(process, "reallyExit", Exit);
env->SetMethod(process, "uptime", Uptime);
env->SetSafeMethod(process, "uptime", Uptime);

#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
env->SetMethod(process, "getuid", GetUid);
env->SetMethod(process, "geteuid", GetEUid);
env->SetMethod(process, "getgid", GetGid);
env->SetMethod(process, "getegid", GetEGid);
env->SetMethod(process, "getgroups", GetGroups);
env->SetSafeMethod(process, "getuid", GetUid);
env->SetSafeMethod(process, "geteuid", GetEUid);
env->SetSafeMethod(process, "getgid", GetGid);
env->SetSafeMethod(process, "getegid", GetEGid);
env->SetSafeMethod(process, "getgroups", GetGroups);
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
}

Expand Down
28 changes: 14 additions & 14 deletions src/node_buffer.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1083,12 +1083,12 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
Local<Object> proto = args[0].As<Object>();
env->set_buffer_prototype_object(proto);

env->SetMethod(proto, "asciiSlice", StringSlice<ASCII>);
env->SetMethod(proto, "base64Slice", StringSlice<BASE64>);
env->SetMethod(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetMethod(proto, "hexSlice", StringSlice<HEX>);
env->SetMethod(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetMethod(proto, "utf8Slice", StringSlice<UTF8>);
env->SetSafeMethod(proto, "asciiSlice", StringSlice<ASCII>);
env->SetSafeMethod(proto, "base64Slice", StringSlice<BASE64>);
env->SetSafeMethod(proto, "latin1Slice", StringSlice<LATIN1>);
env->SetSafeMethod(proto, "hexSlice", StringSlice<HEX>);
env->SetSafeMethod(proto, "ucs2Slice", StringSlice<UCS2>);
env->SetSafeMethod(proto, "utf8Slice", StringSlice<UTF8>);

env->SetMethod(proto, "asciiWrite", StringWrite<ASCII>);
env->SetMethod(proto, "base64Write", StringWrite<BASE64>);
Expand Down Expand Up @@ -1116,22 +1116,22 @@ void Initialize(Local<Object> target,
Environment* env = Environment::GetCurrent(context);

env->SetMethod(target, "setupBufferJS", SetupBufferJS);
env->SetMethod(target, "createFromString", CreateFromString);
env->SetSafeMethod(target, "createFromString", CreateFromString);

env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetSafeMethod(target, "byteLengthUtf8", ByteLengthUtf8);
env->SetMethod(target, "copy", Copy);
env->SetMethod(target, "compare", Compare);
env->SetMethod(target, "compareOffset", CompareOffset);
env->SetSafeMethod(target, "compare", Compare);
env->SetSafeMethod(target, "compareOffset", CompareOffset);
env->SetMethod(target, "fill", Fill);
env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
env->SetMethod(target, "indexOfString", IndexOfString);
env->SetSafeMethod(target, "indexOfBuffer", IndexOfBuffer);
env->SetSafeMethod(target, "indexOfNumber", IndexOfNumber);
env->SetSafeMethod(target, "indexOfString", IndexOfString);

env->SetMethod(target, "swap16", Swap16);
env->SetMethod(target, "swap32", Swap32);
env->SetMethod(target, "swap64", Swap64);

env->SetMethod(target, "encodeUtf8String", EncodeUtf8String);
env->SetSafeMethod(target, "encodeUtf8String", EncodeUtf8String);

target->Set(env->context(),
FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"),
Expand Down
Loading