NEW271337
Static assertions in generate-serializers.py assume RefCounted and ThreadSafeRefCounted are the same size, but this is not true on macOS debug builds
https://bugs.webkit.org/show_bug.cgi?id=271337
Summary Static assertions in generate-serializers.py assume RefCounted and ThreadSafe...
Michael Catanzaro
Reported 2024-03-20 14:02:47 PDT
Currently GeneratedSerializers.cpp assumes that RefCounted and ThreadSafeRefCounted are both the same size. Arguably it shouldn't, since this surely relies on implementation-defined behavior, but it apparently works in practice. If it's not true, then the build will fail on one of various static assertions in GeneratedSerializers.cpp because VirtualTableAndRefCountOverhead inherits from RefCounted<VirtualTableAndRefCountOverhead> and gets compared to the size of ThreadSafeRefCounted objects. Since it's easy to test for, let's add more static asserts to catch this problem specifically. This way, if the assumption fails in the future, the sad developer who has to deal with it will hopefully be moderately less confused. Let's also adjust the preprocessor guards in ThreadSafeRefCounted.h to match RefCounted.h. The size of both classes depends on whether debug assertions are enabled, but ThreadSafeRefCounted is directly checking NDEBUG rather than checking ASSERT_ENABLED, so they could easily wind up enabled in one place and not the other. This would likely cause the size checks in GeneratedSerializers.cpp to fail.
Attachments
Workaround for this bug (1.23 KB, patch)
2024-06-22 06:05 PDT, contact
no flags
Michael Catanzaro
Comment 1 2024-03-20 14:07:54 PDT
Radar WebKit Bug Importer
Comment 2 2024-03-27 14:03:13 PDT
Michael Catanzaro
Comment 3 2024-04-01 14:34:43 PDT
So it turns out we cannot actually assert this because it's not true for macOS debug builds. That is, the size of the virtual table and ref count overhead is unfortunately different from sizeof(VirtualTableAndRefCountOverhead). However, this seems to cause no problems in practice (as otherwise it would fail to build). My guess is that, in all cases where this difference might have mattered, the size of the derived class object just happens to match the expected size by coincidence, probably due to padding bytes. A proper solution would be to modify generate-serializers.py to track RefCounted objects separately from ThreadSafeRefCounted objects, and have separate static assertions for each. This is surely better than forcing RefCounted and ThreadSafeRefCounted to be the same size anyway. But I'm not planning to spend time on this myself.
contact
Comment 4 2024-06-22 06:05:03 PDT
Created attachment 471727 [details] Workaround for this bug I hit this bug while helping troubleshoot an embedded WPE build. I've attached a workaround patch.
Note You need to log in before you can comment on or make changes to this bug.