1645416 - Investigate the struct layout of `MPSCQueue<T>::Message` on Windows

Status: RESOLVED FIXED

(Core :: Audio/Video: cubeb, task)

Description

[Paul Adenot (:padenot)]

The layout is different and there seem to be a hole.

Comment 1

[Paul Adenot (:padenot)]

Same on android it seems. Probably differently sized types.

Comment 2

[Paul Adenot (:padenot)]

Attachment: Bug 1645416 - Fix the TracePayload layout on 32bits and re-enable the static assert that check the struct has a size that is a power of two. r?achronop

Comment 3

[Paul Adenot (:padenot)]

Green on try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=35a45361452378100241ca677d2e3dc19b16617f it was a mix of 32bits vs. 64bits and TimeStamp, that changes size on different platforms. The assert is now re-enabled unconditionally.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a956cf3b8d3
Fix the TracePayload layout on 32bits and re-enable the static assert that check the struct has a size that is a power of two. r=achronop

Comment 5

[Razvan Maries]

Backed out for build bustages.

Push with failure: https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&revision=3a956cf3b8d34a361bf6897a30b99f9dc13eedc2&selectedTaskRun=V0-enEJKSfOD2_a1-4iCKw.0

Failure log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307370363&repo=autoland&lineNumber=18196

Backout: https://hg.mozilla.org/integration/autoland/rev/04ed910d0a2fa35e25c9d88a66cdead6777eacd6

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/4ae7220cf7d8
Fix the TracePayload layout on 32bits and re-enable the static assert that check the struct has a size that is a power of two. r=achronop

Comment 7

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/4ae7220cf7d8

Comment 8

[Noemi Erli[:noemi_erli]]

Reopening since this patch causes Linux build bustages on central:
https://treeherder.mozilla.org/#/jobs?repo=mozilla-central&group_state=expanded&selectedTaskRun=XJ4xXVr4QZ2rdaUTbtJT3A.0&resultStatus=testfailed%2Cbusted%2Cexception&searchStr=linux&revision=1004d422aedb5994f9f2949fd4a4c67bc89e22c7
Backout: https://hg.mozilla.org/mozilla-central/rev/8fcf1cac56600d932240486ffe7a06d46a0c931c
Log: https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=307433529&repo=mozilla-central&lineNumber=32757

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/55a444d9df03
Fix the TracePayload layout on 32bits and re-enable the static assert that check the struct has a size that is a power of two. r=achronop

Comment 10

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/55a444d9df03

Comment 11

[tt_1]

hey everyone, I get this beauty when compiling firefox-79.0_beta2 for armv7:

8:40.97 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/MPSCQueue.h: In instantiation of 'class mozilla::MPSCQueue<mozilla::AsyncLogger::TracePayload>':
8:40.97 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/AsyncLogger.h:251:27: required from here
8:40.97 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/MPSCQueue.h:47:29: error: static assertion failed: MPSCQueue internal allocations must have a size that is a power of two
8:40.97 47 | static_assert(IsPowerOfTwo(sizeof(MPSCQueue<T>::Message)),
8:40.97 | ^

8:56.80 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/MPSCQueue.h: In instantiation of 'class mozilla::MPSCQueue<mozilla::AsyncLogger::TracePayload>':
8:56.80 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/AsyncLogger.h:251:27: required from here
8:56.80 /usr/armv7a-unknown-linux-gnueabihf/tmp/portage/www-client/firefox-79.0_beta2/work/firefox-79.0/dom/media/MPSCQueue.h:47:29: error: static assertion failed: MPSCQueue internal allocations must have a size that is a power of two
8:56.80 47 | static_assert(IsPowerOfTwo(sizeof(MPSCQueue<T>::Message)),

line 47 is guarded, it seems there's still a hole left for a failure:

#if !(defined(ANDROID) && defined(i386))
static_assert(IsPowerOfTwo(sizeof(MPSCQueue<T>::Message)),
"MPSCQueue internal allocations must have a size that is a "
"power of two ");
#endif

Comment 12

[tt_1]

Attachment: build.log.gz

here's the full build log, I'm on linux and cross compiling from amd64 host to armv7 target

Comment 13

[Paul Adenot (:padenot)]

I posted a patch in bug 1649691 if you can try it ?

Comment 14

[tt_1]

let's continue over in 1649691