1698156 - GLSL macros with arguments causes parse errors. (impossible to build Firefox)

Status: RESOLVED FIXED

(Core :: Graphics: WebRender, defect)

Description

[Nicolas B. Pierron [:nbp]]

When building Firefox, I see the following error messages on mozilla/central.

 2:12.41   /home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/brush_blend.frag: Err(
 2:12.42       ParseError {
 2:12.42           info: "0: at line 511, in TakeWhile1:\n#define antialias_brush() 1.0\n                        ^\n\n",
 2:12.42       },
 2:12.42   )

After adding extra code to dump the file name.

I seems that the macro with parentheses are not supported. Doing the following fixes this issue:
git grep -l antialias_brush | xargs sed -ie 's/antialias_brush()/antialias_brush/'

but yield another issue of the same nature:

 0:51.01   /home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/cs_clip_rectangle.frag: Err(
 0:51.01       ParseError {
 0:51.01           info: "0: at line 565:\n#define CLIP_CORNER(offset,normal,info) do { float dist = dot(local_pos0 - (offset), (normal)); float scale = -dot(local_step, (normal)); if (scale >= 0.0) { if (dist > opaque_start * scale) { start_corner = info; start_plane = vec4(offset, normal); float inv_scale = recip(max(scale, 1.0e-6)); opaque_start = dist * inv_scale; float apex = (0.7071 - 0.5) * 2.0 * abs(normal.x * normal.y); aa_start = opaque_start - apex * inv_scale; } } else if (dist > opaque_end * scale) { end_corner = info; end_plane = vec4(offset, normal); float inv_scale = recip(min(scale, -1.0e-6)); opaque_end = dist * inv_scale; float apex = (0.7071 - 0.5) * 2.0 * abs(normal.x * normal.y); aa_end = opaque_end - apex * inv_scale; } } while (false)\n^\nexpected \'}\', found #\n\n",
 0:51.02       },
 0:51.02   )

Comment 1

[Jeff Muizelaar [:jrmuizel]]

It seems like swgl is getting unpreprocessed glsl. It is supposed to be preprocessed by this code: https://searchfox.org/mozilla-central/source/gfx/wr/swgl/build.rs#81

Comment 2

[Nicolas B. Pierron [:nbp]]

Attachment: generated-command-line.txt

Running `./mach build -v` leads to the following command line:

```
"/nix/store/ca37d3qrydh0wpw40kswsx30j8dyzxh2-gcc-wrapper-10.2.0/bin/cc" "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-std=gnu99" "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/system_wrappers" "-include" "/home/nicolas/mozilla/wksp-5/config/gcc_hidden.h" "-U_FORTIFY_SOURCE" "-D_FORTIFY_SOURCE=2" "-fstack-protector-strong" "-DNDEBUG=1" "-DTRIMMED=1" "-I/home/nicolas/mozilla/wksp-5/toolkit/library/rust" "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/toolkit/library/rust" "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include" "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nspr" "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nss" "-fPIC" "-include" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/mozilla-config.h" "-DMOZILLA_CLIENT" "-fno-strict-aliasing" "-fno-math-errno" "-pthread" "-pipe" "-ggdb3" "-freorder-blocks" "-O2" "-fno-omit-frame-pointer" "-funwind-tables" "-DMOZILLA_CONFIG_H" "-xc" "-P" "-DWR_FRAGMENT_SHADER=1" "-E" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/cs_clip_rectangle.c"
```

Which is a wrapper provided by Nix around the compiler, which expand to the following set of command line arguments:

```

```

Comment 3

[Nicolas B. Pierron [:nbp]]

Attachment: generated command line

Comment 4

[Nicolas B. Pierron [:nbp]]

Running ./mach build -v leads to the following command line:

"/nix/store/ca37d3qrydh0wpw40kswsx30j8dyzxh2-gcc-wrapper-10.2.0/bin/cc"
  "-O2" "-ffunction-sections" "-fdata-sections" "-fPIC" "-m64" "-std=gnu99"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/system_wrappers"
  "-include" "/home/nicolas/mozilla/wksp-5/config/gcc_hidden.h"
  "-U_FORTIFY_SOURCE" "-D_FORTIFY_SOURCE=2"
  "-fstack-protector-strong" "-DNDEBUG=1" "-DTRIMMED=1"
  "-I/home/nicolas/mozilla/wksp-5/toolkit/library/rust"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/toolkit/library/rust"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nspr"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nss"
  "-fPIC"
  "-include" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/mozilla-config.h"
  "-DMOZILLA_CLIENT" "-fno-strict-aliasing" "-fno-math-errno" "-pthread"
  "-pipe" "-ggdb3" "-freorder-blocks" "-O2" "-fno-omit-frame-pointer" "-funwind-tables"
  "-DMOZILLA_CONFIG_H" "-xc" "-P" "-DWR_FRAGMENT_SHADER=1"
  "-E" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/cs_clip_rectangle.c"

