Skip to content

src: minor cleanup and simplification of crypto::Hash

Rodrigo Muino Tomonari requested to merge github/fork/jasnell/crypto-hash-cleanup into master

Some minor cleanups and simplification for crypto::Hash

@addaleax @targos... I was working on this because valgrind --leak-check=full is reporting the following when crypto.createHash() is called but I can't seem to be able to track down the uninitialized memory...

==29245== Conditional jump or move depends on uninitialised value(s)
==29245==    at 0xA985F9: node::crypto::Hash::New(v8::FunctionCallbackInfo<v8::Value> const&) (in /home/james/node/node/out/Release/node)
==29245==    by 0xBB3E9A: v8::internal::MaybeHandle<v8::internal::Object> v8::internal::(anonymous namespace)::HandleApiCallHelper<true>(v8::internal::Isolate*, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::HeapObject>, v8::internal::Handle<v8::internal::FunctionTemplateInfo>, v8::internal::Handle<v8::internal::Object>, v8::internal::BuiltinArguments) (in /home/james/node/node/out/Release/node)
==29245==    by 0xBB4D57: v8::internal::Builtin_HandleApiCall(int, unsigned long*, v8::internal::Isolate*) (in /home/james/node/node/out/Release/node)
==29245==    by 0x14B1278: Builtins_CEntry_Return1_DontSaveFPRegs_ArgvOnStack_BuiltinExit (in /home/james/node/node/out/Release/node)
==29245==    by 0x14466A0: Builtins_JSBuiltinsConstructStub (in /home/james/node/node/out/Release/node)
==29245==    by 0x152EE22: Builtins_ConstructHandler (in /home/james/node/node/out/Release/node)
==29245==    by 0x144A521: Builtins_InterpreterEntryTrampoline (in /home/james/node/node/out/Release/node)
==29245==    by 0x14465A9: Builtins_JSConstructStubGeneric (in /home/james/node/node/out/Release/node)
==29245==    by 0x152EE22: Builtins_ConstructHandler (in /home/james/node/node/out/Release/node)
==29245==    by 0x144A521: Builtins_InterpreterEntryTrampoline (in /home/james/node/node/out/Release/node)
==29245==    by 0x1444458: Builtins_ArgumentsAdaptorTrampoline (in /home/james/node/node/out/Release/node)
==29245==    by 0x144A521: Builtins_InterpreterEntryTrampoline (in /home/james/node/node/out/Release/node)
==29245==  Uninitialised value was created by a stack allocation
==29245==    at 0xA98310: node::crypto::Hash::New(v8::FunctionCallbackInfo<v8::Value> const&) (in /home/james/node/node/out/Release/node)

After digging in, I've tracked down the above warning to the HashInit call inside Hash::New ... But, here's the fun part, adding any printf inside HashInit makes the initialized jump/move disappear.

This depends on the recent semver-major crypto refactor but can be backported.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Merge request reports