NodeJS unconditionally uses AVX512 instructions even if they are disabled

mgaudet commented 3 months ago

Version

v22.0.0-pre and above

Platform

Linux ZenTower 6.8.0-35-generic #35-Ubuntu SMP PREEMPT_DYNAMIC Mon May 20 15:51:52 UTC 2024 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

base64

What steps will reproduce the bug?

On a linux machine which supports the AVX-512 extension, disable it, then run ./out/Release/cctest

I have reproduced this on an AMD Ryzen Threadripper PRO 7975WX. For compatibility with https://pernos.co/, I've explicitly disabled AVX-512 by adding clearcpuid=304 to GRUB_CMDLINE_LINUX_DEFAULT in /etc/default/grub

As you can see, AVX512 isn't listed in the /proc/cpuinfo:

flags       : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge mca cmov pat pse36 clflush mmx fxsr sse sse2 ht syscall nx mmxext fxsr_opt pdpe1gb rdtscp lm constant_tsc rep_good amd_lbr_v2 nopl nonstop_tsc cpuid extd_apicid aperfmperf rapl pni pclmulqdq monitor ssse3 fma cx16 pcid sse4_1 sse4_2 movbe popcnt aes xsave avx f16c rdrand lahf_lm cmp_legacy svm extapic cr8_legacy abm sse4a misalignsse 3dnowprefetch osvw ibs skinit wdt tce topoext perfctr_core perfctr_nb bpext perfctr_llc mwaitx cpb cat_l3 cdp_l3 hw_pstate ssbd mba perfmon_v2 ibrs ibpb stibp ibrs_enhanced vmmcall fsgsbase bmi1 avx2 smep bmi2 erms invpcid cqm rdt_a rdseed adx smap clflushopt clwb sha_ni xsaveopt xsavec xgetbv1 xsaves cqm_llc cqm_occup_llc cqm_mbm_total cqm_mbm_local clzero irperf xsaveerptr rdpru wbnoinvd amd_ppin cppc arat npt lbrv svm_lock nrip_save tsc_scale vmcb_clean flushbyasid decodeassists pausefilter pfthreshold avic v_vmsave_vmload vgif x2avic v_spec_ctrl vnmi umip pku ospke rdpid overflow_recov succor smca fsrm flush_l1d debug_swap
      bugs      : sysret_ss_attrs spectre_v1 spectre_v2 spec_store_bypass srso

So this CPU feature is not being correctly tested

How often does it reproduce? Is there a required condition?

As soon as something uses base64 encode, 100%.

What is the expected behavior? Why is that the expected behavior?

Runs correctly

What do you see instead?

SIGILL:

[----------] 3 tests from Base64Test
[ RUN      ] Base64Test.Encode
Illegal instruction (core dumped)

Additional information

Originally reported as https://github.com/microsoft/vscode/issues/214630, but I have bisected node down to this (f45bb801b69c7105756b971ab076c415d85a9e10) commit:

commit f45bb801b69c7105756b971ab076c415d85a9e10
Author: Node.js GitHub Bot <github-bot@iojs.org>
Date:   Wed Nov 8 19:48:51 2023 +0000

    deps: update base64 to 0.5.1

    PR-URL: https://github.com/nodejs/node/pull/50629
    Fixes: https://github.com/nodejs/node/issues/50561
    Fixes: https://github.com/nodejs/node/pull/45091
    Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
    Reviewed-By: Mohammed Keyvanzadeh <mohammadkeyvanzade94@gmail.com>
    Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
    Reviewed-By: Richard Lau <rlau@redhat.com>

targos commented 3 months ago

@lemire @anonrig

lemire commented 3 months ago

The base64 library is no longer a Node.js dependency after https://github.com/nodejs/node/pull/52714

But PR 52714 and following are not yet part of a release.

lemire commented 3 months ago

Looks like @mgaudet has proposed a patch to base64.

nodejs / node