Skip to content

Commit 51d613d

Browse files
TimothyGutargos
TimothyGu
authored and
targos
committed
src: start annotating native code side effect
PR-URL: #21458 Refs: #20977 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 466601f commit 51d613d

14 files changed

+206
-112
lines changed

‎src/cares_wrap.cc‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2148,8 +2148,8 @@ void Initialize(Local<Object> target,
21482148

21492149
env->SetMethod(target, "getaddrinfo", GetAddrInfo);
21502150
env->SetMethod(target, "getnameinfo", GetNameInfo);
2151-
env->SetMethod(target, "isIPv6", IsIPv6);
2152-
env->SetMethod(target, "canonicalizeIP", CanonicalizeIP);
2151+
env->SetMethodNoSideEffect(target, "isIPv6", IsIPv6);
2152+
env->SetMethodNoSideEffect(target, "canonicalizeIP", CanonicalizeIP);
21532153

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

@@ -2206,7 +2206,7 @@ void Initialize(Local<Object> target,
22062206
env->SetProtoMethod(channel_wrap, "querySoa", Query<QuerySoaWrap>);
22072207
env->SetProtoMethod(channel_wrap, "getHostByAddr", Query<GetHostByAddrWrap>);
22082208

2209-
env->SetProtoMethod(channel_wrap, "getServers", GetServers);
2209+
env->SetProtoMethodNoSideEffect(channel_wrap, "getServers", GetServers);
22102210
env->SetProtoMethod(channel_wrap, "setServers", SetServers);
22112211
env->SetProtoMethod(channel_wrap, "cancel", Cancel);
22122212

‎src/env-inl.h‎

Lines changed: 65 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -681,17 +681,41 @@ inline void Environment::ThrowUVException(int errorno,
681681
inline v8::Local<v8::FunctionTemplate>
682682
Environment::NewFunctionTemplate(v8::FunctionCallback callback,
683683
v8::Local<v8::Signature> signature,
684-
v8::ConstructorBehavior behavior) {
684+
v8::ConstructorBehavior behavior,
685+
v8::SideEffectType side_effect_type) {
685686
v8::Local<v8::External> external = as_external();
686687
return v8::FunctionTemplate::New(isolate(), callback, external,
687-
signature, 0, behavior);
688+
signature, 0, behavior, side_effect_type);
688689
}
689690

690691
inline void Environment::SetMethod(v8::Local<v8::Object> that,
691692
const char* name,
692693
v8::FunctionCallback callback) {
693694
v8::Local<v8::Function> function =
694-
NewFunctionTemplate(callback)->GetFunction();
695+
NewFunctionTemplate(callback,
696+
v8::Local<v8::Signature>(),
697+
// TODO(TimothyGu): Investigate if SetMethod is ever
698+
// used for constructors.
699+
v8::ConstructorBehavior::kAllow,
700+
v8::SideEffectType::kHasSideEffect)->GetFunction();
701+
// kInternalized strings are created in the old space.
702+
const v8::NewStringType type = v8::NewStringType::kInternalized;
703+
v8::Local<v8::String> name_string =
704+
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
705+
that->Set(name_string, function);
706+
function->SetName(name_string); // NODE_SET_METHOD() compatibility.
707+
}
708+
709+
inline void Environment::SetMethodNoSideEffect(v8::Local<v8::Object> that,
710+
const char* name,
711+
v8::FunctionCallback callback) {
712+
v8::Local<v8::Function> function =
713+
NewFunctionTemplate(callback,
714+
v8::Local<v8::Signature>(),
715+
// TODO(TimothyGu): Investigate if SetMethod is ever
716+
// used for constructors.
717+
v8::ConstructorBehavior::kAllow,
718+
v8::SideEffectType::kHasNoSideEffect)->GetFunction();
695719
// kInternalized strings are created in the old space.
696720
const v8::NewStringType type = v8::NewStringType::kInternalized;
697721
v8::Local<v8::String> name_string =
@@ -705,7 +729,24 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
705729
v8::FunctionCallback callback) {
706730
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
707731
v8::Local<v8::FunctionTemplate> t =
708-
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow);
732+
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
733+
v8::SideEffectType::kHasSideEffect);
734+
// kInternalized strings are created in the old space.
735+
const v8::NewStringType type = v8::NewStringType::kInternalized;
736+
v8::Local<v8::String> name_string =
737+
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
738+
that->PrototypeTemplate()->Set(name_string, t);
739+
t->SetClassName(name_string); // NODE_SET_PROTOTYPE_METHOD() compatibility.
740+
}
741+
742+
inline void Environment::SetProtoMethodNoSideEffect(
743+
v8::Local<v8::FunctionTemplate> that,
744+
const char* name,
745+
v8::FunctionCallback callback) {
746+
v8::Local<v8::Signature> signature = v8::Signature::New(isolate(), that);
747+
v8::Local<v8::FunctionTemplate> t =
748+
NewFunctionTemplate(callback, signature, v8::ConstructorBehavior::kThrow,
749+
v8::SideEffectType::kHasNoSideEffect);
709750
// kInternalized strings are created in the old space.
710751
const v8::NewStringType type = v8::NewStringType::kInternalized;
711752
v8::Local<v8::String> name_string =
@@ -717,7 +758,26 @@ inline void Environment::SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
717758
inline void Environment::SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
718759
const char* name,
719760
v8::FunctionCallback callback) {
720-
v8::Local<v8::FunctionTemplate> t = NewFunctionTemplate(callback);
761+
v8::Local<v8::FunctionTemplate> t =
762+
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
763+
v8::ConstructorBehavior::kAllow,
764+
v8::SideEffectType::kHasSideEffect);
765+
// kInternalized strings are created in the old space.
766+
const v8::NewStringType type = v8::NewStringType::kInternalized;
767+
v8::Local<v8::String> name_string =
768+
v8::String::NewFromUtf8(isolate(), name, type).ToLocalChecked();
769+
that->Set(name_string, t);
770+
t->SetClassName(name_string); // NODE_SET_METHOD() compatibility.
771+
}
772+
773+
inline void Environment::SetTemplateMethodNoSideEffect(
774+
v8::Local<v8::FunctionTemplate> that,
775+
const char* name,
776+
v8::FunctionCallback callback) {
777+
v8::Local<v8::FunctionTemplate> t =
778+
NewFunctionTemplate(callback, v8::Local<v8::Signature>(),
779+
v8::ConstructorBehavior::kAllow,
780+
v8::SideEffectType::kHasNoSideEffect);
721781
// kInternalized strings are created in the old space.
722782
const v8::NewStringType type = v8::NewStringType::kInternalized;
723783
v8::Local<v8::String> name_string =

‎src/env.h‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -751,19 +751,34 @@ class Environment {
751751
v8::Local<v8::Signature> signature =
752752
v8::Local<v8::Signature>(),
753753
v8::ConstructorBehavior behavior =
754-
v8::ConstructorBehavior::kAllow);
754+
v8::ConstructorBehavior::kAllow,
755+
v8::SideEffectType side_effect =
756+
v8::SideEffectType::kHasSideEffect);
755757

