Bug 241516 – -Wmismatched-new-delete warning spam from CSSStyleValue.h

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Michael Catanzaro]

Unfortunately since this is a header file, it triggers a massive warning spam that exceeds my terminal scrollback. 
Sadly, I won't notice other compiler warnings until it is fixed:

In destructor ‘virtual WebCore::CSSStyleValue::~CSSStyleValue()’,
    inlined from ‘void std::default_delete<_Tp>::operator()(_Tp*) const [with _Tp = WebCore::CSSStyleValue]’ at /usr/include/c++/12/bits/unique_ptr.h:95:2,
    inlined from ‘void WTF::RefCounted<T, Deleter>::deref() const [with T = WebCore::CSSStyleValue; Deleter = std::default_delete<WebCore::CSSStyleValue>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounted.h:190:22,
    inlined from ‘void WTF::RefCounted<T, Deleter>::deref() const [with T = WebCore::CSSStyleValue; Deleter = std::default_delete<WebCore::CSSStyleValue>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/RefCounted.h:187:10,
    inlined from ‘WTF::Ref<T, <template-parameter-1-2> >::~Ref() [with T = WebCore::CSSUnitValue; Traits = WTF::RawPtrTraits<WebCore::CSSUnitValue>]’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WTF/Headers/wtf/Ref.h:61:23,
    inlined from ‘JSC::EncodedJSValue WebCore::jsDOMCSSNamespaceConstructorFunction_chBody(JSC::JSGlobalObject*, JSC::CallFrame*)’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDOMCSSNamespace.cpp:1015:5,
    inlined from ‘static JSC::EncodedJSValue WebCore::IDLOperation<JSClass>::callStatic(JSC::JSGlobalObject&, JSC::CallFrame&, const char*) [with JSC::EncodedJSValue (* operation)(JSC::JSGlobalObject*, JSC::CallFrame*) = WebCore::jsDOMCSSNamespaceConstructorFunction_chBody; WebCore::CastedThisErrorBehavior shouldThrow = WebCore::CastedThisErrorBehavior::Throw; JSClass = WebCore::JSDOMCSSNamespace]’ at /home/mcatanzaro/Projects/WebKit/Source/WebCore/bindings/js/JSDOMOperation.h:70:25,
    inlined from ‘JSC::EncodedJSValue WebCore::jsDOMCSSNamespaceConstructorFunction_ch(JSC::JSGlobalObject*, JSC::CallFrame*)’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDOMCSSNamespace.cpp:1020:100:
/home/mcatanzaro/Projects/WebKit/Source/WebCore/css/typedom/CSSStyleValue.h:113:13: warning: ‘static void WebCore::CSSStyleValue::operator delete(void*)’ called on pointer returned from a mismatched allocation function [-Wmismatched-new-delete]
  113 |     virtual ~CSSStyleValue() = default;
      |             ^
In static member function ‘static WTF::Ref<WebCore::CSSUnitValue> WebCore::CSSUnitValue::create(double, WebCore::CSSUnitType)’,
    inlined from ‘static WTF::Ref<WebCore::CSSUnitValue> WebCore::CSSNumericFactory::ch(double)’ at /home/mcatanzaro/Projects/WebKit/Source/WebCore/css/typedom/CSSNumericFactory.h:54:104,
    inlined from ‘JSC::EncodedJSValue WebCore::jsDOMCSSNamespaceConstructorFunction_chBody(JSC::JSGlobalObject*, JSC::CallFrame*)’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDOMCSSNamespace.cpp:1015:5,
    inlined from ‘static JSC::EncodedJSValue WebCore::IDLOperation<JSClass>::callStatic(JSC::JSGlobalObject&, JSC::CallFrame&, const char*) [with JSC::EncodedJSValue (* operation)(JSC::JSGlobalObject*, JSC::CallFrame*) = WebCore::jsDOMCSSNamespaceConstructorFunction_chBody; WebCore::CastedThisErrorBehavior shouldThrow = WebCore::CastedThisErrorBehavior::Throw; JSClass = WebCore::JSDOMCSSNamespace]’ at /home/mcatanzaro/Projects/WebKit/Source/WebCore/bindings/js/JSDOMOperation.h:70:25,
    inlined from ‘JSC::EncodedJSValue WebCore::jsDOMCSSNamespaceConstructorFunction_ch(JSC::JSGlobalObject*, JSC::CallFrame*)’ at /home/mcatanzaro/Projects/WebKit/WebKitBuild/GNOME-gtk3/WebCore/DerivedSources/JSDOMCSSNamespace.cpp:1020:100:
/home/mcatanzaro/Projects/WebKit/Source/WebCore/css/typedom/CSSUnitValue.h:43:116: note: returned from ‘static void* WebCore::CSSUnitValue::operator new(size_t)’
   43 |     static Ref<CSSUnitValue> create(double value, CSSUnitType unit) { return adoptRef(*new CSSUnitValue(value, unit)); }
      |                                                                                                                    ^

I'm frustrated that I do not see the problem. I think it must be somehow related to the WTF_MAKE_ISO_ALLOCATED, but I'm bamboozled as to what exactly is wrong. I wouldn't be surprised if GCC is just totally confused.

[Michael Catanzaro]

I found a fix, but it makes no sense:

diff --git a/Source/WebCore/css/typedom/CSSStyleValue.h b/Source/WebCore/css/typedom/CSSStyleValue.h
index da615424d5c4..72f580a5facf 100644
--- a/Source/WebCore/css/typedom/CSSStyleValue.h
+++ b/Source/WebCore/css/typedom/CSSStyleValue.h
@@ -110,7 +110,10 @@ class CSSStyleValue : public RefCounted<CSSStyleValue>, public ScriptWrappable {
 public:
     String toString() const;
     virtual void serialize(StringBuilder&, OptionSet<SerializationArguments> = { }) const;
-    virtual ~CSSStyleValue() = default;
+    virtual ~CSSStyleValue()
+    {
+        m_customPropertyName = { };
+    }
 
     virtual CSSStyleValueType getType() const { return CSSStyleValueType::CSSStyleValue; }
 

If anybody knows a logical reason why this should make any difference....

[Michael Catanzaro]

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

[Radar WebKit Bug Importer]

<rdar://problem/95401608>

[EWS]

Committed r295692 (251697@main): <https://commits.webkit.org/251697@main>

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

[Lauro Moura]

I'm getting these locally and they are happening on the bots too:

https://build.webkit.org/#/builders/41/builds/13326

Maybe related to this upstream gcc <12 issue (closed as wontfix): "False positive in -Wmismatched-new-delete" https://gcc.gnu.org/bugzilla/show_bug.cgi?id=100485

On why it might still be generating the warning:

>  Alas, keeping the diagnostic ignore local by means of `diagnostic push` / `diagnostic pop` makes it not work correctly, presumably since the warning is generated at the call site.  Making it global to everything including the header file seems like a bad idea to me.

About the bug closure as wontfix:

>  That does sound frustrating, sorry.  The #pragma has a limitation that can make it difficult to use in inlining contexts.  I submitted a patch for it for GCC 11 but it got pushed to GCC 12 (see pr98465 comment 32).  Until it's fixed, the only other solution I can think of is to prevent the member operator delete from inlining using attribute noinline.

I tried moving the ignore guard to the call site at CSSUnitValue.h (where the new call happens), but it didn't work. Adding noinline to the destructor though seems to have made the warning disappear, but not sure if this would be an acceptable approach for a compiler warning.

In any case, I did not get the warning with gcc-12, as the upstream bug mentions. For the bots, it should be supported once we migrate to the FDO SDK 22.08.