tickprocessor: pass proper arguments to /bin/sh

[indutny]

Checklist

• make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

tickprocessor

Description of change

/bin/sh -c trick wasn't working for several reasons:

• /bin/sh -c "..." expects the first argument after "..." to be a
  $0, not a $1. Previously -n wasn't passed to nm because of
  this, and many symbols were ordered improperly
• c++filt was applied not only to the names of the functions but to
  their nm prefixes like t and a (t xxx turns into
  unsigned char xxx).

Instead of applying c++filt wide and using sh -c, execute nm as
requested by deps/v8/tools/tickprocessor.js and apply c++filt to all
matching entries manually.

Included test demonstrates where previous approach failed: all builtins
were merged into v8::internal::Builtins::~Builtins, because they were
prefixed by t in nm output.

R= @bnoordhuis

[indutny]

cc @nodejs/v8

[indutny]

Should be backported everywhere cc @thealphanerd

[indutny]

One more patch is needed to make this thing work properly on all systems, but I have to file it to V8 team first. Will link it here in a bit.

[indutny]

Ah, actually this patch should be for node only...

[indutny]

I'll push it to this branch too in a moment.

[indutny]

Force-pushed proper fix, PTAL

[indutny]

CI: https://ci.nodejs.org/job/node-test-pull-request/3998/

[indutny]

CI is green with unrelated crypto failures.

[Trott]

Is there any chance at all that this fixes the flakiness on nearly all platforms of the existing test-tick-processor test? (See #4427.)

I know nearly nothing about the V8 tick processor, so I'm asking from a point of near-complete ignorance.

[indutny]

@Trott unlikely, this affects only macs.

[indutny]

@bnoordhuis ping ;)

[indutny]

ping @nodejs/v8 too

[matthewloring]

LGTM

[indutny]

Thank you, landing.

[indutny]

Landed in 15d72c8.

[bnoordhuis]

Not a comment, just a note: this matches any hex digit string >= 8 && <= 16 characters long, not just strings of size 8 or 16. Ditto below.

[indutny]

It is a copy-paste from V8, but I agree with you. It doesn't matter that much in the end, though.

[bnoordhuis]

Belated LGTM.

[MylesBorins]

@indutny this lands cleanly on v4.x but blows up due to using let without use strict

If I replace all instances of let with var in the polyfill the code runs, but the test fails