756758
// Convenience methods for NewFunctionTemplate().
757759
inline void SetMethod(v8::Local<v8::Object> that,
758760
const char* name,
759761
v8::FunctionCallback callback);
762+
760763
inline void SetProtoMethod(v8::Local<v8::FunctionTemplate> that,
761764
const char* name,
762765
v8::FunctionCallback callback);
763766
inline void SetTemplateMethod(v8::Local<v8::FunctionTemplate> that,
764767
const char* name,
765768
v8::FunctionCallback callback);
766769

770+
// Safe variants denote the function has no side effects.
771+
inline void SetMethodNoSideEffect(v8::Local<v8::Object> that,
772+
const char* name,
773+
v8::FunctionCallback callback);
774+
inline void SetProtoMethodNoSideEffect(v8::Local<v8::FunctionTemplate> that,
775+
const char* name,
776+
v8::FunctionCallback callback);
777+
inline void SetTemplateMethodNoSideEffect(
778+
v8::Local<v8::FunctionTemplate> that,
779+
const char* name,
780+
v8::FunctionCallback callback);
781+
767782
void BeforeExit(void (*cb)(void* arg), void* arg);
768783
void RunBeforeExitCallbacks();
769784
void AtExit(void (*cb)(void* arg), void* arg);

‎src/inspector_js_api.cc‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -287,7 +287,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
287287
if (agent->WillWaitForConnect())
288288
env->SetMethod(target, "callAndPauseOnStart", CallAndPauseOnStart);
289289
env->SetMethod(target, "open", Open);
290-
env->SetMethod(target, "url", Url);
290+
env->SetMethodNoSideEffect(target, "url", Url);
291291

