zlib: disable CRC32 SIMD optimization - #49511

lpinca · 2023-09-06T10:31:01Z

It seems that the optimization causes memory corruption. Disable it until the issue is fixed upstream.

nodejs-github-bot · 2023-09-06T10:31:07Z

Review requested:

@nodejs/gyp

It seems that the optimization causes memory corruption. Disable it until the issue is fixed upstream. Fixes: nodejs#45268

nodejs-github-bot · 2023-09-06T14:05:31Z

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

nodejs-github-bot · 2023-09-07T06:00:07Z

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

nodejs-github-bot · 2023-09-09T12:41:21Z

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

nodejs-github-bot · 2023-09-10T05:44:51Z

Landed in ae115d6

lpinca · 2023-09-11T19:00:58Z

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1378/

                                                                       confidence improvement accuracy (*)   (**)  (***)
zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216                           -0.11 %       ±1.63% ±2.18% ±2.85%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                       -0.41 %       ±1.24% ±1.65% ±2.15%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                      0.26 %       ±1.13% ±1.50% ±1.96%
zlib/creation.js n=500000 options='false' type='Deflate'                               0.75 %       ±1.34% ±1.78% ±2.32%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                           -0.42 %       ±1.59% ±2.12% ±2.75%
zlib/creation.js n=500000 options='false' type='Gunzip'                               -0.01 %       ±1.27% ±1.69% ±2.20%
zlib/creation.js n=500000 options='false' type='Gzip'                                  0.16 %       ±1.59% ±2.12% ±2.76%
zlib/creation.js n=500000 options='false' type='Inflate'                              -1.09 %       ±1.37% ±1.82% ±2.37%
zlib/creation.js n=500000 options='false' type='InflateRaw'                            0.27 %       ±1.18% ±1.57% ±2.05%
zlib/creation.js n=500000 options='false' type='Unzip'                                 0.16 %       ±1.49% ±1.99% ±2.59%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                         0.32 %       ±1.33% ±1.78% ±2.32%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                       0.22 %       ±1.32% ±1.76% ±2.32%
zlib/creation.js n=500000 options='true' type='Deflate'                               -0.66 %       ±1.91% ±2.54% ±3.30%
zlib/creation.js n=500000 options='true' type='DeflateRaw'                             0.50 %       ±1.35% ±1.80% ±2.34%
zlib/creation.js n=500000 options='true' type='Gunzip'                                 0.07 %       ±1.37% ±1.83% ±2.38%
zlib/creation.js n=500000 options='true' type='Gzip'                                  -0.35 %       ±1.39% ±1.86% ±2.42%
zlib/creation.js n=500000 options='true' type='Inflate'                               -0.71 %       ±1.46% ±1.94% ±2.52%
zlib/creation.js n=500000 options='true' type='InflateRaw'                             0.62 %       ±1.45% ±1.93% ±2.52%
zlib/creation.js n=500000 options='true' type='Unzip'                                  1.08 %       ±1.30% ±1.73% ±2.25%
zlib/deflate.js n=400000 inputLen=1024 method='createDeflate'                 ***     13.00 %       ±2.29% ±3.06% ±4.00%
zlib/deflate.js n=400000 inputLen=1024 method='deflate'                         *      2.06 %       ±1.60% ±2.13% ±2.79%
zlib/deflate.js n=400000 inputLen=1024 method='deflateSync'                   ***      4.17 %       ±1.22% ±1.63% ±2.12%
zlib/inflate.js n=400000 inputLen=1024 method='inflate'                               -1.60 %       ±2.60% ±3.49% ±4.58%
zlib/inflate.js n=400000 inputLen=1024 method='inflateSync'                           -0.05 %       ±1.54% ±2.05% ±2.67%
zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024                -1.22 %       ±2.21% ±2.94% ±3.85%
zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024                -0.45 %       ±2.28% ±3.04% ±3.96%
zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024          ***     23.98 %       ±2.70% ±3.61% ±4.73%
zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024          ***     35.61 %       ±3.38% ±4.51% ±5.92%

lpinca · 2023-09-12T10:42:09Z

https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1379/

                                                                       confidence improvement accuracy (*)   (**)   (***)
