Skip to content

wasm: add missing init reported by coverity - #42897

Closed
mhdawson wants to merge 1 commit into
nodejs:masterfrom
mhdawson:coverity-21
Closed

wasm: add missing init reported by coverity#42897
mhdawson wants to merge 1 commit into
nodejs:masterfrom
mhdawson:coverity-21

Conversation

@mhdawson

Copy link
Copy Markdown
Member

Signed-off-by: Michael Dawson mdawson@devrus.com

Signed-off-by: Michael Dawson <mdawson@devrus.com>
@nodejs-github-botnodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Apr 28, 2022
@mhdawson

Copy link
Copy Markdown
MemberAuthor

Report from coverity:

26private:27WasmStreamingObject(Environment* env, v8::Local<v8::Object> object)
28 : BaseObject(env, object) {
29MakeWeak();
CID254660 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
2. uninit_member: Non-staticclassmember wasm_size_ is not initialized in this constructor nor in any functions that it calls.
30 }
3132~WasmStreamingObject() override {}
3334private:35staticvoidNew(const v8::FunctionCallbackInfo<v8::Value>& args);
36staticvoidSetURL(const v8::FunctionCallbackInfo<v8::Value>& args);
37staticvoidPush(const v8::FunctionCallbackInfo<v8::Value>& args);
38staticvoidFinish(const v8::FunctionCallbackInfo<v8::Value>& args);
39staticvoidAbort(const v8::FunctionCallbackInfo<v8::Value>& args);
4041 std::shared_ptr<v8::WasmStreaming> streaming_;
1. member_decl: Class member declaration for wasm_size_.
42size_t wasm_size_;
43};

@tniessen

Copy link
Copy Markdown
Member

FWIW, this is where the initialization currently happens:

Local<Function> ctor = Initialize(env);
Local<Object> obj;
if (!ctor->NewInstance(env->context(), 0, nullptr).ToLocal(&obj)) {
return MaybeLocal<Object>();
}
CHECK(streaming);
WasmStreamingObject* ptr = Unwrap<WasmStreamingObject>(obj);
CHECK_NOT_NULL(ptr);
ptr->streaming_ = streaming;
ptr->wasm_size_ = 0;
return obj;

Other functions rely on the assumption that the initialization ran (e.g., will abort the process if streaming_ has not been initialized to a non-nullptr value).

Eventually, we might want to revisit how we initialize C++ backed JS objects in general.

@mhdawsonmhdawson added the request-ci Add this label to start a Jenkins CI on a PR. label May 2, 2022
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 2, 2022
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

mhdawson added a commit that referenced this pull request May 10, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #42897
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
@mhdawson

Copy link
Copy Markdown
MemberAuthor

Landed in 7649989

BethGriggs pushed a commit that referenced this pull request May 16, 2022
Signed-off-by: Michael Dawson <mdawson@devrus.com>
PR-URL: #42897
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Mestery <mestery@protonmail.com>
@RafaelGSSRafaelGSS mentioned this pull request May 16, 2022
@juanarbol

Copy link
Copy Markdown
Member

Backport refs: #42701

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++Issues and PRs that require attention from people who are familiar with C++.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants

@mhdawson@tniessen@nodejs-github-bot@juanarbol@lpinca@cjihrig@Mesteery@VoltrexKeyva@targos