292292
env->SetMethod(target, "asyncTaskScheduled", AsyncTaskScheduledWrapper);
293293
env->SetMethod(target, "asyncTaskCanceled",
@@ -298,7 +298,7 @@ void Initialize(Local<Object> target, Local<Value> unused,
298298
InvokeAsyncTaskFnWithId<&Agent::AsyncTaskFinished>);
299299

300300
env->SetMethod(target, "registerAsyncHook", RegisterAsyncHookWrapper);
301-
env->SetMethod(target, "isEnabled", IsEnabled);
301+
env->SetMethodNoSideEffect(target, "isEnabled", IsEnabled);
302302

303303
auto conn_str = FIXED_ONE_BYTE_STRING(env->isolate(), "Connection");
304304
Local<FunctionTemplate> tmpl =

‎src/module_wrap.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -789,11 +789,11 @@ void ModuleWrap::Initialize(Local<Object> target,
789789
env->SetProtoMethod(tpl, "link", Link);
790790
env->SetProtoMethod(tpl, "instantiate", Instantiate);
791791
env->SetProtoMethod(tpl, "evaluate", Evaluate);
792-
env->SetProtoMethod(tpl, "namespace", Namespace);
793-
env->SetProtoMethod(tpl, "getStatus", GetStatus);
794-
env->SetProtoMethod(tpl, "getError", GetError);
795-
env->SetProtoMethod(tpl, "getStaticDependencySpecifiers",
796-
GetStaticDependencySpecifiers);
792+
env->SetProtoMethodNoSideEffect(tpl, "namespace", Namespace);
793+
env->SetProtoMethodNoSideEffect(tpl, "getStatus", GetStatus);
794+
env->SetProtoMethodNoSideEffect(tpl, "getError", GetError);
795+
env->SetProtoMethodNoSideEffect(tpl, "getStaticDependencySpecifiers",
796+
GetStaticDependencySpecifiers);
797797

798798
target->Set(FIXED_ONE_BYTE_STRING(isolate, "ModuleWrap"), tpl->GetFunction());
799799
env->SetMethod(target, "resolve", Resolve);

‎src/node.cc‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ using v8::Promise;
160160
using v8::PropertyCallbackInfo;
161161
using v8::ScriptOrigin;
162162
using v8::SealHandleScope;
163+
using v8::SideEffectType;
163164
using v8::String;
164165
using v8::TryCatch;
165166
using v8::Undefined;
@@ -1969,7 +1970,10 @@ void SetupProcessObject(Environment* env,
19691970
title_string,
19701971
ProcessTitleGetter,
19711972
env->is_main_thread() ? ProcessTitleSetter : nullptr,
1972-
env->as_external()).FromJust());
1973+
env->as_external(),
1974+
v8::DEFAULT,
1975+
v8::None,
1976+
SideEffectType::kHasNoSideEffect).FromJust());
19731977

