1320892 - Remove useless assertion (which is breaking the build with gcc 6.2-7.0 and --enable-warnings-as-errors)

Status: RESOLVED FIXED

(Core :: Graphics: CanvasWebGL, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

gcc 7.0 considers that MOZ_ASSERT(bool(&object)); in WebGLContext::ValidateObjectRef
is always going to be true.
In file included from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:18:0,
                 from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/CheckedInt.h:13,
                 from /root/firefox-gcc-last/dom/canvas/WebGLContext.h:14,
                 from /root/firefox-gcc-last/dom/canvas/WebGL2Context.h:9,
                 from /root/firefox-gcc-last/dom/canvas/WebGL2ContextBuffers.cpp:6,
                 from /root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dom/canvas/Unified_cpp_dom_canvas1.cpp:2:
/root/firefox-gcc-last/dom/canvas/WebGLContext.h: In instantiation of 'bool mozilla::WebGLContext::ValidateObjectRef(const char*, const ObjectType&) [with ObjectType = mozilla::WebGLProgram]':
/root/firefox-gcc-last/dom/canvas/WebGL2ContextPrograms.cpp:22:64:   required from here
/root/firefox-gcc-last/dom/canvas/WebGLContext.h:2025:16: error: the compiler can assume that the address of 'object' will never be NULL [-Werror=address]
     MOZ_ASSERT(bool(&object));
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Likely.h:17:48: note: in definition of macro 'MOZ_UNLIKELY'
 #  define MOZ_UNLIKELY(x) (__builtin_expect(!!(x), 0))
                                                ^
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:377:23: note: in expansion of macro 'MOZ_CHECK_ASSERT_ASSIGNMENT'
     if (MOZ_UNLIKELY(!MOZ_CHECK_ASSERT_ASSIGNMENT(expr))) { \
                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:394:39: note: in expansion of macro 'MOZ_ASSERT_HELPER1'
 #define MOZ_RELEASE_ASSERT_GLUE(a, b) a b
                                       ^
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:396:3: note: in expansion of macro 'MOZ_RELEASE_ASSERT_GLUE'
   MOZ_RELEASE_ASSERT_GLUE( \
   ^~~~~~~~~~~~~~~~~~~~~~~
/root/firefox-gcc-last/obj-x86_64-pc-linux-gnu/dist/include/mozilla/Assertions.h:401:27: note: in expansion of macro 'MOZ_RELEASE_ASSERT'
 #  define MOZ_ASSERT(...) MOZ_RELEASE_ASSERT(__VA_ARGS__)
                           ^~~~~~~~~~~~~~~~~~
/root/firefox-gcc-last/dom/canvas/WebGLContext.h:2025:5: note: in expansion of macro 'MOZ_ASSERT'
     MOZ_ASSERT(bool(&object));
     ^

Comment 1

[Sylvestre Ledru [:Sylvestre]]

Attachment: Bug 1320892 - Remove useless assertion (which is breaking the build with gcc 7.0 and --enable-warnings-as-errors)

Review commit: https://reviewboard.mozilla.org/r/96242/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/96242/

Comment 2

[:Cykesiopka]

This also seems to affect GCC 6.2 as provided by Fedora 24.

Comment 3

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8815207 [details]
Bug 1320892 - Remove useless assertion (which is breaking the build with gcc 7.0 and --enable-warnings-as-errors)

https://reviewboard.mozilla.org/r/96242/#review96576

It's not useless, it's ineffective. This is a good sanity check to have, since it's quite easy to accidentally pass a null reference.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/616c1cdadade
Remove useless assertion (which is breaking the build with gcc 7.0 and --enable-warnings-as-errors) r=jgilbert

Comment 5

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/616c1cdadade

Comment 6

[Kelsey Gilbert [:jgilbert]]

Comment on attachment 8815207 [details]
Bug 1320892 - Remove useless assertion (which is breaking the build with gcc 7.0 and --enable-warnings-as-errors)

Approval Request Comment
[Feature/Bug causing the regression]: webgl2
[User impact if declined]:
[Is this code covered by automated tests?]:
[Has the fix been verified in Nightly?]:
[Needs manual test from QE? If yes, steps to reproduce]: 
[List of other uplifts needed for the feature/fix]:
[Is the change risky?]:
[Why is the change risky/not risky?]:
[String changes made/needed]:

Comment 7

[Gerry Chang [:gchang]]

Comment on attachment 8815207 [details]
Bug 1320892 - Remove useless assertion (which is breaking the build with gcc 7.0 and --enable-warnings-as-errors)

WebGL2 related patch. Beta51+ & Aurora52+. Should be in 51 beta 8.

Comment 8

[Carsten Book [:Tomcat]]

needs rebasing for aurora

Comment 9

[Sylvestre Ledru [:Sylvestre]]

Sure but, Jeff, could you explain why you need it before I spend time on it?

Comment 10

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/01baa08ef776

Comment 11

[Kelsey Gilbert [:jgilbert]]

https://hg.mozilla.org/releases/mozilla-beta/rev/cac3f56905a6