=== release test-tick-processor === Path: parallel/test-tick-processor assert.js:85 throw new assert.AssertionError({^ AssertionError: /Builtin_DateNow/ not matching Statistical profiling result from isolate-0x101805000-v8.log, (1629 ticks, 10 unaccounted, 0 excluded). [Shared libraries]: ticks total nonlib name 66 4.1% /usr/lib/system/libsystem_pthread.dylib [JavaScript]: ticks total nonlib name 47 2.9% 3.0% LazyCompile: processImmediate timers.js:367:26 19 1.2% 1.2% LazyCompile: *_tickCallback node.js:352:27 18 1.1% 1.2% LazyCompile: *exports.setImmediate timers.js:420:32 11 0.7% 0.7% LazyCompile: *f [eval]:1:11 10 0.6% 0.6% Stub: JSEntryStub 10 0.6% 0.6% Stub: CEntryStub 9 0.6% 0.6% LazyCompile: ~<anonymous> [eval]:3:33 8 0.5% 0.5% Builtin: JSEntryTrampoline 7 0.4% 0.4% Builtin: JSConstructStubGeneric 6 0.4% 0.4% Stub: FastNewContextStub 6 0.4% 0.4% LazyCompile: *now native date.js:197:17 4 0.2% 0.3% Stub: FastNewClosureStub 4 0.2% 0.3% Builtin: ArgumentsAdaptorTrampoline 3 0.2% 0.2% LazyCompile: *init _linklist.js:3:14 3 0.2% 0.2% LazyCompile: *emitPendingUnhandledRejections node.js:510:44 2 0.1% 0.1% Stub: ToBooleanStub 2 0.1% 0.1% Stub: LoadICStub 2 0.1% 0.1% Stub: CallApiAccessorStub 2 0.1% 0.1% LazyCompile: ~emitPendingUnhandledRejections node.js:510:44 2 0.1% 0.1% LazyCompile: *isEmpty _linklist.js:54:17 1 0.1% 0.1% Stub: CompareICStub 1 0.1% 0.1% LazyCompile: *append _linklist.js:44:16 1 0.1% 0.1% Handler: An IC handler from the snapshot{1} 1 0.1% 0.1% Handler: An IC handler from the snapshot [C++]: ticks total nonlib name 475 29.2% 30.4% ___mkfifo_extended 146 9.0% 9.3% ___sigwait 42 2.6% 2.7% __simple_asl_msg_set 31 1.9% 2.0% v8::internal::Context::native_context() 27 1.7% 1.7% _vm_region_recurse_64 26 1.6% 1.7% v8::Context::Exit() 21 1.3% 1.3% v8::Function::Call(v8::Local<v8::Value>, int, v8::Local<v8::Value>*) 20 1.2% 1.3% v8::internal::NameDictionaryBase<v8::internal::NameDictionary, v8::internal::NameDictionaryShape>::FindEntry(v8::internal::Handle<v8::internal::Name>) 20 1.2% 1.3% ___vfprintf 19 1.2% 1.2% v8::internal::Runtime_CreateObjectLiteral(int, v8::internal::Object**, v8::internal::Isolate*) 18 1.1% 1.2% v8::internal::LookupIterator::LookupInHolder(v8::internal::Map*, v8::internal::JSReceiver*) 18 1.1% 1.2% _uv__io_poll 17 1.0% 1.1% _uv_run 16 1.0% 1.0% ___vfwscanf 15 0.9% 1.0% _uv__run_check 14 0.9% 0.9% node::MakeCallback(node::Environment*, v8::Local<v8::Value>, v8::Local<v8::Function>, int, v8::Local<v8::Value>*) 13 0.8% 0.8% v8::internal::Object::GetProperty(v8::internal::LookupIterator*, v8::internal::LanguageMode) 13 0.8% 0.8% v8::internal::LookupIterator::Next() 13 0.8% 0.8% node::CheckImmediate(uv_check_s*) 12 0.7% 0.8% v8::internal::JSObjectWalkVisitor<v8::internal::AllocationSiteUsageContext>::StructureWalk(v8::internal::Handle<v8::internal::JSObject>) 12 0.7% 0.8% node::Start(int, char**) 12 0.7% 0.8% node::ContextifyScript::New(v8::FunctionCallbackInfo<v8::Value> const&) 12 0.7% 0.8% _uv__hrtime 11 0.7% 0.7% v8::internal::Object::GetPropertyWithAccessor(v8::internal::LookupIterator*, v8::internal::LanguageMode) 11 0.7% 0.7% v8::internal::Heap::AllocateRaw(int, v8::internal::AllocationSpace, v8::internal::AllocationSpace, v8::internal::AllocationAlignment) 11 0.7% 0.7% node::NeedImmediateCallbackGetter(v8::Local<v8::String>, v8::PropertyCallbackInfo<v8::Value> const&) 10 0.6% 0.6% _tempnam 10 0.6% 0.6% __enlarge 9 0.6% 0.6% v8::internal::Invoke(bool, v8::internal::Handle<v8::internal::JSFunction>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*) 9 0.6% 0.6% v8::internal::Heap::CopyJSObject(v8::internal::JSObject*, v8::internal::AllocationSite*) 9 0.6% 0.6% v8::Function::Call(v8::Local<v8::Context>, v8::Local<v8::Value>, int, v8::Local<v8::Value>*) 9 0.6% 0.6% v8::Context::Enter() 8 0.5% 0.5% v8::internal::Runtime_DateCurrentTime(int, v8::internal::Object**, v8::internal::Isolate*) 8 0.5% 0.5% v8::internal::Runtime::GetObjectProperty(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, v8::internal::LanguageMode) 8 0.5% 0.5% v8::internal::Heap::AllocateHeapNumber(double, v8::internal::MutableMode, v8::internal::PretenureFlag) 8 0.5% 0.5% _catopen 8 0.5% 0.5% __simple_dprintf 8 0.5% 0.5% ___printf_comp 7 0.4% 0.4% v8::internal::LookupIterator::LookupIterator(v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Name>, v8::internal::LookupIterator::Configuration) 7 0.4% 0.4% v8::internal::Execution::Call(v8::internal::Isolate*, v8::internal::Handle<v8::internal::Object>, v8::internal::Handle<v8::internal::Object>, int, v8::internal::Handle<v8::internal::Object>*, bool) 7 0.4% 0.4% v8::base::TimeTicks::HighResolutionNow() 7 0.4% 0.4% v8::Object::Get(v8::Local<v8::Context>, v8::Local<v8::Value>) 7 0.4% 0.4% _vm_region_64 7 0.4% 0.4% _uv__run_idle 7 0.4% 0.4% _swscanf_l 7 0.4% 0.4% _getipv4sourcefilter 6 0.4% 0.4% v8::internal::Logger::CurrentTimeEvent() 6 0.4% 0.4% v8::internal::Factory::NewNumber(double, v8::internal::PretenureFlag) 6 0.4% 0.4% _putchar 5 0.3% 0.3% v8::internal::Log::MessageBuilder::WriteToLogFile() 5 0.3% 0.3% v8::HandleScope::Initialize(v8::Isolate*) 5 0.3% 0.3% ___fread 4 0.2% 0.3% v8::internal::ThreadId::GetCurrentThreadId() 4 0.2% 0.3% v8::internal::SaveContext::SaveContext(v8::internal::Isolate*) 4 0.2% 0.3% v8::base::OS::TimeCurrentMillis() 4 0.2% 0.3% v8::Value::IsObject() const 4 0.2% 0.3% v8::Object::Get(v8::Local<v8::Value>) 4 0.2% 0.3% v8::Isolate::GetCurrentContext() 4 0.2% 0.3% _mach_msg_server_once 3 0.2% 0.2% v8::internal::LookupIterator::GetRoot(v8::internal::Handle<v8::internal::Object>, v8::internal::Isolate*) 3 0.2% 0.2% v8::internal::Logger::ApiEntryCall(char const*) 3 0.2% 0.2% v8::internal::Factory::CopyJSObjectWithAllocationSite(v8::internal::Handle<v8::internal::JSObject>, v8::internal::Handle<v8::internal::AllocationSite>) 3 0.2% 0.2% _tmpfile 3 0.2% 0.2% _fscanf_l 3 0.2% 0.2% _freopen 2 0.1% 0.1% v8::internal::PerIsolateAssertScope<(v8::internal::PerIsolateAssertType)1, false>::IsAllowed(v8::internal::Isolate*) 2 0.1% 0.1% v8::internal::PerIsolateAssertScope<(v8::internal::PerIsolateAssertType)0, true>::IsAllowed(v8::internal::Isolate*) 2 0.1% 0.1% v8::internal::LookupIterator::IsIntegerIndexedExotic(v8::internal::JSReceiver*) 2 0.1% 0.1% v8::internal::LookupIterator::FetchValue() const 2 0.1% 0.1% v8::internal::Isolate::RunMicrotasks() 2 0.1% 0.1% v8::internal::Deserializer::ReadData(v8::internal::Object**, v8::internal::Object**, int, unsigned char*) 2 0.1% 0.1% v8::internal::Assembler::j(v8::internal::Condition, v8::internal::Label*, v8::internal::Label::Distance) 2 0.1% 0.1% v8::base::Time::ToJsTime() const 2 0.1% 0.1% v8::base::Time::Now() 2 0.1% 0.1% v8::base::Mutex::Unlock() 2 0.1% 0.1% v8::V8::GetEternal(v8::Isolate*, int) 2 0.1% 0.1% node::Binding(v8::FunctionCallbackInfo<v8::Value> const&) 2 0.1% 0.1% _uv_backend_timeout 2 0.1% 0.1% _uv__run_timers 2 0.1% 0.1% _swscanf 2 0.1% 0.1% _loadCat 2 0.1% 0.1% _flush_queues 2 0.1% 0.1% __platform_memmove$VARIANT$Merom 2 0.1% 0.1% __GBK_mbrtowc 1 0.1% 0.1% void v8::internal::Scanner::Advance<false, true>() 1 0.1% 0.1% v8::platform::DefaultPlatform::PumpMessageLoop(v8::Isolate*) 1 0.1% 0.1% v8::internal::Zone::New(unsigned long) 1 0.1% 0.1% v8::internal::Translation::StoreJSFrameFunction() 1 0.1% 0.1% v8::internal::StoreBuffer::Compact() 1 0.1% 0.1% v8::internal::StackGuard::HandleInterrupts() 1 0.1% 0.1% v8::internal::Scope::LookupLocal(v8::internal::AstRawString const*) 1 0.1% 0.1% v8::internal::Runtime_HasFastPackedElements(int, v8::internal::Object**, v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::Runtime_GetProperty(int, v8::internal::Object**, v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::RecordFunctionCompilation(v8::internal::Logger::LogEventsAndTags, v8::internal::CompilationInfo*, v8::internal::Handle<v8::internal::SharedFunctionInfo>) 1 0.1% 0.1% v8::internal::ParserBase<v8::internal::ParserTraits>::ParsePostfixExpression(v8::internal::ExpressionClassifier*, bool*) 1 0.1% 0.1% v8::internal::ParserBase<v8::internal::ParserTraits>::ExpectSemicolon(bool*) 1 0.1% 0.1% v8::internal::OptimizingCompileDispatcher::AddToOsrBuffer(v8::internal::OptimizedCompileJob*) 1 0.1% 0.1% v8::internal::Map::RawCopy(v8::internal::Handle<v8::internal::Map>, int) 1 0.1% 0.1% v8::internal::Map::CopyWithField(v8::internal::Handle<v8::internal::Map>, v8::internal::Handle<v8::internal::Name>, v8::internal::Handle<v8::internal::TypeImpl<v8::internal::HeapTypeConfig> >, PropertyAttributes, v8::internal::Representation, v8::internal::TransitionFlag) 1 0.1% 0.1% v8::internal::Log::MessageBuilder::MessageBuilder(v8::internal::Log*) 1 0.1% 0.1% v8::internal::Log::MessageBuilder::Append(char const*, ...) 1 0.1% 0.1% v8::internal::LCodeGen::GenerateDeferredCode() 1 0.1% 0.1% v8::internal::LCodeGen::AddToTranslation(v8::internal::LEnvironment*, v8::internal::Translation*, v8::internal::LOperand*, bool, bool, int*, int*) 1 0.1% 0.1% v8::internal::LAllocator::ProcessInstructions(v8::internal::HBasicBlock*, v8::internal::BitVector*) 1 0.1% 0.1% v8::internal::LAllocator::AllocateRegisters() 1 0.1% 0.1% v8::internal::JSObject::DefineOwnPropertyIgnoreAttributes(v8::internal::LookupIterator*, v8::internal::Handle<v8::internal::Object>, PropertyAttributes, v8::internal::JSObject::ExecutableAccessorInfoHandling) 1 0.1% 0.1% v8::internal::Isolate::use_crankshaft() const 1 0.1% 0.1% v8::internal::Isolate::FireCallCompletedCallback() 1 0.1% 0.1% v8::internal::InnerPointerToCodeCache::GetCacheEntry(unsigned char*) 1 0.1% 0.1% v8::internal::HandleScope::DeleteExtensions(v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::HGraph::Optimize(v8::internal::BailoutReason*) 1 0.1% 0.1% v8::internal::HEnvironmentLivenessAnalysisPhase::Run() 1 0.1% 0.1% v8::internal::HCanonicalizePhase::Run() 1 0.1% 0.1% v8::internal::FeedbackNexus::ConfigurePremonomorphic() 1 0.1% 0.1% v8::internal::Factory::NewCodeRaw(int, bool) 1 0.1% 0.1% v8::internal::CodePatcher::CodePatcher(unsigned char*, int) 1 0.1% 0.1% v8::internal::CodeCache::LookupDefaultCache(v8::internal::Name*, unsigned int) 1 0.1% 0.1% v8::internal::Code::CopyFrom(v8::internal::CodeDesc const&) 1 0.1% 0.1% v8::internal::Assembler::immediate_arithmetic_op(unsigned char, v8::internal::Register, v8::internal::Immediate, int) 1 0.1% 0.1% v8::internal::Assembler::bind_to(v8::internal::Label*, int) 1 0.1% 0.1% v8::internal::AllocationSiteUsageContext::ShouldCreateMemento(v8::internal::Handle<v8::internal::JSObject>) 1 0.1% 0.1% v8::base::Thread::GetThreadLocal(int) 1 0.1% 0.1% v8::base::OS::VSNPrintF(char*, int, char const*, __va_list_tag*) 1 0.1% 0.1% v8::Value::IsFunction() const 1 0.1% 0.1% v8::ObjectTemplate::SetInternalFieldCount(int) 1 0.1% 0.1% v8::Locker::IsActive() 1 0.1% 0.1% v8::HandleScope::~HandleScope() 1 0.1% 0.1% node::IdleImmediateDummy(uv_idle_s*) 1 0.1% 0.1% _xprintf_malloc 1 0.1% 0.1% _uv__run_prepare 1 0.1% 0.1% _putc 1 0.1% 0.1% _fscanf 1 0.1% 0.1% __simple_esprintf 1 0.1% 0.1% ___part_load_locale 1 0.1% 0.1% ___mkdir_extended 1 0.1% 0.1% ___infl [Summary]: ticks total nonlib name 179 11.0% 11.5% JavaScript 1374 84.3% 87.9% C++ 7 0.4% 0.4% GC 66 4.1% Shared libraries 10 0.6% Unaccounted [C++ entry points]: ticks cpp total name 754 84.0% 46.3% v8::internal::Runtime_DateCurrentTime(int, v8::internal::Object**, v8::internal::Isolate*) 52 5.8% 3.2% v8::internal::Runtime_GetProperty(int, v8::internal::Object**, v8::internal::Isolate*) 44 4.9% 2.7% v8::internal::Runtime_CreateObjectLiteral(int, v8::internal::Object**, v8::internal::Isolate*) 12 1.3% 0.7% v8::internal::Builtin_HandleApiCallConstruct(int, v8::internal::Object**, v8::internal::Isolate*) 8 0.9% 0.5% v8::internal::Runtime_CompileLazy(int, v8::internal::Object**, v8::internal::Isolate*) 5 0.6% 0.3% v8::internal::StoreIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) 5 0.6% 0.3% v8::internal::Runtime_Interrupt(int, v8::internal::Object**, v8::internal::Isolate*) 4 0.4% 0.2% v8::internal::KeyedStoreIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) 3 0.3% 0.2% v8::internal::Runtime_StackGuard(int, v8::internal::Object**, v8::internal::Isolate*) 3 0.3% 0.2% v8::internal::LoadIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) 2 0.2% 0.1% v8::internal::ToBooleanIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) 2 0.2% 0.1% v8::internal::Builtin_HandleApiCall(int, v8::internal::Object**, v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::Runtime_ParseJson(int, v8::internal::Object**, v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::Runtime_HasFastPackedElements(int, v8::internal::Object**, v8::internal::Isolate*) 1 0.1% 0.1% v8::internal::InnerPointerToCodeCache::GetCacheEntry(unsigned char*) 1 0.1% 0.1% v8::internal::CompareIC_Miss(int, v8::internal::Object**, v8::internal::Isolate*) [Bottom up (heavy) profile]: Note: percentage shows a share of a particular caller in the total amount of its parent calls. Callers occupying less than 2.0% are not shown. ticks parent name 475 29.2% ___mkfifo_extended 473 99.6% v8::internal::Runtime_DateCurrentTime(int, v8::internal::Object**, v8::internal::Isolate*) 473 100.0% LazyCompile: *now native date.js:197:17 473 100.0% LazyCompile: *f [eval]:1:11 473 100.0% LazyCompile: ~<anonymous> [eval]:3:33 473 100.0% LazyCompile: processImmediate timers.js:367:26 146 9.0% ___sigwait 66 4.1% /usr/lib/system/libsystem_pthread.dylib 46 69.7% v8::internal::Runtime_DateCurrentTime(int, v8::internal::Object**, v8::internal::Isolate*) 46 100.0% LazyCompile: *now native date.js:197:17 46 100.0% LazyCompile: *f [eval]:1:11 46 100.0% LazyCompile: ~<anonymous> [eval]:3:33 46 100.0% LazyCompile: processImmediate timers.js:367:26 47 2.9% LazyCompile: processImmediate timers.js:367:26 1 2.1% LazyCompile: processImmediate timers.js:367:26 42 2.6% __simple_asl_msg_set 30 71.4% v8::internal::Runtime_DateCurrentTime(int, v8::internal::Object**, v8::internal::Isolate*) 30 100.0% LazyCompile: *now native date.js:197:17 30 100.0% LazyCompile: *f [eval]:1:11 30 100.0% LazyCompile: ~<anonymous> [eval]:3:33 30 100.0% LazyCompile: processImmediate timers.js:367:26 3 7.1% v8::internal::Runtime_Interrupt(int, v8::internal::Object**, v8::internal::Isolate*) 2 66.7% LazyCompile: ~<anonymous> [eval]:3:33 2 100.0% LazyCompile: processImmediate timers.js:367:26 1 33.3% LazyCompile: processImmediate timers.js:367:26 at runTest (/Users/thealphanerd/code/node/v4.x/test/parallel/test-tick-processor.js:64:3) at Object.<anonymous> (/Users/thealphanerd/code/node/v4.x/test/parallel/test-tick-processor.js:46:1) at Module._compile (module.js:409:26) at Object.Module._extensions..js (module.js:416:10) at Module.load (module.js:343:32) at Function.Module._load (module.js:300:12) at Function.Module.runMain (module.js:441:10) at startup (node.js:139:18) at node.js:990:3 Command: out/Release/node /Users/thealphanerd/code/node/v4.x/test/parallel/test-tick-processor.js 

Thoughts?

[targos]

I think the regex should be changed: Builtin_DateNow => Runtime_DateCurrentTime

[MylesBorins]

landed in v4.x as 54c38eb

changing the regex and let -> var got the tests passing.