zlib/createInflate.js n=100 chunkLen=1024 inputLen=16777216                     *     -2.28 %       ±2.16% ±2.88%  ±3.75%
zlib/creation.js n=500000 options='false' type='BrotliCompress'                        0.10 %       ±1.40% ±1.87%  ±2.43%
zlib/creation.js n=500000 options='false' type='BrotliDecompress'                     -0.15 %       ±1.08% ±1.44%  ±1.87%
zlib/creation.js n=500000 options='false' type='Deflate'                               0.78 %       ±1.54% ±2.05%  ±2.68%
zlib/creation.js n=500000 options='false' type='DeflateRaw'                           -0.23 %       ±1.34% ±1.79%  ±2.33%
zlib/creation.js n=500000 options='false' type='Gunzip'                               -1.30 %       ±1.78% ±2.38%  ±3.11%
zlib/creation.js n=500000 options='false' type='Gzip'                                 -0.98 %       ±1.17% ±1.55%  ±2.02%
zlib/creation.js n=500000 options='false' type='Inflate'                               0.15 %       ±1.46% ±1.94%  ±2.53%
zlib/creation.js n=500000 options='false' type='InflateRaw'                           -0.42 %       ±2.90% ±3.88%  ±5.07%
zlib/creation.js n=500000 options='false' type='Unzip'                          *     -1.84 %       ±1.64% ±2.19%  ±2.87%
zlib/creation.js n=500000 options='true' type='BrotliCompress'                         1.16 %       ±2.33% ±3.11%  ±4.07%
zlib/creation.js n=500000 options='true' type='BrotliDecompress'                *      1.24 %       ±1.07% ±1.43%  ±1.86%
zlib/creation.js n=500000 options='true' type='Deflate'                                2.11 %       ±2.90% ±3.88%  ±5.09%
zlib/creation.js n=500000 options='true' type='DeflateRaw'                             0.26 %       ±1.39% ±1.85%  ±2.41%
zlib/creation.js n=500000 options='true' type='Gunzip'                                 0.58 %       ±1.36% ±1.82%  ±2.36%
zlib/creation.js n=500000 options='true' type='Gzip'                                   0.29 %       ±1.22% ±1.63%  ±2.12%
zlib/creation.js n=500000 options='true' type='Inflate'                         *     -3.10 %       ±2.78% ±3.72%  ±4.91%
zlib/creation.js n=500000 options='true' type='InflateRaw'                            -0.47 %       ±1.23% ±1.64%  ±2.14%
zlib/creation.js n=500000 options='true' type='Unzip'                                  0.35 %       ±1.21% ±1.60%  ±2.09%
zlib/deflate.js n=400000 inputLen=1024 method='createDeflate'                 ***      9.51 %       ±3.55% ±4.77%  ±6.29%
zlib/deflate.js n=400000 inputLen=1024 method='deflate'                                3.20 %       ±3.89% ±5.22%  ±6.90%
zlib/deflate.js n=400000 inputLen=1024 method='deflateSync'                   ***      3.17 %       ±1.31% ±1.74%  ±2.27%
zlib/inflate.js n=400000 inputLen=1024 method='inflate'                               -1.55 %       ±1.84% ±2.45%  ±3.19%
zlib/inflate.js n=400000 inputLen=1024 method='inflateSync'                            1.38 %       ±1.56% ±2.07%  ±2.70%
zlib/pipe.js algorithm='brotli' type='buffer' duration=5 inputLen=1024                 0.10 %       ±2.01% ±2.68%  ±3.51%
zlib/pipe.js algorithm='brotli' type='string' duration=5 inputLen=1024                 0.36 %       ±2.77% ±3.70%  ±4.84%
zlib/pipe.js algorithm='gzip' type='buffer' duration=5 inputLen=1024          ***     26.79 %       ±3.21% ±4.27%  ±5.56%
zlib/pipe.js algorithm='gzip' type='string' duration=5 inputLen=1024          ***     33.82 %       ±6.22% ±8.28% ±10.79%

It seems that the optimization causes memory corruption. Disable it until the issue is fixed upstream. Fixes: #45268 PR-URL: #49511 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>

The memory corruption that led to disabling CRC32_SIMD_SSE42_PCLMUL came from crc_fold_copy() in deps/zlib/crc_folding.c. For the final (len % 16) bytes of a copy it did a 16-byte unaligned store, writing up to 15 bytes past the destination. With gzip at level 0, deflate_stored() copies input straight into strm->next_out through read_buf() and copy_with_crc(), so the overrun landed past the end of the caller's output buffer and corrupted the heap. Chromium replaced that store with zmemcpy(), and the fix arrived here with the zlib update in 70a0510 (v22.0.0). With the old code, the added test aborts v19.0.0 and v20.6.0 builds every time. With the current code it passes, and the CRC32 SIMD path is clean under ASan. The restored zlib_crc32_simd target predates clang-cl support on Windows, so also pass -msse4.2 and -mpclmul through msvs_settings, as zlib_adler32_simd does. The bundled zlib only detects CPU features in deflateInit2() and when crc32() or adler32() is called with a NULL buffer. zlib.crc32() always passes a buffer, so it used the portable code until a stream had triggered the detection. Trigger it when the binding is initialized so zlib.crc32() uses the SIMD path from the start. Fixes: nodejs#45268 Refs: nodejs#49511 Refs: nodejs#52123 Assisted-by: Claude Code Signed-off-by: Parker Loflin <parker@cronstable.dev>