Which is a wrapper provided by Nix around the compiler, which expand to the following set of command line arguments

Comment 5

[Nicolas B. Pierron [:nbp]]

Ok, bisecting the command line, it seems that -ggdb3 adds all the macro definitions, and -include /home/nicolas/mozilla/wksp-5/config/gcc_hidden.h add a visibility pragma, which does not seems to cause errors while parsing.

Comment 6

[Nicolas B. Pierron [:nbp]]

I will note, that -ggdb3 comes from --enable-debug-symbols=-ggdb3 in the mozconfig.

Comment 7

[Jamie Nicol [:jnicol]]

Lee, any ideas why this compiler command line would cause this swgl build error?

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Apparently -ggdb3 adds back the macro definitions after preprocessing which breaks the glsl parsing which assumes preprocessed source.

Comment 9

[Nicolas B. Pierron [:nbp]]

My guess would be that we could filter out these macros from the output, as these are being collected in Rust code.
We could have some code similar to:

  let mut in_macro = false;
  let fs : Vec<u8> = fs.into_iter().filter(|c| {
    if c == b'#' {
      in_macro = true;
    } else if c == b'\n' {
      in_macro = false;
    }
    !in_macro
  }).collect();

Should I make this patch?

Comment 10

[Jeff Muizelaar [:jrmuizel]]

Or strip out '-ggdb3' or even all of the CFLAGS from the compiler flags.

Comment 11

[Nicolas B. Pierron [:nbp]]