19741978
// process.version
19751979
READONLY_PROPERTY(process,
@@ -2279,17 +2283,17 @@ void SetupProcessObject(Environment* env,
22792283
env->SetMethod(process, "_getActiveHandles", GetActiveHandles);
22802284
env->SetMethod(process, "_kill", Kill);
22812285

2282-
env->SetMethod(process, "cwd", Cwd);
2286+
env->SetMethodNoSideEffect(process, "cwd", Cwd);
22832287
env->SetMethod(process, "dlopen", DLOpen);
22842288
env->SetMethod(process, "reallyExit", Exit);
2285-
env->SetMethod(process, "uptime", Uptime);
2289+
env->SetMethodNoSideEffect(process, "uptime", Uptime);
22862290

22872291
#if defined(__POSIX__) && !defined(__ANDROID__) && !defined(__CloudABI__)
2288-
env->SetMethod(process, "getuid", GetUid);
2289-
env->SetMethod(process, "geteuid", GetEUid);
2290-
env->SetMethod(process, "getgid", GetGid);
2291-
env->SetMethod(process, "getegid", GetEGid);
2292-
env->SetMethod(process, "getgroups", GetGroups);
2292+
env->SetMethodNoSideEffect(process, "getuid", GetUid);
2293+
env->SetMethodNoSideEffect(process, "geteuid", GetEUid);
2294+
env->SetMethodNoSideEffect(process, "getgid", GetGid);
2295+
env->SetMethodNoSideEffect(process, "getegid", GetEGid);
2296+
env->SetMethodNoSideEffect(process, "getgroups", GetGroups);
22932297
#endif // __POSIX__ && !defined(__ANDROID__) && !defined(__CloudABI__)
22942298
}
22952299

‎src/node_buffer.cc‎

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1083,12 +1083,12 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
10831083
Local<Object> proto = args[0].As<Object>();
10841084
env->set_buffer_prototype_object(proto);
10851085

1086-
env->SetMethod(proto, "asciiSlice", StringSlice<ASCII>);
1087-
env->SetMethod(proto, "base64Slice", StringSlice<BASE64>);
1088-
env->SetMethod(proto, "latin1Slice", StringSlice<LATIN1>);
1089-
env->SetMethod(proto, "hexSlice", StringSlice<HEX>);
1090-
env->SetMethod(proto, "ucs2Slice", StringSlice<UCS2>);
1091-
env->SetMethod(proto, "utf8Slice", StringSlice<UTF8>);
1086+
env->SetMethodNoSideEffect(proto, "asciiSlice", StringSlice<ASCII>);
1087+
env->SetMethodNoSideEffect(proto, "base64Slice", StringSlice<BASE64>);
1088+
env->SetMethodNoSideEffect(proto, "latin1Slice", StringSlice<LATIN1>);
1089+
env->SetMethodNoSideEffect(proto, "hexSlice", StringSlice<HEX>);
1090+
env->SetMethodNoSideEffect(proto, "ucs2Slice", StringSlice<UCS2>);
1091+
env->SetMethodNoSideEffect(proto, "utf8Slice", StringSlice<UTF8>);
10921092

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

11181118
env->SetMethod(target, "setupBufferJS", SetupBufferJS);
1119-
env->SetMethod(target, "createFromString", CreateFromString);
1119+
env->SetMethodNoSideEffect(target, "createFromString", CreateFromString);
11201120

1121-
env->SetMethod(target, "byteLengthUtf8", ByteLengthUtf8);
1121+
env->SetMethodNoSideEffect(target, "byteLengthUtf8", ByteLengthUtf8);
11221122
env->SetMethod(target, "copy", Copy);
1123-
env->SetMethod(target, "compare", Compare);
1124-
env->SetMethod(target, "compareOffset", CompareOffset);
1123+
env->SetMethodNoSideEffect(target, "compare", Compare);
1124+
env->SetMethodNoSideEffect(target, "compareOffset", CompareOffset);
11251125
env->SetMethod(target, "fill", Fill);
1126-
env->SetMethod(target, "indexOfBuffer", IndexOfBuffer);
1127-
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
1128-
env->SetMethod(target, "indexOfString", IndexOfString);
1126+
env->SetMethodNoSideEffect(target, "indexOfBuffer", IndexOfBuffer);
1127+
env->SetMethodNoSideEffect(target, "indexOfNumber", IndexOfNumber);
1128+
env->SetMethodNoSideEffect(target, "indexOfString", IndexOfString);
11291129

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

1134-
env->SetMethod(target, "encodeUtf8String", EncodeUtf8String);
1134+
env->SetMethodNoSideEffect(target, "encodeUtf8String", EncodeUtf8String);
11351135

11361136
target->Set(env->context(),
11371137
FIXED_ONE_BYTE_STRING(env->isolate(), "kMaxLength"),

0 commit comments

Comments
 (0)