From ae85676d44baee3d12168a5c929347b3836f2cf2 Mon Sep 17 00:00:00 2001 From: Nick Terrell Date: Thu, 17 Dec 2020 14:27:53 -0800 Subject: [PATCH 1/5] Fix alignment of scratchBuffer in HUF_compressWeights() The scratch buffer must be 4-byte aligned. This causes test failures in 32-bit systems, where the stack isn't aligned. Fixes Issue #2428. --- lib/common/fse.h | 5 +++-- lib/compress/huf_compress.c | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/common/fse.h b/lib/common/fse.h index 83a07847a..dd5fc44e8 100644 --- a/lib/common/fse.h +++ b/lib/common/fse.h @@ -335,9 +335,10 @@ size_t FSE_buildCTable_rle (FSE_CTable* ct, unsigned char symbolValue); /* FSE_buildCTable_wksp() : * Same as FSE_buildCTable(), but using an externally allocated scratch buffer (`workSpace`). - * `wkspSize` must be >= `FSE_BUILD_CTABLE_WORKSPACE_SIZE(maxSymbolValue, tableLog)`. + * `wkspSize` must be >= `FSE_BUILD_CTABLE_WORKSPACE_SIZE_U32(maxSymbolValue, tableLog)` of `unsigned`. */ -#define FSE_BUILD_CTABLE_WORKSPACE_SIZE(maxSymbolValue, tableLog) (sizeof(unsigned) * (maxSymbolValue + 2) + (1ull << tableLog)) +#define FSE_BUILD_CTABLE_WORKSPACE_SIZE_U32(maxSymbolValue, tableLog) (maxSymbolValue + 2 + (1ull << (tableLog - 2))) +#define FSE_BUILD_CTABLE_WORKSPACE_SIZE(maxSymbolValue, tableLog) (sizeof(unsigned) * FSE_BUILD_CTABLE_WORKSPACE_SIZE_U32(maxSymbolValue, tableLog)) size_t FSE_buildCTable_wksp(FSE_CTable* ct, const short* normalizedCounter, unsigned maxSymbolValue, unsigned tableLog, void* workSpace, size_t wkspSize); #define FSE_BUILD_DTABLE_WKSP_SIZE(maxTableLog, maxSymbolValue) (sizeof(short) * (maxSymbolValue + 1) + (1ULL << maxTableLog) + 8) diff --git a/lib/compress/huf_compress.c b/lib/compress/huf_compress.c index abbcc3192..00c593d7e 100644 --- a/lib/compress/huf_compress.c +++ b/lib/compress/huf_compress.c @@ -69,7 +69,7 @@ static size_t HUF_compressWeights (void* dst, size_t dstSize, const void* weight U32 tableLog = MAX_FSE_TABLELOG_FOR_HUFF_HEADER; FSE_CTable CTable[FSE_CTABLE_SIZE_U32(MAX_FSE_TABLELOG_FOR_HUFF_HEADER, HUF_TABLELOG_MAX)]; - BYTE scratchBuffer[FSE_BUILD_CTABLE_WORKSPACE_SIZE(HUF_TABLELOG_MAX, MAX_FSE_TABLELOG_FOR_HUFF_HEADER)]; + U32 scratchBuffer[FSE_BUILD_CTABLE_WORKSPACE_SIZE_U32(HUF_TABLELOG_MAX, MAX_FSE_TABLELOG_FOR_HUFF_HEADER)]; unsigned count[HUF_TABLELOG_MAX+1]; S16 norm[HUF_TABLELOG_MAX+1]; From 4680d817c0522c57ad905aadef1bb7e1b59bf0f5 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 17 Dec 2020 14:53:36 -0800 Subject: [PATCH 2/5] added a simple runtime test in CI for 32-bit binaries --- .github/workflows/generic-dev.yml | 14 ++++++++++---- .github/workflows/generic-release.yml | 4 ++-- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/.github/workflows/generic-dev.yml b/.github/workflows/generic-dev.yml index bb88de57c..3f564878b 100644 --- a/.github/workflows/generic-dev.yml +++ b/.github/workflows/generic-dev.yml @@ -2,14 +2,13 @@ name: generic-dev on: pull_request: - branches: [ dev, master, actionsTest ] + branches: [ dev, release, actionsTest ] jobs: # Dev PR jobs that still have to be migrated from travis # -# icc (need self-hosted) -# versionTag +# versionTag (only on release tags) # valgrindTest (keeps failing for some reason. need investigation) # staticAnalyze (need trusty so need self-hosted) # pcc-fuzz: (need trusty so need self-hosted) @@ -19,7 +18,7 @@ jobs: # I need admins permissions to the repo for that it looks like # So I'm tabling that for now # -# The master branch exclusive jobs will be in a separate +# The release branch exclusive jobs will be in a separate # workflow file (the osx tests and meson build that is) benchmarking: @@ -36,6 +35,13 @@ jobs: - name: make test run: make test + check-32bit: # designed to catch https://github.com/facebook/zstd/issues/2428 + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - name: make check on 32-bit + run: CFLAGS="-m32 -O1 -fstack-protector" make test V=1 + gcc-6-7-libzstd: runs-on: ubuntu-latest steps: diff --git a/.github/workflows/generic-release.yml b/.github/workflows/generic-release.yml index de4a1cb91..a41f80f3e 100644 --- a/.github/workflows/generic-release.yml +++ b/.github/workflows/generic-release.yml @@ -2,10 +2,10 @@ name: generic-release on: pull_request: - # This will eventually only be for pushes to master + # This will eventually only be for pushes to release # but for dogfooding purposes, I'm running it even # on dev pushes - branches: [ dev, master, actionsTest ] + branches: [ dev, release, actionsTest ] jobs: # missing jobs From c11db9c8b5dcf2bd03f507304f8f081cb9afe4cb Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 17 Dec 2020 15:01:04 -0800 Subject: [PATCH 3/5] additional master->release switches (CI tests) --- .circleci/config.yml | 3 ++- .github/workflows/linux-kernel.yml | 2 +- .travis.yml | 23 ++++++++++++----------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index bcf2e1d5e..9738c1bd4 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -143,8 +143,9 @@ workflows: filters: branches: only: - - master + - release - dev + - master jobs: # Run daily long regression tests - regression-test diff --git a/.github/workflows/linux-kernel.yml b/.github/workflows/linux-kernel.yml index 35871ff08..124f77778 100644 --- a/.github/workflows/linux-kernel.yml +++ b/.github/workflows/linux-kernel.yml @@ -2,7 +2,7 @@ name: linux-kernel on: pull_request: - branches: [ dev, master, actionsTest ] + branches: [ dev, release, actionsTest ] jobs: test: diff --git a/.travis.yml b/.travis.yml index 226d4c015..ee473733c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -8,6 +8,7 @@ git: branches: only: - dev + - release - master - travisTest @@ -162,33 +163,33 @@ matrix: - make -C tests checkTag - tests/checkTag "$TRAVIS_BRANCH" - # tests for master branch and cron job only + # tests for release branch and cron job only - name: OS-X # ~13mn - if: branch = master + if: branch = release os: osx script: - make test - make -C lib all - name: zbuff test - if: branch = master + if: branch = release script: - make -C tests test-zbuff - name: Versions Compatibility Test # 11.5mn - if: branch = master + if: branch = release script: - make -C tests versionsTest - name: thread sanitizer # ~29mn - if: branch = master + if: branch = release script: - make clang38install - CC=clang-3.8 make tsan-test-zstream - CC=clang-3.8 make tsan-fuzztest - name: PPC64LE + Fuzz test # ~13mn - if: branch = master + if: branch = release arch: ppc64le script: - cat /proc/cpuinfo @@ -196,28 +197,28 @@ matrix: - name: Qemu PPC64 + Fuzz test # ~13mn, presumed Big-Endian (?) dist: trusty # note : PPC64 cross-compilation for Qemu tests seems broken on Xenial - if: branch = master + if: branch = release script: - make ppcinstall - make ppc64fuzz # note : we already have aarch64 tests on hardware - name: Qemu aarch64 + Fuzz Test (on Xenial) # ~14mn - if: branch = master + if: branch = release dist: xenial script: - make arminstall - make aarch64fuzz - name: zlib wrapper test # ~7.5mn - if: branch = master + if: branch = release script: - make gpp6install valgrindinstall - make -C zlibWrapper test - make -C zlibWrapper valgrindTest - name: LZ4, thread pool, and partial libs tests # ~4mn - if: branch = master + if: branch = release script: - make lz4install - make -C tests test-lz4 @@ -229,7 +230,7 @@ matrix: # meson dedicated test - name: Xenial (Meson + clang) # ~15mn - if: branch = master + if: branch = release dist: xenial language: cpp compiler: clang From d5eb7d156904e2c7ffbfc3fd71285ac1340792b2 Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 17 Dec 2020 15:05:26 -0800 Subject: [PATCH 4/5] added pre-requisites for 32-bit tests in CI --- .github/workflows/generic-dev.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/generic-dev.yml b/.github/workflows/generic-dev.yml index 3f564878b..4f0a677be 100644 --- a/.github/workflows/generic-dev.yml +++ b/.github/workflows/generic-dev.yml @@ -40,7 +40,9 @@ jobs: steps: - uses: actions/checkout@v2 - name: make check on 32-bit - run: CFLAGS="-m32 -O1 -fstack-protector" make test V=1 + run: | + make libc6install + CFLAGS="-m32 -O1 -fstack-protector" make test V=1 gcc-6-7-libzstd: runs-on: ubuntu-latest From 3536e9d5ff659c7804214aeed599f17cd2666e4c Mon Sep 17 00:00:00 2001 From: Yann Collet Date: Thu, 17 Dec 2020 15:44:54 -0800 Subject: [PATCH 5/5] removing tests using too much resources for 32-bit address space --- .github/workflows/generic-dev.yml | 2 +- tests/playTests.sh | 5 ----- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/.github/workflows/generic-dev.yml b/.github/workflows/generic-dev.yml index 4f0a677be..9abe98b73 100644 --- a/.github/workflows/generic-dev.yml +++ b/.github/workflows/generic-dev.yml @@ -42,7 +42,7 @@ jobs: - name: make check on 32-bit run: | make libc6install - CFLAGS="-m32 -O1 -fstack-protector" make test V=1 + CFLAGS="-m32 -O1 -fstack-protector" make check V=1 gcc-6-7-libzstd: runs-on: ubuntu-latest diff --git a/tests/playTests.sh b/tests/playTests.sh index 51b42b60a..4d6abbcc0 100755 --- a/tests/playTests.sh +++ b/tests/playTests.sh @@ -1342,8 +1342,6 @@ optCSize19=$(datagen -g2M | zstd -19 -c | wc -c) longCSize19=$(datagen -g2M | zstd -19 --long -c | wc -c) optCSize19wlog23=$(datagen -g2M | zstd -19 -c --zstd=wlog=23 | wc -c) longCSize19wlog23=$(datagen -g2M | zstd -19 -c --long=23 | wc -c) -optCSize22=$(datagen -g900K | zstd -22 --ultra -c | wc -c) -longCSize22=$(datagen -g900K | zstd -22 --ultra --long -c | wc -c) if [ "$longCSize16" -gt "$optCSize16" ]; then echo using --long on compression level 16 should not cause compressed size regression exit 1 @@ -1353,9 +1351,6 @@ elif [ "$longCSize19" -gt "$optCSize19" ]; then elif [ "$longCSize19wlog23" -gt "$optCSize19wlog23" ]; then echo using --long on compression level 19 with wLog=23 should not cause compressed size regression exit 1 -elif [ "$longCSize22" -gt "$optCSize22" ]; then - echo using --long on compression level 22 should not cause compressed size regression - exit 1 fi