Bug 296043 – REGRESSION(296858@main): Build fails with ENABLE_JOURNALD_LOG=OFF due to static assert in logging macros

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Adrian Perez]

Trying to build with Clang 18.0.1, in particular the version included with the Android NDK r27, results in all release logging statements to produce errors due to failed static assertions through SAFE_FPRINTF, so it's caused by 296858@main. They all more or less look like the following:

In file included from /home/aperez/devel/WebKit-Android/Source/WebCore/PAL/pal/text/TextEncodingRegistry.cpp:27:
In file included from /home/aperez/devel/WebKit-Android/Source/WebCore/PAL/config.h:36:
In file included from /home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/FastMalloc.h:26:
In file included from /home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/MallocCommon.h:29:
/home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/StdLibExtras.h:1183:19: error: static assertion failed due to requirement '!std::is_same_v<char, char>': char* is not bounds safe; please use a null terminated string type
 1183 |     static_assert(!std::is_same_v<std::remove_cv_t<std::remove_pointer_t<T>>, char>, "char* is not bounds safe; please use a null terminated string type");
      |                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/aperez/devel/WebKit-Android/Source/WebCore/PAL/pal/text/TextEncodingRegistry.cpp:299:9: note: in instantiation of function template specialization 'WTF::safePrintfType<const char *>' requested here
  299 |         RELEASE_LOG_ERROR(TextEncoding, "Trying to create new text codec with invalid (null) encoding name. Will default to UTF-8.");
      |         ^
/home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/Assertions.h:745:41: note: expanded from macro 'RELEASE_LOG_ERROR'
  745 | #define RELEASE_LOG_ERROR(channel, ...) LOGF(channel, 1, __VA_ARGS__)
      |                                         ^
/home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/Assertions.h:741:9: note: expanded from macro 'LOGF'
  741 |         SAFE_FPRINTF(stderr, "[" LOG_CHANNEL_WEBKIT_SUBSYSTEM ":%s:%i] " fmt "\n", logChannel.name, priority, ##__VA_ARGS__); \
      |         ^
/home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/StdLibExtras.h:1203:39: note: expanded from macro 'SAFE_FPRINTF'
 1203 |     fprintf(file, format __VA_OPT__(, SAFE_PRINTF_TYPE(__VA_ARGS__))) \
      |                                       ^
/home/aperez/wpe/cerbero/build/sources/android_arm64/wpewebkit-2.48.3/b/WTF/Headers/wtf/StdLibExtras.h:1194:49: note: expanded from macro 'SAFE_PRINTF_TYPE'
 1194 | #define SAFE_PRINTF_TYPE(...) WTF_FOR_EACH(WTF::safePrintfType, __VA_ARGS__)
      |                                                 ^
1 error generated.

[Adrian Perez]

Happens also with using Clang 20.1, to reproduce one needs to make sure that -DENABLE_RELEASE_LOG=ON and -DENABLE_JOURNALD_LOG=OFF are set.

[Adrian Perez]

We may want to switch the logging macros to use plain fprintf(), to make sure it accepts (and the compiler checks) the same format specifiers as for __android_log_print()/sd_journal_send() and so on. The SAFE_FPRINTF() macro does some templating tricks to convert types to plain C strings so if we have different build options using code  paths that do different things to formatting arguments, either one build option or the other is going to be broken.

If we want to use SAFE_FPRINTF, we have to make the other logging implementations do the same checks and type conversions as well.

[Adrian Perez]

The issue can be reproduced with GCC as well.

[Adrian Perez]

After some pondering, I think the SAFE_FPRINTF() needs to be changed back to plain fprintf(), for the following reason:

- SAFE_FPRINTF() applies one level of conversion to certain types, to get a “const char*” out of them.
- But neither __android_log_print() nor sd_journal_send[_with_location]() apply the conversion.
- We need the three code paths (stderr logging, Android logging, and journald logging) to behave the same way, because the logging macros may expand to any of those.

So I think first we should revert to plain fprintf() when logging to stderr.

*Later*, if we want to move over to SAFE_FPRINTF() then we need to change the other versions of the logging macros to behave the same way as SAFE_FPRINTF().

[Adrian Perez]

Pull request: https://github.com/WebKit/WebKit/pull/48366

[Radar WebKit Bug Importer]

<rdar://problem/156477982>

[Adrian Perez]

*** Bug 296880 has been marked as a duplicate of this bug. ***

[EWS]

Committed 298228@main (00bafe7995cf): <https://commits.webkit.org/298228@main>

Reviewed commits have been landed. Closing PR #48366 and removing active labels.

[Philippe Normand]

In file included from /var/home/phil/WebKit/local-build-gtk/WebKitBuild/GTK/Release/WebCore/DerivedSources/unified-sources/UnifiedSource-aba958d6-3.cpp:7:
/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp:730:18: warning: address of function 'isAccessibilityLogChannelEnabled' will always evaluate to 'true' [-Wpointer-bool-conversion]
  730 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(isTableComponent(child) || isTableComponent(*this) || child.parentObject() == this)), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", "CODE_LINE=" "730", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", 730, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |              ~~  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp:730:18: note: prefix with the address-of operator to silence this warning
  730 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(isTableComponent(child) || isTableComponent(*this) || child.parentObject() == this)), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", "CODE_LINE=" "730", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", 730, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |                  ^
      |                  &
