When LTO is enabled (using -flto=auto), GCC is able to report violations of C++'s one definition rule (ODR). Currently we have a huge spam of such warnings from WebCore caused by our ENABLE(BINDING_INTEGRITY) feature. CodeGeneratorJS.pm generates code that looks like this: extern "C" { extern void* ${vtableNameGnu}[]; } For example, JSTestGenerateIsReachable.cpp includes: extern "C" { extern void* _ZTVN7WebCore23TestGenerateIsReachableE[]; } It eventually gets used here: // If you hit this assertion you either have a use after free bug, or // ${implType} has subclasses. If ${implType} has subclasses that get passed // to toJS() we currently require $interfaceName you to opt out of binding hardening // by adding the SkipVTableValidation attribute to the interface IDL definition RELEASE_ASSERT(actualVTablePointer == expectedVTablePointer); This seems like a reasonable thing for us to do, and it does not seem like it should be an ODR violation because we are not actually defining the vtable symbol at all! We are merely declaring it. So I don't understand why GCC would be complaining about this. I'm tempted to consider this not a bug and suppress the warnings, but I want to check with the GCC developers first. Anyway, since this is caused by our code generation, there are separate warnings for each generated bindings file, which adds up to a nice spam. The first few look like this: ../../Source/WebCore/page/UserMessageHandlersNamespace.h:45: warning: virtual table of type ‘struct UserMessageHandlersNamespace’ violates one definition rule [-Wodr] 45 | class UserMessageHandlersNamespace : public RefCounted<UserMessageHandlersNamespace>, public FrameDestructionObserver, public UserContentProviderInvalidationClient { | WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp:270: note: variable of same assembler name as the virtual table is defined in another translation unit 270 | extern "C" { extern void* _ZTVN7WebCore28UserMessageHandlersNamespaceE[]; } | ../../Source/WebCore/page/UserMessageHandler.h:38: warning: virtual table of type ‘struct UserMessageHandler’ violates one definition rule [-Wodr] 38 | class UserMessageHandler : public RefCounted<UserMessageHandler>, public FrameDestructionObserver { | WebCore/DerivedSources/JSUserMessageHandler.cpp:251: note: variable of same assembler name as the virtual table is defined in another translation unit 251 | extern "C" { extern void* _ZTVN7WebCore18UserMessageHandlerE[]; } | ../../Source/WebCore/Modules/gamepad/GamepadEvent.h:36: warning: virtual table of type ‘struct GamepadEvent’ violates one definition rule [-Wodr] 36 | class GamepadEvent final : public Event { | WebCore/DerivedSources/JSGamepadEvent.cpp:304: note: variable of same assembler name as the virtual table is defined in another translation unit 304 | extern "C" { extern void* _ZTVN7WebCore12GamepadEventE[]; } | ../../Source/WebCore/xml/XMLHttpRequest.h:55: warning: virtual table of type ‘struct XMLHttpRequest’ violates one definition rule [-Wodr] 55 | class XMLHttpRequest final : public ActiveDOMObject, public RefCounted<XMLHttpRequest>, private ThreadableLoaderClient, public XMLHttpRequestEventTarget { | WebCore/DerivedSources/JSXMLHttpRequest.cpp:819: note: variable of same assembler name as the virtual table is defined in another translation unit 819 | extern "C" { extern void* _ZTVN7WebCore14XMLHttpRequestE[]; } | ../../Source/WebCore/xml/XMLHttpRequestUpload.h:33: warning: virtual table of type ‘struct XMLHttpRequestUpload’ violates one definition rule [-Wodr] 33 | class XMLHttpRequestUpload final : public XMLHttpRequestEventTarget { | WebCore/DerivedSources/JSXMLHttpRequestUpload.cpp:213: note: variable of same assembler name as the virtual table is defined in another translation unit 213 | extern "C" { extern void* _ZTVN7WebCore20XMLHttpRequestUploadE[]; } |
I'm going to attach the preprocessed source for the two translation units that trigger this issue for the vtable of class UserMessageHandlersNamespace. We'll probably wind up needing a GCC bug report, but I don't have a Bugzilla account there, so I'll start here.
Created attachment 437300 [details] Preprocessed sources for two affected translation units
(In reply to Michael Catanzaro from comment #2) > Created attachment 437300 [details] > Preprocessed sources for two affected translation units Amazingly, I messed up and included a local change in this build: a pragma intended to suppress these warnings. It's in UnifiedSource-3a52ce78-115.cpp.ii: # 270 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" #pragma GCC diagnostic push # 270 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" # 270 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" #pragma GCC diagnostic ignored "-W" "odr" # 270 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" extern "C" { extern void* _ZTVN7WebCore28UserMessageHandlersNamespaceE[]; } # 272 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" #pragma GCC diagnostic pop # 272 "WebCore/DerivedSources/JSUserMessageHandlersNamespace.cpp" Oops. I don't want to rebuild everything unless really needed, so best just manually delete the pragmas.
Created attachment 437536 [details] Preprocessed sources for two affected translation units Uploading fixed preprocessed sources that are unmodified, no pragmas added this time
I tried: diff --git a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm index 1f24f1eb010b..014a99f87b8b 100644 --- a/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm +++ b/Source/WebCore/bindings/scripts/CodeGeneratorJS.pm @@ -5106,7 +5106,9 @@ sub GenerateImplementation #pragma warning(disable: 4483) extern "C" { extern void (*const ${vtableRefWin}[])(); } #else +IGNORE_GCC_WARNINGS_BEGIN("odr") extern "C" { extern void* ${vtableNameGnu}[]; } +IGNORE_GCC_WARNINGS_END #endif #endif But apparently these warnings are not suppressible at all. :/
(In reply to Michael Catanzaro from comment #5) > But apparently these warnings are not suppressible at all. :/ Removing the generated files from unified build so that we can use -Wno-odr just for these files seems like a nonstarter to me. The only reasonable option seems to be to globally disable -Wodr, like I did for -Warray-bounds and -Wnonnull in bug #228601.
<rdar://problem/82975115>
I think expecting a GCC solution to this is unrealistic. Since we cannot suppress these warnings, our only option is to either suppress -Wodr globally or else disable the bindings integrity feature. Opinions welcome, but I'm inclined to disable -Wodr. It's sad that this will obscure real ODR bugs, though, which are inevitable in a codebase as large as WebKit's (in fact, I fixed several earlier this year only thanks to this warning).
Created attachment 443962 [details] Patch
Ping reviewers
Committed r290597 (?): <https://commits.webkit.org/r290597> All reviewed patches have been landed. Closing bug and clearing flags on attachment 443962 [details].