Bug 281132 – [GTK][WPE] GCC build error with fno-delete-null-pointer-checks

• Status: NEW
• Component: WebKitGTK

[Miguel Gomez]

The build is currently failing when using gcc and passing the -fno-delete-null-pointer-checks flag. I've tested gcc11 and 12, but it seems to affect all versions due to this gcc bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97913.

This causes 2 build errors, one in ANGLE:

In file included from /host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/Framebuffer.h:24,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/Context.h:32,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/renderer/gl/SemaphoreGL.cpp:9:
/host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/State.h: In member function ‘angle::Result gl::State::dirtyObjectHandler(size_t, const gl::Context*, gl::Command)’:
/host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/State.h:1538:84:   in ‘constexpr’ expansion of ‘gl::State::MakeDirtyObjectHandlers()’
/host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/State.h:1527:25: error: ‘(gl::State::syncActiveTextures == 0)’ is not a constant expression
 1527 |             if (handler == nullptr)
      |                 ~~~~~~~~^~~~~~~~~~
/host/home/magomez/webkit/WebKit/Source/ThirdParty/ANGLE/src/libANGLE/State.h:1539:35: error: non-constant condition for static assertion
 1539 |         static_assert(handlers[0] != nullptr, "MakeDirtyObjectHandlers missing a handler");



and another one in libwebrtc:

In file included from /host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/raw_hash_set.h:213,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/raw_hash_map.h:26,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/flat_hash_map.h:42,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/flags/reflection.h:29,
                 from /host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/flags/reflection.cc:16:
/host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/raw_hash_set.h: In instantiation of ‘static const absl::container_internal::PolicyFunctions& absl::container_internal::raw_hash_set<Policy, Hash, Eq, Alloc>::GetPolicyFunctions() [with Policy = absl::container_internal::FlatHashMapPolicy<std::basic_string_view<char>, absl::CommandLineFlag*>; Hash = absl::container_internal::StringHash; Eq = absl::container_internal::StringEq; Alloc = std::allocator<std::pair<const std::basic_string_view<char>, absl::CommandLineFlag*> >]’:
/host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/raw_hash_set.h:2846:53:   required from ‘void absl::container_internal::raw_hash_set<Policy, Hash, Eq, Alloc>::clear() [with Policy = absl::container_internal::FlatHashMapPolicy<std::basic_string_view<char>, absl::CommandLineFlag*>; Hash = absl::container_internal::StringHash; Eq = absl::container_internal::StringEq; Alloc = std::allocator<std::pair<const std::basic_string_view<char>, absl::CommandLineFlag*> >]’
/host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/flags/reflection.cc:212:24:   required from here
/host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/raw_hash_set.h:4033:56:   in ‘constexpr’ expansion of ‘absl::container_internal::hash_policy_traits<absl::container_internal::FlatHashMapPolicy<std::basic_string_view<char>, absl::CommandLineFlag*>, void>::get_hash_slot_fn<absl::container_internal::StringHash>()’
/host/home/magomez/webkit/WebKit/Source/ThirdParty/libwebrtc/Source/third_party/abseil-cpp/absl/container/internal/hash_policy_traits.h:163:54: error: ‘(absl::container_internal::TypeErasedApplyToSlotFn<StringHash, std::basic_string_view<char> > == 0)’ is not a constant expression
  163 |     return Policy::template get_hash_slot_fn<Hash>() == nullptr
      |            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~


As in both cases the code is not ours, I'm not sure what to do here. I have a patch to avoid those errors with basically means disabling fno-delete-null-pointer-checks for the problematic functions, but I don't think we want this upstream. Not sure we want to report this to the ANGLE or libwebrtc devs, as this is a gcc bug, not their problem. I'll attach the patch here and we can discuss what to do, if we want to do anything about it.

[Miguel Gomez]

Created attachment 472876 [details]
Patch to avoid the problems

[Michael Catanzaro]

Why are you using this flag, though?

I doubt you really need this flag, but if you do, surely that indicates a WebKit bug to be fixed.

[Miguel Gomez]

(In reply to Michael Catanzaro from comment #2)
> Why are you using this flag, though?
> 
> I doubt you really need this flag, but if you do, surely that indicates a
> WebKit bug to be fixed.

I don't know exactly why it's used, but I guess it's useful when using address sanitizers, as the flag is added in WebKitCompilerFlags.cmake when the enable the "undefined" sanitizer.

[Michael Catanzaro]

I was surprised because Linux distros don't use this flag, but I see https://github.com/ossf/wg-best-practices-os-developers/blob/main/docs/Compiler-Hardening-Guides/Compiler-Options-Hardening-Guide-for-C-and-C%2B%2B.md#do-not-delete-null-pointer-checks recommends using it simply as a generic hardening measure. OK, seems reasonable to do so.

I wonder if this would work or not:

if constexpr (handler == nullptr)