/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp:730:18: note: suffix with parentheses to turn this into a function call
  730 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(isTableComponent(child) || isTableComponent(*this) || child.parentObject() == this)), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", "CODE_LINE=" "730", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AccessibilityObject.cpp", 730, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |                  ^
      |                                                  ()
1 warning generated.

In file included from /var/home/phil/WebKit/local-build-gtk/WebKitBuild/GTK/Release/WebCore/DerivedSources/unified-sources/UnifiedSource-aba958d6-1.cpp:5:
/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp:1045:14: warning: address of function 'isAccessibilityLogChannelEnabled' will always evaluate to 'true' [-Wpointer-bool-conversion]
 1045 |     do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(!get(renderer))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", "CODE_LINE=" "1045", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", 1045, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |          ~~  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp:1045:14: note: prefix with the address-of operator to silence this warning
 1045 |     do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(!get(renderer))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", "CODE_LINE=" "1045", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", 1045, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |              ^
      |              &
/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp:1045:14: note: suffix with parentheses to turn this into a function call
 1045 |     do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(!get(renderer))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", "CODE_LINE=" "1045", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXObjectCache.cpp", 1045, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |              ^
      |                                              ()
In file included from /var/home/phil/WebKit/local-build-gtk/WebKitBuild/GTK/Release/WebCore/DerivedSources/unified-sources/UnifiedSource-aba958d6-1.cpp:7:
/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp:246:18: warning: address of function 'isAccessibilityLogChannelEnabled' will always evaluate to 'true' [-Wpointer-bool-conversion]
  246 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(is<AccessibilityObject>(startObject))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", "CODE_LINE=" "246", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", 246, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |              ~~  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp:246:18: note: prefix with the address-of operator to silence this warning
  246 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(is<AccessibilityObject>(startObject))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", "CODE_LINE=" "246", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", 246, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |                  ^
      |                  &
/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp:246:18: note: suffix with parentheses to turn this into a function call
  246 |         do { if (isAccessibilityLogChannelEnabled) { do { if (__builtin_expect(!!(!(is<AccessibilityObject>(startObject))), 0)) do { if (LogAccessibility.state != WTFLogChannelState::Off) sd_journal_send_with_location("CODE_FILE=" "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", "CODE_LINE=" "246", __func__, "WEBKIT_SUBSYSTEM=" "WebKitGTK", "WEBKIT_CHANNEL=%s", LogAccessibility.name, "PRIORITY=%i", 3, "MESSAGE=" "BROKEN ASSERTION FAILED in %s(%d) : %s\n", "/var/home/phil/WebKit/Source/WebCore/accessibility/AXSearchManager.cpp", 246, __PRETTY_FUNCTION__, nullptr); } while (0); } while (0); } } while (0);
      |                  ^
      |                                                  ()
2 warnings generated.

[Philippe Normand]

Sorry, these warnings are unrelated to this patch, my bad :)

[Adrian Perez]

(In reply to Philippe Normand from comment #10)
> Sorry, these warnings are unrelated to this patch, my bad :)

No problem =)