(In reply to Jeff Muizelaar [:jrmuizel] from comment #10)

Or strip out '-ggdb3' or even all of the CFLAGS from the compiler flags.

This is my current work-around, to remove the --enable-debug-symbols from the mozconfig.

Comment 12

[Jeff Muizelaar [:jrmuizel]]

I meant remove it from CFLAGS here https://searchfox.org/mozilla-central/source/gfx/wr/swgl/build.rs#76

Comment 13

[Lee Salzman [:lsalzman]]

The cc crate says the setting the environment variable CRATE_CC_NO_DEFAULTS=true will make it not use the default C flags. Does this fix it for you?

Comment 14

[Lee Salzman [:lsalzman]]

Attachment: Bug 1698156 - Disable default flags for SWGL preprocessor. r?jrmuizel

Comment 15

[Nicolas B. Pierron [:nbp]]

Adding the same line as the patch locally does not seems to fix the issue:

"/nix/store/ca37d3qrydh0wpw40kswsx30j8dyzxh2-gcc-wrapper-10.2.0/bin/cc"
  "-std=gnu99"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/system_wrappers"
  "-include" "/home/nicolas/mozilla/wksp-5/config/gcc_hidden.h"
  "-U_FORTIFY_SOURCE" "-D_FORTIFY_SOURCE=2"
  "-fstack-protector-strong" "-DNDEBUG=1" "-DTRIMMED=1"
  "-I/home/nicolas/mozilla/wksp-5/toolkit/library/rust"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/toolkit/library/rust"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nspr"
  "-I/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/dist/include/nss"
  "-fPIC"
  "-include" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/mozilla-config.h"
  "-DMOZILLA_CLIENT" "-fno-strict-aliasing" "-fno-math-errno" "-pthread"
  "-pipe" "-ggdb2" "-freorder-blocks" "-O2" "-fno-omit-frame-pointer" "-funwind-tables"
  "-DMOZILLA_CONFIG_H" "-xc" "-P" "-DWR_FRAGMENT_SHADER=1"
  "-E" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/cs_clip_rectangle.c"

This is tested without the CRATE_CC_NO_DEFAULTS environment variable.
Here we see the -ggdb2 (same as -ggdb) added in the mozconfig file using ac_add_options --enable-debug-symbols=-ggdb2.

Comment 16

[Nicolas B. Pierron [:nbp]]

Testing with the patch and export CRATE_CC_NO_DEFAULTS=true in the mozconfig behaves the same way as in comment 15.

Comment 17

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3de8645714dc
Disable default flags for SWGL preprocessor. r=jrmuizel

Comment 18

[Cristian Brindusan [:cbrindusan]]

https://hg.mozilla.org/mozilla-central/rev/3de8645714dc

Comment 19

[Nicolas B. Pierron [:nbp]]

Based on comment 15 and comment 16, I do not think this bug is resolved.
However, I do understand that this is not a priority given that it can be worked around.

Comment 20

[Jeff Muizelaar [:jrmuizel]]

Yeah, it looks like no_default_flags doesn't prevent CFLAGS/CXXFLAGS from being used.

Comment 21

[Lee Salzman [:lsalzman]]

Attachment: Bug 1698156 - Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r?jrmuizel

Comment 22

[Lee Salzman [:lsalzman]]

Nicolas, can you see if that SWGLPP workaround works for you?

Comment 23

[Jamie Nicol [:jnicol]]

FWIW, this causes an error when compiling the unoptimized shaders on Adreno 3xx. (Presumably a driver bug). How would we feel about replacing it with a function? Presumably the compiler would optimize it out.

Comment 24

[Jamie Nicol [:jnicol]]

To clarify, the antialias_brush macro causes the issue, not the workarounds

ERROR: 0:1287: '(' : Syntax error:  syntax error

Comment 25

[Jeff Muizelaar [:jrmuizel]]

Replacing it with a function should be fine. Usually we're getting optimized shaders anyways.

Comment 26

[Nicolas B. Pierron [:nbp]]

(In reply to Lee Salzman [:lsalzman] from comment #22)

Nicolas, can you see if that SWGLPP workaround works for you?

This seems to work!

"/nix/store/ca37d3qrydh0wpw40kswsx30j8dyzxh2-gcc-wrapper-10.2.0/bin/cc"
  "-std=gnu99" "-O2" "-ffunction-sections" "-fdata-sections"
  "-fPIC"
  "-xc"
  "-P"
  "-DWR_FRAGMENT_SHADER=1"
  "-E" "/home/nicolas/mozilla/_build/firefox/bugzil.la/1698045/wip/x64/gcc/opt/x86_64-unknown-linux-gnu/release/build/swgl-2a88ebe753c74f59/out/cs_clip_rectangle.c"

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38a9b0040e8f
Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r=jrmuizel

Comment 28

[Cristian Brindusan [:cbrindusan]]

Backed out changeset 38a9b0040e8f (Bug 1698156) for build bustages.
https://hg.mozilla.org/integration/autoland/rev/a6b4a0648b99f0c5bd528d482b60c60879dcc6be

Push with failures:
https://treeherder.mozilla.org/jobs?repo=autoland&revision=38a9b0040e8f2cddd1095363dc68a279efe93622&selectedTaskRun=BZeEr5ePS7alMqDzZRDIsA.0

Failure log:
https://treeherder.mozilla.org/logviewer?job_id=334312057&repo=autoland&lineNumber=45101

Comment 29

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/97c081de7aa5
Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r=jrmuizel

Comment 30

[Alexandru Michis [:malexandru]]

Backed out changeset 97c081de7aa5 (Bug 1698156) for causing bustages.
Backout link: https://hg.mozilla.org/integration/autoland/rev/0c0112e20d168ff6389aa47456f5b0aaf6008ff0
Push with failures, failure log.

Comment 31

[Lee Salzman [:lsalzman]]

Argh, there was an unfortunate interaction with our build system wherein overriding the target caused us to pick up the wrong CC var. I found a workaround to force the existing CC target though.

Comment 32

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/be9bb9b16fc0
Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r=jrmuizel

Comment 33

[Cosmin Sabou [:CosminS]]

Backed out for hazard failures.

Push with failures: https://treeherder.mozilla.org/jobs?repo=autoland&resultStatus=pending%2Crunning%2Csuccess%2Ctestfailed%2Cbusted%2Cexception%2Cretry%2Cusercancel&fromchange=be9bb9b16fc0ae155f56dca498cf373eac01bb2e&tochange=c9ec596a51aa9611910aa64128071c1a0a0cd601&searchStr=hazard&selectedTaskRun=MiooG_UURY6ozTGxzbWTwQ.0

Failure log: https://treeherder.mozilla.org/logviewer?job_id=334385781&repo=autoland

Backout link: https://hg.mozilla.org/integration/autoland/rev/c9ec596a51aa9611910aa64128071c1a0a0cd601

Comment 34

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/7781dd4fce4a
Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r=jrmuizel

Comment 35

[Sandor Molnar[:smolnar]]

Backed out for causing linux build bustages

Backout link: https://hg.mozilla.org/integration/autoland/rev/14dbcc65d5826b46fbb8da0a9be23fcede418f16

Push with failures

Failure log

Comment 36

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d9094761d8a9
Set up SWGLPP env target with null CFLAGS and CXXFLAGS. r=jrmuizel

Comment 37

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/d9094761d8a9

Comment 38

[Andra Esanu (needinfo me)]

(In reply to Sandor Molnar from comment #35)

Backed out for causing linux build bustages

Backout link: https://hg.mozilla.org/integration/autoland/rev/14dbcc65d5826b46fbb8da0a9be23fcede418f16

Push with failures

Failure log

== Change summary for alert #29513 (as of Tue, 30 Mar 2021 07:04:03 GMT) ==

Regressions:

Ratio	Suite	Test	Platform	Options	Absolute values (old vs new)
0.30%	Base Content JS		macosx1015-64-shippable-qr		2,407,737.67 -> 2,414,969.33
0.05%	Base Content JS		macosx1015-64-shippable-qr		2,407,750.00 -> 2,408,839.33

For up to date results, see: https://treeherder.mozilla.org/perfherder/alerts?id=29513