1851301 - simde addition in bug 1829765 requires a recent clang version ?

Status: RESOLVED FIXED

(Core :: Security: RLBox, defect)

Description

[Landry Breuil (:gaston)]

m-c builds fine (in a buildbot) on OpenBSD with llvm 13, which is our baseline compiler for now.

i've tried building m-c on a laptop with a similar .mozconfig, and it seems some primitives expected by simde arent provided by our version of the clang headers:

 0:22.73 In file included from /home/landry/src/m-c/media/libsoundtouch/src/sse_optimized.cpp:65:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx2.h:33:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx.h:32:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.2.h:31:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.1.h:31:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/ssse3.h:30:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse3.h:30:
 0:22.74 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5043:22: error: use of undeclared identifier 'wasm_u8x16_make'; did you mean 'wasm_i8x16_make'?
 0:22.74       r_.wasm_v128 = wasm_u8x16_make(e0, e1, e2, e3, e4, e5, e6, e7, e8, e9, e10, e11, e12, e13, e14, e15);
 0:22.74                      ^~~~~~~~~~~~~~~
 0:22.74                      wasm_i8x16_make
 0:22.74 /usr/local/lib/clang/13.0.0/include/wasm_simd128.h:270:1: note: 'wasm_i8x16_make' declared here
 0:22.74 wasm_i8x16_make(int8_t __c0, int8_t __c1, int8_t __c2, int8_t __c3, int8_t __c4,
 0:22.74 ^
 0:22.74 In file included from /home/landry/src/m-c/media/libsoundtouch/src/sse_optimized.cpp:65:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx2.h:33:
 0:22.74 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx.h:32:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.2.h:31:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.1.h:31:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/ssse3.h:30:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse3.h:30:
 0:22.75 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5070:22: error: use of undeclared identifier 'wasm_u16x8_make'; did you mean 'wasm_i16x8_make'?
 0:22.75       r_.wasm_v128 = wasm_u16x8_make(e0, e1, e2, e3, e4, e5, e6, e7);
 0:22.75                      ^~~~~~~~~~~~~~~
 0:22.75                      wasm_i16x8_make
 0:22.75 /usr/local/lib/clang/13.0.0/include/wasm_simd128.h:280:1: note: 'wasm_i16x8_make' declared here
 0:22.75 wasm_i16x8_make(int16_t __c0, int16_t __c1, int16_t __c2, int16_t __c3,
 0:22.75 ^
 0:22.75 In file included from /home/landry/src/m-c/media/libsoundtouch/src/sse_optimized.cpp:65:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx2.h:33:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx.h:32:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.2.h:31:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.1.h:31:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/ssse3.h:30:
 0:22.75 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse3.h:30:
 0:22.76 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5093:22: error: use of undeclared identifier 'wasm_u32x4_make'
 0:22.76       r_.wasm_v128 = wasm_u32x4_make(e0, e1, e2, e3);
 0:22.76 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5117:22: error: use of undeclared identifier 'wasm_u64x2_make'
 0:22.76       r_.wasm_v128 = wasm_u64x2_make(e0, e1);
 0:22.76                      ^
 0:22.76 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5276:40: error: use of undeclared identifier 'wasm_u8x16_splat'; did you mean 'wasm_i8x16_splat'?
 0:22.76     return simde__m128i_from_wasm_v128(wasm_u8x16_splat(value));
 0:22.76                                        ^~~~~~~~~~~~~~~~
 0:22.76                                        wasm_i8x16_splat
 0:22.76 /usr/local/lib/clang/13.0.0/include/wasm_simd128.h:394:45: note: 'wasm_i8x16_splat' declared here
 0:22.76 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i8x16_splat(int8_t __a) {
 0:22.76                                             ^
 0:22.76 In file included from /home/landry/src/m-c/media/libsoundtouch/src/sse_optimized.cpp:65:
 0:22.76 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx2.h:33:
 0:22.76 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx.h:32:
 0:22.76 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.2.h:31:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.1.h:31:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/ssse3.h:30:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse3.h:30:
 0:22.77 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5288:40: error: use of undeclared identifier 'wasm_u16x8_splat'; did you mean 'wasm_i16x8_splat'?
 0:22.77     return simde__m128i_from_wasm_v128(wasm_u16x8_splat(value));
 0:22.77                                        ^~~~~~~~~~~~~~~~
 0:22.77                                        wasm_i16x8_splat
 0:22.77 /usr/local/lib/clang/13.0.0/include/wasm_simd128.h:420:45: note: 'wasm_i16x8_splat' declared here
 0:22.77 static __inline__ v128_t __DEFAULT_FN_ATTRS wasm_i16x8_splat(int16_t __a) {
 0:22.77                                             ^
 0:22.77 In file included from /home/landry/src/m-c/media/libsoundtouch/src/sse_optimized.cpp:65:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx2.h:33:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/avx.h:32:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.2.h:31:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse4.1.h:31:
 0:22.77 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/ssse3.h:30:
 0:22.78 In file included from /home/landry/src/m-c/third_party/simde/simde/x86/sse3.h:30:
 0:22.78 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5300:40: error: use of undeclared identifier 'wasm_u32x4_splat'
 0:22.78     return simde__m128i_from_wasm_v128(wasm_u32x4_splat(value));
 0:22.78                                        ^
 0:22.78 /home/landry/src/m-c/third_party/simde/simde/x86/sse2.h:5312:40: error: use of undeclared identifier 'wasm_u64x2_splat'
 0:22.78     return simde__m128i_from_wasm_v128(wasm_u64x2_splat(value));
 0:22.78                                        ^

from my understanding, our /usr/local/lib/clang/13.0.0/include/wasm_simd128.h file only provides wasm_i* and wasm_f* variants, not wasm_u*..

i'll try to figure out what is different between this broken env and the working env.

Comment 1

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1829765

:wrv, since you are the author of the regressor, bug 1829765, could you take a look?

For more information, please visit BugBot documentation.

Comment 2

[Landry Breuil (:gaston)]

after looking at https://github.com/llvm/llvm-project/commit/2456e11614c10a2e648005e27e3213c77b7ab7a4 it seems those unsigned variants are only present in llvm 14, so.. llvm 13 is now unsupported to build m-c ?

Comment 3

[Landry Breuil (:gaston)]

to be 100% clear, that's not a 'definite' failure since 117 (where this bug landed) builds and works fine on OpenBSD, i'm just trying to understand why m-c failed to build on a particular environment similar to other working environments.

Comment 4

[Landry Breuil (:gaston)]

i can reproduce the failure on my buildbot with m-c (cf http://buildbot.rhaalovely.net/nine/#/builders/3/builds/1811/steps/8/logs/stdio), now to understand what changed in the last system update i did that could have caused this fallout/side effect..

Comment 5

[Landry Breuil (:gaston)]

manually applying https://github.com/llvm/llvm-project/commit/2456e11614c10a2e648005e27e3213c77b7ab7a4 on the systemwide clang 13 wasm_simd128.h header fixes the build failure.. but that isnt a solution, and is definitely puzzling.

Comment 6

[Willy R. Vasquez]

The likely reason this is breaking now is because of this patch for this bug.

It's not clear to me why the envs are different -- do you have a place I can look where it works with llvm13 and OpenBSD?

For now, would it make sense to disable the soundtouch SSE optimization for OpenBSD? https://searchfox.org/mozilla-central/source/security/rlbox/moz.build#108

Comment 7

[Mike Hommey [:glandium]]

It's not an OS problem. It's a clang version problem. wasm_u8x16_make was added in clang/llvm 14.

Comment 8

[Landry Breuil (:gaston)]

(In reply to Mike Hommey [:glandium] from comment #7)

It's not an OS problem. It's a clang version problem. wasm_u8x16_make was added in clang/llvm 14.

yeah that's what i had figured out too - we're slowly in the process of moving to clang 16 but so far 13 is still our baseline.

that means there's no tier1 build tests with clang 13 anymore ? Is 14 going to be a requirement or that's just an oversight and the wasm_u8x16_make usage can be put behind a clang version check ?

(In reply to Willy R. Vasquez from comment #6)

The likely reason this is breaking now is because of this patch for this bug.

It's not clear to me why the envs are different -- do you have a place I can look where it works with llvm13 and OpenBSD?

in the end, yes seems that's fallout from this bug, since my builds of m-c with clang 13 only recently started failling, and i worked around it by manually applying the clang commit on top of my builders.

For now, would it make sense to disable the soundtouch SSE optimization for OpenBSD? https://searchfox.org/mozilla-central/source/security/rlbox/moz.build#108

or rather disabling it if clang is older than 14 ?

Comment 9

[Willy R. Vasquez]

:glandium should I submit a patch to disable SSE if clang is older than 14, or do you have another recommendation?

Comment 10

[Mike Hommey [:glandium]]

Sounds fair.

Comment 11

[Landry Breuil (:gaston)]

in the meantime in https://github.com/openbsd/ports/commit/5ff76cd5fa052b0fdf43f4a732ac3b871c493094 i've had the llvm commit backported to our llvm 13 so that i can keep my buildbot of m-c churning .. but i dunno if other third-parties are still building with llvm13.

Comment 12

[Willy R. Vasquez]

Attachment: Bug 1851301 - Disable WASMSIMD for Clang version less than 14. r=glandium

Comment 13

[Willy R. Vasquez]

Attachment: Bug 1851301 - Disable Soundtouch WASMSIMD for Clang version less than 14. r=glandium

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97b17bf99b01
Disable Soundtouch WASMSIMD for Clang version less than 14. r=glandium

Comment 15

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/97b17bf99b01

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

The patch landed in nightly and beta is affected.
:wrv, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox119 to wontfix.

For more information, please visit BugBot documentation.

Comment 17

[Willy R. Vasquez]

:glandium do you think this requires an uplift?

Comment 18

[Mike Hommey [:glandium]]

soundtouch sandboxing was disabled at the time central was merged to beta, so beta is actually not affected.