The memory corruption that led to disabling CRC32_SIMD_SSE42_PCLMUL came from crc_fold_copy() in deps/zlib/crc_folding.c. For the final (len % 16) bytes of a copy it did a 16-byte unaligned store, writing up to 15 bytes past the destination. With gzip at level 0, deflate_stored() copies input straight into strm->next_out through read_buf() and copy_with_crc(), so the overrun landed past the end of the caller's output buffer and corrupted the heap. Chromium replaced that store with zmemcpy(), and the fix arrived here with the zlib update in 70a0510 (v22.0.0). With the old code, the added test aborts v19.0.0 and v20.6.0 builds every time. With the current code it passes, and the CRC32 SIMD path is clean under ASan and valgrind. The restored zlib_crc32_simd target predates clang-cl support on Windows, so also pass -msse4.2 and -mpclmul through msvs_settings, as zlib_adler32_simd does. The bundled zlib only detects CPU features in deflateInit2() and when crc32() or adler32() is called with a NULL buffer. zlib.crc32() always passes a buffer, so it used the portable code until a stream had triggered the detection. Trigger it when the binding is initialized so zlib.crc32() uses the SIMD path from the start. Refs: nodejs#45268 Refs: nodejs#49511 Refs: nodejs#52123 Assisted-by: Claude Code Signed-off-by: Parker Loflin <parker@cronstable.dev>

nodejs-github-bot added dependencies PRs that add, update, or configure Node.js dependencies. needs-ci PRs that need a full CI run. zlib Issues and PRs related to the zlib module and its compression dependencies. labels Sep 6, 2023

lpinca force-pushed the disable/zlib-optimization branch 2 times, most recently from d39b222 to a7bb3ee Compare September 6, 2023 10:35

zlib: disable CRC32 SIMD optimization

550a760

It seems that the optimization causes memory corruption. Disable it until the issue is fixed upstream. Fixes: nodejs#45268

lpinca force-pushed the disable/zlib-optimization branch from a7bb3ee to 550a760 Compare September 6, 2023 10:45

bnoordhuis approved these changes Sep 6, 2023

View reviewed changes

lpinca added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 6, 2023

github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 6, 2023

anonrig approved these changes Sep 6, 2023

View reviewed changes

lpinca mentioned this pull request Sep 7, 2023

Memory corruption and crash when streaming to zlib #45268

Closed

This was referenced Sep 8, 2023

CI Reliability 2023-09-08 nodejs/reliability#659

Open

CI Reliability 2023-09-09 nodejs/reliability#660

Open

github-actions Bot mentioned this pull request Sep 10, 2023

CI Reliability 2023-09-10 nodejs/reliability#661

Open

19 tasks

lpinca added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 10, 2023

nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 10, 2023

nodejs-github-bot merged commit ae115d6 into nodejs:main Sep 10, 2023

lpinca deleted the disable/zlib-optimization branch September 10, 2023 05:59

github-actions Bot mentioned this pull request Sep 11, 2023

CI Reliability 2023-09-11 nodejs/reliability#662

Open

18 tasks

lpinca added needs-benchmark-ci PRs that need a benchmark CI run. and removed needs-benchmark-ci PRs that need a benchmark CI run. labels Sep 11, 2023

This was referenced Sep 28, 2023

v20.7.1 proposal #49917

Closed

v20.8.0 proposal #49932

Merged

ptweezy mentioned this pull request Oct 30, 2026

zlib: re-enable CRC32 SIMD optimization #67148

Open

Navigation Menu

Repository navigation

zlib: disable CRC32 SIMD optimization - #49511

zlib: disable CRC32 SIMD optimization#49511
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
lpinca:disable/zlib-optimization

lpinca commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 7, 2023

Uh oh!

nodejs-github-bot commented Sep 9, 2023

Uh oh!

nodejs-github-bot commented Sep 10, 2023

Uh oh!

lpinca commented Sep 11, 2023 •

edited

Loading

Uh oh!

lpinca commented Sep 12, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

lpinca commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 6, 2023

Uh oh!

nodejs-github-bot commented Sep 7, 2023

Uh oh!

nodejs-github-bot commented Sep 9, 2023

Uh oh!

nodejs-github-bot commented Sep 10, 2023

Uh oh!

lpinca commented Sep 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lpinca commented Sep 12, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lpinca commented Sep 11, 2023 •

edited

Loading