Build error on SmartOS

[targos]

https://ci.nodejs.org/job/node-test-commit-smartos/45420/nodes=smartos20-64/console

10:51:08 In file included from ../deps/v8/src/base/free_deleter.h:15,
10:51:08                  from ../deps/v8/src/base/debug/stack_trace_posix.cc:41:
10:51:08 ../deps/v8/src/base/platform/memory.h: In function 'std::size_t v8::base::MallocUsableSize(void*)':
10:51:08 ../deps/v8/src/base/platform/memory.h:118:10: error: 'malloc_usable_size' was not declared in this scope; did you mean 'MallocUsableSize'?
10:51:08   118 |   return malloc_usable_size(ptr);
10:51:08       |          ^~~~~~~~~~~~~~~~~~
10:51:08       |          MallocUsableSize
10:51:08 make[2]: *** [tools/v8_gypfiles/v8_libbase.target.mk:177: /home/iojs/build/workspace/node-test-commit-smartos/nodes/smartos20-64/out/Release/obj.target/v8_libbase/deps/v8/src/base/debug/stack_trace_posix.o] Error 1

@nodejs/platform-smartos

[bahamat]

My team will be taking a look at this.

[jperkin]

Hi @targos, I'm struggling to find where this is coming from - GitHub search across the entire nodejs and v8 organisations show no code matches for malloc_usable_size(). It's not something we have in SmartOS, and many other platforms don't have it either, so the fix should be pretty straight-forward, I just can't figure out where it's actually being used.

I'd be more than happy to submit a fix for it if you could help me get up to speed on which repositories that test job is pulling from. Thanks!

[targos]

In the V8 repo: https://github.com/v8/v8/blob/50504b168d2c364d49e3332cbd934dc60a4bfad8/src/base/platform/memory.h#L117-L118

Introduced by https://chromium-review.googlesource.com/c/v8/v8/+/3858226

In this repo (canary is updated every day so the commit is ephemeral):

node-v8/deps/v8/src/base/platform/memory.h

Lines 117 to 118 in 4e97ad7

	#else // POSIX.
	return malloc_usable_size(ptr);

[jperkin]

Ah, ok, thanks! Looks like GitHub search just completely doesn't work.

The fix looks straight-forward (add V8_OS_SOLARIS to the exclusion list for V8_HAS_MALLOC_USABLE_SIZE on line 26, similar to AIX) but I assume I'll need to get this through the v8 contribution process first.

[targos]

The fix looks straight-forward (add V8_OS_SOLARIS to the exclusion list for V8_HAS_MALLOC_USABLE_SIZE on line 26, similar to AIX) but I assume I'll need to get this through the v8 contribution process first.

I'm floating a patch for now in nodejs/node#44741 but let's keep this issue open until it's upstreamed.

[bnoordhuis]

@bahamat the patch still needs to be upstreamed, see nodejs/node#45118.

[bahamat]

@bnoordhuis Ok, I'm looking into the CLA requirement. It's not clear to me that MNX is the most appropriate agent to contribute the patch, since we are not the author/copyright holder.

[bnoordhuis]

@targos That means you'd have to upstream it?

[targos]

https://chromium-review.googlesource.com/c/v8/v8/+/4026402