From 72e3ba6b5456b0f7252ac003ead8eeac362a9cf7 Mon Sep 17 00:00:00 2001 From: Ben Clayton Date: Thu, 30 Sep 2021 18:04:01 +0000 Subject: [PATCH] dawn_node: Fix macOS build Generate exported node symbol stubs with weak linking. This keeps the linker happy, and these are replaced by the real node symbols at runtime. Fix clang warnings. Have WGPUBufferMapAsyncStatus_DestroyedBeforeCallback reject the promise with an AbortError. Bug: dawn:1123 Change-Id: I503f889b027b6cfc0e458abf434d4888990fb67b Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/65560 Reviewed-by: Austin Eng Reviewed-by: Antonio Maiorano Reviewed-by: Corentin Wallez Commit-Queue: Ben Clayton --- CMakeLists.txt | 2 +- src/dawn_node/CMakeLists.txt | 22 ++++++++++++++---- src/dawn_node/NapiSymbols.cpp | 34 ++++++++++++++++++++++++++++ src/dawn_node/binding/CMakeLists.txt | 8 +++---- src/dawn_node/binding/GPUBuffer.cpp | 15 +++++++----- src/dawn_node/binding/GPUDevice.cpp | 4 ++++ src/dawn_node/interop/CMakeLists.txt | 8 +++---- src/dawn_node/utils/Debug.h | 32 +++++++++++++++----------- 8 files changed, 92 insertions(+), 33 deletions(-) create mode 100644 src/dawn_node/NapiSymbols.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 5aa4e67ff1..bbdf7c8735 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -135,7 +135,7 @@ set_if_not_defined(DAWN_TINT_DIR "${DAWN_THIRD_PARTY_DIR}/tint" "Directory in wh # Dependencies for DAWN_BUILD_NODE_BINDINGS set_if_not_defined(NODE_ADDON_API_DIR "${DAWN_THIRD_PARTY_DIR}/node-addon-api" "Directory in which to find node-addon-api") -set_if_not_defined(NODE_API_HEADERS_DIR "${DAWN_THIRD_PARTY_DIR}/node-api-headers/include" "Directory in which to find node-api-headers") +set_if_not_defined(NODE_API_HEADERS_DIR "${DAWN_THIRD_PARTY_DIR}/node-api-headers" "Directory in which to find node-api-headers") set_if_not_defined(WEBGPU_IDL_PATH "${DAWN_THIRD_PARTY_DIR}/gpuweb/webgpu.idl" "Path to the webgpu.idl definition file") # Much of the backend code is shared among desktop OpenGL and OpenGL ES diff --git a/src/dawn_node/CMakeLists.txt b/src/dawn_node/CMakeLists.txt index b4f3ae4215..dd1e2e2ee9 100644 --- a/src/dawn_node/CMakeLists.txt +++ b/src/dawn_node/CMakeLists.txt @@ -55,10 +55,22 @@ function(idlgen) ) endfunction() +# Generate the NapiSymbols.h file from the ${NODE_API_HEADERS_DIR}/symbols.js symbol list +set(NAPI_SYMBOLS_H "${GEN_DIR}/NapiSymbols.h") +file(READ "${NODE_API_HEADERS_DIR}/symbols.js" NAPI_SYMBOLS_JS_CONTENT) +string(REGEX MATCHALL "napi_[a-z0-9_]*" NAPI_SYMBOLS "${NAPI_SYMBOLS_JS_CONTENT}") +list(TRANSFORM NAPI_SYMBOLS PREPEND "NAPI_SYMBOL(") +list(TRANSFORM NAPI_SYMBOLS APPEND ")\n") +string(REPLACE ";" "" NAPI_SYMBOLS "${NAPI_SYMBOLS}") +file(GENERATE OUTPUT "${NAPI_SYMBOLS_H}" CONTENT "${NAPI_SYMBOLS}") + add_subdirectory(binding) add_subdirectory(interop) -add_library(dawn_node SHARED "Module.cpp") +add_library(dawn_node SHARED + "Module.cpp" + "NapiSymbols.cpp" +) set_target_properties(dawn_node PROPERTIES PREFIX "" OUTPUT_NAME "dawn" @@ -67,10 +79,10 @@ set_target_properties(dawn_node PROPERTIES ) target_link_libraries(dawn_node dawn_node_binding dawn_node_interop dawn_native dawncpp dawn_proc) target_include_directories(dawn_node PRIVATE - ${CMAKE_SOURCE_DIR} - ${NODE_API_HEADERS_DIR} - ${NODE_ADDON_API_DIR} - ${GEN_DIR} + "${CMAKE_SOURCE_DIR}" + "${NODE_API_HEADERS_DIR}/include" + "${NODE_ADDON_API_DIR}" + "${GEN_DIR}" ) # dawn_node targets require C++17 diff --git a/src/dawn_node/NapiSymbols.cpp b/src/dawn_node/NapiSymbols.cpp new file mode 100644 index 0000000000..6bfce0bcd4 --- /dev/null +++ b/src/dawn_node/NapiSymbols.cpp @@ -0,0 +1,34 @@ +// Copyright 2021 The Dawn Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "src/dawn_node/utils/Debug.h" + +// To reduce the build dependencies for compiling the dawn.node targets, we do +// not use cmake-js for building, but instead just depend on node_api_headers. +// As the name suggests, node_api_headers contains just the *headers* of the +// Napi, and does not provide a library to link against. Fortunately +// node_api_headers provides a list of symbols exported by Node, which we can +// stub here to keep the linker happy. + +#define NAPI_SYMBOL(NAME) \ + __attribute__((weak)) void NAME() { \ + UNREACHABLE( \ + "#NAME is a weak stub, and should have been runtime replaced by the node " \ + "implementation"); \ + } + +extern "C" { +// List of Napi symbols generated from the node_api_headers/symbols.js file +#include "NapiSymbols.h" +} diff --git a/src/dawn_node/binding/CMakeLists.txt b/src/dawn_node/binding/CMakeLists.txt index b160644cf8..554459d7aa 100644 --- a/src/dawn_node/binding/CMakeLists.txt +++ b/src/dawn_node/binding/CMakeLists.txt @@ -67,10 +67,10 @@ add_library(dawn_node_binding STATIC target_include_directories(dawn_node_binding PRIVATE - ${CMAKE_SOURCE_DIR} - ${NODE_API_HEADERS_DIR} - ${NODE_ADDON_API_DIR} - ${GEN_DIR} + "${CMAKE_SOURCE_DIR}" + "${NODE_API_HEADERS_DIR}/include" + "${NODE_ADDON_API_DIR}" + "${GEN_DIR}" ) target_link_libraries(dawn_node_binding diff --git a/src/dawn_node/binding/GPUBuffer.cpp b/src/dawn_node/binding/GPUBuffer.cpp index 7b74bebbbb..9921b7280a 100644 --- a/src/dawn_node/binding/GPUBuffer.cpp +++ b/src/dawn_node/binding/GPUBuffer.cpp @@ -79,19 +79,22 @@ namespace wgpu { namespace binding { c->state = State::Unmapped; switch (status) { - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_Success: + case WGPUBufferMapAsyncStatus_Force32: + UNREACHABLE("WGPUBufferMapAsyncStatus_Force32"); + break; + case WGPUBufferMapAsyncStatus_Success: c->promise.Resolve(); c->state = State::Mapped; break; - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_Error: + case WGPUBufferMapAsyncStatus_Error: c->promise.Reject(Errors::OperationError(c->env)); break; - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_UnmappedBeforeCallback: + case WGPUBufferMapAsyncStatus_UnmappedBeforeCallback: + case WGPUBufferMapAsyncStatus_DestroyedBeforeCallback: c->promise.Reject(Errors::AbortError(c->env)); break; - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_Unknown: - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_DeviceLost: - case WGPUBufferMapAsyncStatus::WGPUBufferMapAsyncStatus_DestroyedBeforeCallback: + case WGPUBufferMapAsyncStatus_Unknown: + case WGPUBufferMapAsyncStatus_DeviceLost: // TODO: The spec is a bit vague around what the promise should do // here. c->promise.Reject(Errors::UnknownError(c->env)); diff --git a/src/dawn_node/binding/GPUDevice.cpp b/src/dawn_node/binding/GPUDevice.cpp index 5d40b2687b..1e902eb23d 100644 --- a/src/dawn_node/binding/GPUDevice.cpp +++ b/src/dawn_node/binding/GPUDevice.cpp @@ -92,7 +92,11 @@ namespace wgpu { namespace binding { [](WGPUDeviceLostReason reason, char const* message, void* userdata) { auto r = interop::GPUDeviceLostReason::kDestroyed; switch (reason) { + case WGPUDeviceLostReason_Force32: + UNREACHABLE("WGPUDeviceLostReason_Force32"); + break; case WGPUDeviceLostReason_Destroyed: + case WGPUDeviceLostReason_Undefined: r = interop::GPUDeviceLostReason::kDestroyed; break; } diff --git a/src/dawn_node/interop/CMakeLists.txt b/src/dawn_node/interop/CMakeLists.txt index 321f817929..0b84c0ab97 100644 --- a/src/dawn_node/interop/CMakeLists.txt +++ b/src/dawn_node/interop/CMakeLists.txt @@ -50,10 +50,10 @@ add_library(dawn_node_interop STATIC target_include_directories(dawn_node_interop PRIVATE - ${CMAKE_SOURCE_DIR} - ${NODE_API_HEADERS_DIR} - ${NODE_ADDON_API_DIR} - ${GEN_DIR} + "${CMAKE_SOURCE_DIR}" + "${NODE_API_HEADERS_DIR}/include" + "${NODE_ADDON_API_DIR}" + "${GEN_DIR}" ) target_link_libraries(dawn_node_interop diff --git a/src/dawn_node/utils/Debug.h b/src/dawn_node/utils/Debug.h index 96c19863cd..3873540981 100644 --- a/src/dawn_node/utils/Debug.h +++ b/src/dawn_node/utils/Debug.h @@ -101,19 +101,18 @@ namespace wgpu { namespace utils { return out; } - // Unimplemented() prints an 'UNIMPLEMENTED' message to stdout with the given - // file, line, function and optional message, then calls abort(). - // Unimplemented() is usually not called directly, but by the UNIMPLEMENTED() - // macro below. + // Fatal() prints a message to stdout with the given file, line, function and optional message, + // then calls abort(). Fatal() is usually not called directly, but by the UNREACHABLE() and + // UNIMPLEMENTED() macro below. template - [[noreturn]] inline void Unimplemented(const char* file, - int line, - const char* function, - MSG_ARGS&&... msg_args) { + [[noreturn]] inline void Fatal(const char* reason, + const char* file, + int line, + const char* function, + MSG_ARGS&&... msg_args) { std::stringstream msg; - msg << file << ":" << line << ": " - << "UNIMPLEMENTED: " << function << "()"; - if constexpr (sizeof...(msg_args)) { + msg << file << ":" << line << ": " << reason << ": " << function << "()"; + if constexpr (sizeof...(msg_args) > 0) { msg << " "; Write(msg, std::forward(msg_args)...); } @@ -130,10 +129,17 @@ namespace wgpu { namespace utils { // UNIMPLEMENTED() prints 'UNIMPLEMENTED' with the current file, line and // function to stdout, along with the optional message, then calls abort(). -// The macro calls Unimplemented(), which is annotated with [[noreturn]]. +// The macro calls Fatal(), which is annotated with [[noreturn]]. // Used to stub code that has not yet been implemented. #define UNIMPLEMENTED(...) \ - ::wgpu::utils::Unimplemented(__FILE__, __LINE__, __FUNCTION__, ##__VA_ARGS__) + ::wgpu::utils::Fatal("UNIMPLEMENTED", __FILE__, __LINE__, __FUNCTION__, ##__VA_ARGS__) + +// UNREACHABLE() prints 'UNREACHABLE' with the current file, line and +// function to stdout, along with the optional message, then calls abort(). +// The macro calls Fatal(), which is annotated with [[noreturn]]. +// Used to stub code that has not yet been implemented. +#define UNREACHABLE(...) \ + ::wgpu::utils::Fatal("UNREACHABLE", __FILE__, __LINE__, __FUNCTION__, ##__VA_ARGS__) }} // namespace wgpu::utils