1513114 - "error: address argument to atomic operation must be a pointer to a trivially-copyable type" in libdav1d

Status: RESOLVED WONTFIX

(Core :: Audio/Video: Playback, defect, P2)

Description

[Botond Ballo [:botond]]

Attachment: Full errors

I'm trying to build m-c from today and getting errors like this:

 1:24.69 /home/botond/dev/mozilla/central/third_party/dav1d/src/cdf.c:4222:9: error: address argument to atomic operation must be a pointer to a trivially-copyable type ('_Atomic(unsigned int) *' invalid)
 1:24.69         atomic_init(cdf->progress, 0);
 1:24.69         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1:24.69 /usr/bin/../lib/gcc/x86_64-linux-gnu/6.3.0/include/stdatomic.h:83:3: note: expanded from macro 'atomic_init'
 1:24.69   atomic_store_explicit (PTR, VAL, __ATOMIC_RELAXED)
 1:24.69   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 1:24.69 /usr/bin/../lib/gcc/x86_64-linux-gnu/6.3.0/include/stdatomic.h:126:5: note: expanded from macro 'atomic_store_explicit'
 1:24.69     __atomic_store (__atomic_store_ptr, &__atomic_store_tmp, (MO));     \
 1:24.69     ^               ~~~~~~~~~~~~~~~~~~

The compiler I'm building with is clang 5.0.

Comment 1

[Botond Ballo [:botond]]

(In reply to Botond Ballo [:botond] from comment #0)
> The compiler I'm building with is clang 5.0.

Using clang 6.0 makes the errors go away.

Comment 2

[Andreas Pehrson [:pehrsons]]

Per [1], the minimum supported version of clang is 3.9, so this is then a regression blocking anyone on certain supported compilers from building.

Marking P1 for fixing in 66. Thomas, can you take this?


[1] https://searchfox.org/mozilla-central/rev/fd62b95c187a40b328d9e7fd9d848833a6942b57/build/moz.configure/toolchain.configure#433

Comment 3

[Brendan Dahl [:bdahl]]

(In reply to Botond Ballo [:botond] from comment #1)
> (In reply to Botond Ballo [:botond] from comment #0)
> > The compiler I'm building with is clang 5.0.
> 
> Using clang 6.0 makes the errors go away.

I still see this error using clang-6.0.0. What's your full version number?

Comment 4

[Thomas Daede [:TD-Linux]]

dav1d has an atomics compat header that we can enable for old compiler versions. It should just be a matter of matching the clang version the moz.build / config.h.

Comment 5

[Brendan Dahl [:bdahl]]

7.0 fixed it locally.

Comment 6

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Is this a new regression from 66?

Comment 7

[Botond Ballo [:botond]]

(In reply to Brendan Dahl [:bdahl] from comment #3)
> (In reply to Botond Ballo [:botond] from comment #1)
> > (In reply to Botond Ballo [:botond] from comment #0)
> > > The compiler I'm building with is clang 5.0.
> > 
> > Using clang 6.0 makes the errors go away.
> 
> I still see this error using clang-6.0.0. What's your full version number?

$ clang-6.0 --version
clang version 6.0.1-svn334776-1~exp1~20181018153230.114 (branches/release_60)
Target: x86_64-pc-linux-gnu
Thread model: posix
InstalledDir: /usr/bin

(In reply to Liz Henry (:lizzard) (needinfo? me) from comment #6)
> Is this a new regression from 66?

I started seeing this after pulling on Monday, and I previously pulled ~2 weeks before that, so it seems like a regression from sometime in the past 2 weeks.

Comment 8

[Alex Chronopoulos [:achronop]]

It must have been introduced with bug 1493400. 

Can you please do a stand alone build of dav1d library and provide the config.h and config.asm file found in build directory?
Instructions: https://code.videolan.org/videolan/dav1d

Can you also mention your platform and your OS?
Thanks!

Comment 9

[Botond Ballo [:botond]]

(In reply to Alex Chronopoulos [:achronop] from comment #8)
> Can you please do a stand alone build of dav1d library and provide the
> config.h and config.asm file found in build directory?
> Instructions: https://code.videolan.org/videolan/dav1d

It seems to require things that a Firefox build does not, like newer versions of meson and nasm than are available in my distribution's package repository. Not sure if it's worth the effort...

> Can you also mention your platform and your OS?

Debian 9.

Comment 10

[Alex Chronopoulos [:achronop]]

I would suggest to have a try, it's easy enough to install meson (requires python3) and ninja. Also, they are useful tools, nice to have. New question, are you using icecc to build?

Comment 11

[Botond Ballo [:botond]]

Ok, I built nasm 2.14 from source and was able to proceed to build dav1d.

(In reply to Alex Chronopoulos [:achronop] from comment #8)
> Can you please do a stand alone build of dav1d library and provide the
> config.h and config.asm file found in build directory?

Coming up.

(In reply to Alex Chronopoulos [:achronop] from comment #10)
> New question, are you using icecc to build?

Nope.

Comment 12

[Botond Ballo [:botond]]

Attachment: config.h from standalone dav1d build

Comment 13

[Botond Ballo [:botond]]

Attachment: config.asm from standalone dav1d build

Comment 14

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Alex are you going to take this on? Or can you help suggest an owner? Thanks!

Comment 15

[Alex Chronopoulos [:achronop]]

Sure, I don't believe it's a P1, though. In general dav1d can only be build on o modern compiler so we have to disable DAV1D support on older versions on clang. Also the error is limited to some local builds.

Comment 16

[Liz Henry (:lizzard) (privacy/fingerprinting team)]

Dropping tracking as it sounds like the workaround is to update the clang version.

Comment 17

[Brendan Dahl [:bdahl]]

Can we make this fail earlier on a configure step? I've run into this a few times when moving around mozconfig or machines where I end up waiting 20 minutes only to find it failed because I didn't have the right clang version.

Comment 18

[Alex Chronopoulos [:achronop]]

Thomas, do you thing we can check that during configure step, similar we do for nasm version in Bug 1515641?

Comment 19

[Thomas Daede [:TD-Linux]]

Yeah sure. What exactly are the clang versions affected? Should we check for clang 6 or 7?

Comment 20

[Brendan Dahl [:bdahl]]

^^

Comment 21

[Alex Chronopoulos [:achronop]]

Thanks for bumping this Brendan, can you please answer on Thomas question from comment 19?

Comment 22

[Brendan Dahl [:bdahl]]

7.0 fixed it for me locally, but I have no idea what the actual requirements are. I was hoping someone who merges in or works on dav1d knows.

Comment 23

[Brendan Dahl [:bdahl]]

FWIW, ./mach bootstrap is pulling down clang version 8.0.0 though.

Comment 24

[Alex Chronopoulos [:achronop]]

Thomas could you check for clang 6 and abort during configuration (similar to nasm)?

Comment 25

[Mike Hommey [:glandium]]

FWIW, this still affects clang 11 when targeting mips.

Comment 26

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

The bug assignee didn't login in Bugzilla in the last 7 months.
:jimm, could you have a look please?
For more information, please visit auto_nag documentation.