Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NodeJS unconditionally uses AVX512 instructions even if they are disabled #53426

Open
mgaudet opened this issue Jun 12, 2024 · 3 comments
Open
Labels
dependencies Pull requests that update a dependency file.

Comments

@mgaudet
Copy link

mgaudet commented Jun 12, 2024

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 microsoft/vscode#214630, but I have bisected node down to this (f45bb80) commit:

commit f45bb801b69c7105756b971ab076c415d85a9e10
Author: Node.js GitHub Bot <[email protected]>
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 <[email protected]>
    Reviewed-By: Mohammed Keyvanzadeh <[email protected]>
    Reviewed-By: Yagiz Nizipli <[email protected]>
    Reviewed-By: Richard Lau <[email protected]>
@targos
Copy link
Member

targos commented Jun 12, 2024

@lemire @anonrig

@lemire
Copy link
Member

lemire commented Jun 12, 2024

The base64 library is no longer a Node.js dependency after #52714

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

@lemire
Copy link
Member

lemire commented Jun 12, 2024

Looks like @mgaudet has proposed a patch to base64.

@tniessen tniessen added the dependencies Pull requests that update a dependency file. label Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file.
Projects
None yet
Development

No branches or pull requests

4 participants