From a22991b04461a1aa8dd6e1793f26f3b9e6eaaa27 Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Fri, 1 May 2020 07:06:39 -0400 Subject: [PATCH 1/2] Revert "src: add aliased-buffer-overflow abort test" This reverts commit babeb58c5286c55f00e6dfc48c0b609314bcc381. PR-URL: https://github.com/nodejs/node/pull/33196 Refs: https://github.com/nodejs/node/pull/31740 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- test/abort/common.gypi | 8 ------ .../binding.cc | 23 --------------- .../binding.gyp | 9 ------ .../test-abort-aliased-buffer-overflow.js | 28 ------------------- 4 files changed, 68 deletions(-) delete mode 100644 test/abort/common.gypi delete mode 100644 test/abort/test_abort-aliased-buffer-overflow/binding.cc delete mode 100644 test/abort/test_abort-aliased-buffer-overflow/binding.gyp delete mode 100644 test/abort/test_abort-aliased-buffer-overflow/test-abort-aliased-buffer-overflow.js diff --git a/test/abort/common.gypi b/test/abort/common.gypi deleted file mode 100644 index 19396c61856af3..00000000000000 --- a/test/abort/common.gypi +++ /dev/null @@ -1,8 +0,0 @@ -{ - 'defines': [ 'V8_DEPRECATION_WARNINGS=1', 'NODE_WANT_INTERNALS=1' ], - 'conditions': [ - [ 'OS in "linux freebsd openbsd solaris android aix cloudabi"', { - 'cflags': ['-Wno-cast-function-type'], - }], - ], -} diff --git a/test/abort/test_abort-aliased-buffer-overflow/binding.cc b/test/abort/test_abort-aliased-buffer-overflow/binding.cc deleted file mode 100644 index c3bf66061bf0ee..00000000000000 --- a/test/abort/test_abort-aliased-buffer-overflow/binding.cc +++ /dev/null @@ -1,23 +0,0 @@ -#include -#include -#include - -#include -#include - -void AllocateAndResizeBuffer( - const v8::FunctionCallbackInfo& args) { - v8::Isolate* isolate = args.GetIsolate(); - int64_t length = args[0].As()->Int64Value(); - - node::AliasedBigUint64Array array{isolate, 0}; - - array.reserve(length); - assert(false); - } - -void init(v8::Local exports) { - NODE_SET_METHOD(exports, - "allocateAndResizeBuffer", - AllocateAndResizeBuffer); -} diff --git a/test/abort/test_abort-aliased-buffer-overflow/binding.gyp b/test/abort/test_abort-aliased-buffer-overflow/binding.gyp deleted file mode 100644 index 55fbe7050f18e4..00000000000000 --- a/test/abort/test_abort-aliased-buffer-overflow/binding.gyp +++ /dev/null @@ -1,9 +0,0 @@ -{ - 'targets': [ - { - 'target_name': 'binding', - 'sources': [ 'binding.cc' ], - 'includes': ['../common.gypi'], - } - ] -} diff --git a/test/abort/test_abort-aliased-buffer-overflow/test-abort-aliased-buffer-overflow.js b/test/abort/test_abort-aliased-buffer-overflow/test-abort-aliased-buffer-overflow.js deleted file mode 100644 index 33cd21295848b9..00000000000000 --- a/test/abort/test_abort-aliased-buffer-overflow/test-abort-aliased-buffer-overflow.js +++ /dev/null @@ -1,28 +0,0 @@ -'use strict'; -const common = require('../common'); -const assert = require('assert'); -const cp = require('child_process'); - -// This test ensures that during resizing of an Aliased*Array the computation -// of the new size does not overflow. - -if (process.argv[2] === 'child') { - // test - const binding = require(`./build/${common.buildType}/binding`); - - const bigValue = BigInt('0xE000 0000 E000 0000'); - binding.AllocateAndResizeBuffer(bigValue); - assert.fail('this should be unreachable'); -} else { - // observer - const child = cp.spawn(`${process.execPath}`, [`${__filename}`, 'child']); - child.on('exit', common.mustCall(function(code, signal) { - if (common.isWindows) { - assert.strictEqual(code, 134); - assert.strictEqual(signal, null); - } else { - assert.strictEqual(code, null); - assert.strictEqual(signal, 'SIGABRT'); - } - })); -} From 120d7076d5462f7f664ed0c92be26541e3298a24 Mon Sep 17 00:00:00 2001 From: Richard Lau Date: Fri, 1 May 2020 07:07:15 -0400 Subject: [PATCH 2/2] Revert "src: add test/abort build tasks" This reverts commit e08ac095690015a8c488e05251e05feb2acc57a0. PR-URL: https://github.com/nodejs/node/pull/33196 Refs: https://github.com/nodejs/node/pull/31740 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen Reviewed-By: Ruben Bridgewater --- Makefile | 53 ++++++----------------------------------------------- vcbuild.bat | 30 ++++-------------------------- 2 files changed, 10 insertions(+), 73 deletions(-) diff --git a/Makefile b/Makefile index 21794191d8b32c..43d23459284609 100644 --- a/Makefile +++ b/Makefile @@ -295,7 +295,7 @@ v8: tools/make-v8.sh $(V8_ARCH).$(BUILDTYPE_LOWER) $(V8_BUILD_OPTIONS) .PHONY: jstest -jstest: build-addons build-abort-tests build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests +jstest: build-addons build-js-native-api-tests build-node-api-tests ## Runs addon tests and JS tests $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \ --skip-tests=$(CI_SKIP_TESTS) \ $(CI_JS_SUITES) \ @@ -319,7 +319,6 @@ test: all ## Runs default tests, linters, and builds docs. $(MAKE) -s tooltest $(MAKE) -s test-doc $(MAKE) -s build-addons - $(MAKE) -s build-abort-tests $(MAKE) -s build-js-native-api-tests $(MAKE) -s build-node-api-tests $(MAKE) -s cctest @@ -328,7 +327,6 @@ test: all ## Runs default tests, linters, and builds docs. .PHONY: test-only test-only: all ## For a quick test, does not run linter or build docs. $(MAKE) build-addons - $(MAKE) build-abort-tests $(MAKE) build-js-native-api-tests $(MAKE) build-node-api-tests $(MAKE) cctest @@ -338,7 +336,6 @@ test-only: all ## For a quick test, does not run linter or build docs. # Used by `make coverage-test` test-cov: all $(MAKE) build-addons - $(MAKE) build-abort-tests $(MAKE) build-js-native-api-tests $(MAKE) build-node-api-tests $(MAKE) cctest @@ -458,31 +455,6 @@ test/node-api/.buildstamp: $(ADDONS_PREREQS) \ # TODO(bnoordhuis) Force rebuild after gyp or node-gyp update. build-node-api-tests: | $(NODE_EXE) test/node-api/.buildstamp -ABORT_BINDING_GYPS := \ - $(filter-out test/abort/??_*/binding.gyp, \ - $(wildcard test/abort/*/binding.gyp)) - -ABORT_BINDING_SOURCES := \ - $(filter-out test/abort/??_*/*.c, $(wildcard test/abort/*/*.c)) \ - $(filter-out test/abort/??_*/*.cc, $(wildcard test/abort/*/*.cc)) \ - $(filter-out test/abort/??_*/*.h, $(wildcard test/abort/*/*.h)) - -# Implicitly depends on $(NODE_EXE), see the build-node-api-tests rule for rationale. -test/abort/.buildstamp: $(ADDONS_PREREQS) \ - $(ABORT_BINDING_GYPS) $(ABORT_BINDING_SOURCES) \ - src/node_api.h src/node_api_types.h src/js_native_api.h \ - src/js_native_api_types.h src/js_native_api_v8.h src/js_native_api_v8_internals.h - @$(call run_build_addons,"$$PWD/test/abort",$@) - -.PHONY: build-abort-tests -# .buildstamp needs $(NODE_EXE) but cannot depend on it -# directly because it calls make recursively. The parent make cannot know -# if the subprocess touched anything so it pessimistically assumes that -# .buildstamp is out of date and need a rebuild. -# Just goes to show that recursive make really is harmful... -# TODO(bnoordhuis) Force rebuild after gyp or node-gyp update. -build-abort-tests: | $(NODE_EXE) test/abort/.buildstamp - BENCHMARK_NAPI_BINDING_GYPS := $(wildcard benchmark/napi/*/binding.gyp) BENCHMARK_NAPI_BINDING_SOURCES := \ @@ -503,14 +475,12 @@ clear-stalled: echo $${PS_OUT} | xargs kill -9; \ fi -test-build: | all build-addons build-abort-tests build-js-native-api-tests build-node-api-tests +test-build: | all build-addons build-js-native-api-tests build-node-api-tests test-build-js-native-api: all build-js-native-api-tests test-build-node-api: all build-node-api-tests -test-build-abort: all build-abort-tests - .PHONY: test-all test-all: test-build ## Run default tests with both Debug and Release builds. $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=debug,release @@ -523,7 +493,7 @@ test-all-suites: | clear-stalled test-build bench-addons-build doc-only ## Run a $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test/* # CI_* variables should be kept synchronized with the ones in vcbuild.bat -CI_NATIVE_SUITES ?= addons js-native-api node-api abort +CI_NATIVE_SUITES ?= addons js-native-api node-api CI_JS_SUITES ?= default ifeq ($(node_use_openssl), false) CI_DOC := doctool @@ -535,7 +505,7 @@ endif # Build and test addons without building anything else # Related CI job: node-test-commit-arm-fanned test-ci-native: LOGLEVEL := info -test-ci-native: | test/addons/.buildstamp test/js-native-api/.buildstamp test/node-api/.buildstamp test/abort/.buildstamp +test-ci-native: | test/addons/.buildstamp test/js-native-api/.buildstamp test/node-api/.buildstamp $(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \ --mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \ $(TEST_CI_ARGS) $(CI_NATIVE_SUITES) @@ -557,7 +527,7 @@ test-ci-js: | clear-stalled .PHONY: test-ci # Related CI jobs: most CI tests, excluding node-test-commit-arm-fanned test-ci: LOGLEVEL := info -test-ci: | clear-stalled build-addons build-abort-tests build-js-native-api-tests build-node-api-tests doc-only +test-ci: | clear-stalled build-addons build-js-native-api-tests build-node-api-tests doc-only out/Release/cctest --gtest_output=xml:out/junit/cctest.xml $(PYTHON) tools/test.py $(PARALLEL_ARGS) -p tap --logfile test.tap \ --mode=$(BUILDTYPE_LOWER) --flaky-tests=$(FLAKY_TESTS) \ @@ -663,17 +633,8 @@ test-node-api-clean: $(RM) -r test/node-api/*/build $(RM) test/node-api/.buildstamp -.PHONY: test-abort -test-abort: test-build-abort - $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) test-abort - -.PHONY: test-abort-clean -test-abort-clean: - $(RM) -r test/abort/*/build - $(RM) test/abort/.buildstamp - .PHONY: test-addons -test-addons: test-build test-js-native-api test-node-api test-abort +test-addons: test-build test-js-native-api test-node-api $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) addons .PHONY: test-addons-clean @@ -683,7 +644,6 @@ test-addons-clean: $(RM) test/addons/.buildstamp test/addons/.docbuildstamp $(MAKE) test-js-native-api-clean $(MAKE) test-node-api-clean - $(MAKE) test-abort-clean test-async-hooks: $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) async-hooks @@ -692,7 +652,6 @@ test-with-async-hooks: $(MAKE) build-addons $(MAKE) build-js-native-api-tests $(MAKE) build-node-api-tests - $(MAKE) build-abort-tests $(MAKE) cctest NODE_TEST_WITH_ASYNC_HOOKS=1 $(PYTHON) tools/test.py $(PARALLEL_ARGS) --mode=$(BUILDTYPE_LOWER) \ $(CI_JS_SUITES) \ diff --git a/vcbuild.bat b/vcbuild.bat index 22c3e237acf570..0187e652bd7764 100644 --- a/vcbuild.bat +++ b/vcbuild.bat @@ -16,11 +16,11 @@ if /i "%1"=="/?" goto help cd %~dp0 @rem CI_* variables should be kept synchronized with the ones in Makefile -set CI_NATIVE_SUITES=addons js-native-api node-api abort +set CI_NATIVE_SUITES=addons js-native-api node-api set CI_JS_SUITES=default set CI_DOC=doctool @rem Same as the test-ci target in Makefile -set "common_test_suites=%CI_JS_SUITES% %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1&set build_aborts_tests=1" +set "common_test_suites=%CI_JS_SUITES% %CI_NATIVE_SUITES% %CI_DOC%&set build_addons=1&set build_js_native_api_tests=1&set build_node_api_tests=1" @rem Process arguments. set config=Release @@ -68,7 +68,6 @@ set openssl_no_asm= set doc= set extra_msbuild_args= set exit_code=0 -set build_aborts_tests= :next-arg if "%1"=="" goto args-done @@ -97,8 +96,6 @@ if /i "%1"=="test-ci-js" set test_args=%test_args% %test_ci_args% -J -p tap - if /i "%1"=="build-addons" set build_addons=1&goto arg-ok if /i "%1"=="build-js-native-api-tests" set build_js_native_api_tests=1&goto arg-ok if /i "%1"=="build-node-api-tests" set build_node_api_tests=1&goto arg-ok -if /i "%1"=="build-abort-tests" set build_abort_tests=1&goto arg-ok -if /i "%1"=="test-abort" set test_args=%test_args% abort&set build_abort_tests=1&goto arg-ok if /i "%1"=="test-addons" set test_args=%test_args% addons&set build_addons=1&goto arg-ok if /i "%1"=="test-js-native-api" set test_args=%test_args% js-native-api&set build_js_native_api_tests=1&goto arg-ok if /i "%1"=="test-node-api" set test_args=%test_args% node-api&set build_node_api_tests=1&goto arg-ok @@ -588,10 +585,10 @@ endlocal goto build-node-api-tests :build-node-api-tests -if not defined build_node_api_tests goto build-abort-tests +if not defined build_node_api_tests goto run-tests if not exist "%node_exe%" ( echo Failed to find node.exe - goto build-abort-tests + goto run-tests ) echo Building node-api :: clear @@ -604,25 +601,6 @@ set npm_config_nodedir=%~dp0 "%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\node-api" if errorlevel 1 exit /b 1 endlocal -goto build-abort-tests - -:build-abort-tests -if not defined build_abort_tests goto run-tests -if not exist "%node_exe%" ( - echo Failed to find node.exe - goto run-tests -) -echo Building abort -:: clear -for /d %%F in (test\abort\??_*) do ( - rd /s /q %%F -) -:: building abort -setlocal -set npm_config_nodedir=%~dp0 -"%node_exe%" "%~dp0tools\build-addons.js" "%~dp0deps\npm\node_modules\node-gyp\bin\node-gyp.js" "%~dp0test\abort" -if errorlevel 1 exit /b 1 -endlocal goto run-tests :run-tests