Bug 188598 – [JSC] Remove gcc warnings on mips and armv7

• Component: JavaScriptCore
• Status: RESOLVED DUPLICATE

[Guillaume Emont]

This is a follow up from #187803. We still have many warnings on armv7 and mips, mostly -Wcast-align.

[Guillaume Emont]

Created attachment 347156 [details]
Patch

This patch addresses the warnings. See ChangeLogs and comments for explanations. I left out an alignment warning in Source/ThirdParty/capstone/Source/cs.c as I'm not sure how to address issues in ThirdParty/.

[EWS Watchlist]

Comment on attachment 347156 [details]
Patch

Attachment 347156 [details] did not pass jsc-ews (mac):
Output: https://webkit-queues.webkit.org/results/8866207

New failing tests:
stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no-cjit
apiTests

[Guillaume Emont]

(In reply to Build Bot from comment #2)
> Comment on attachment 347156 [details]
> Patch
> 
> Attachment 347156 [details] did not pass jsc-ews (mac):
> Output: https://webkit-queues.webkit.org/results/8866207
> 
> New failing tests:
> stress/ftl-put-by-id-setter-exception-interesting-live-state.js.ftl-eager-no-
> cjit
> apiTests

Not sure what platform is jsc-ews, but I suspect it's neither mips nor armv7. Given the changes in the file, this looks a lot like a false positive here.

[Guillaume Emont]

Created attachment 347180 [details]
Patch

New version that should fix compilation on compilers other than gcc/clang.

[Michael Catanzaro]

Comment on attachment 347180 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347180&action=review

I'm not qualified to review the uses of this macro, but here are my thoughts on the macro itself:

> Source/JavaScriptCore/runtime/JSBigInt.cpp:1318
> +// offsetOfData() makes sure that its return value is aligned to the size of
> +// Digit, so even though we cast to char* for pointer arithmetics, the cast to
> +// Digit* is properly aligned, though the compiler doesn't know about it,
> +// therefore we disable this warning.

This should be aligned four spaces to the right.

> Source/WTF/wtf/Compiler.h:400
> +#if !defined(IGNORE_WARNING) && COMPILER(GCC_OR_CLANG)
> +#define _WEBKIT_IGNORE_WARNING_STRINGIFY(a) #a
> +#define IGNORE_WARNING(warning) do {\
> +    _Pragma("GCC diagnostic push") \
> +    _Pragma(_WEBKIT_IGNORE_WARNING_STRINGIFY(GCC diagnostic ignored warning)) \
> +    } while (false)
> +
> +#define IGNORE_WARNING_END do {\
> +    _Pragma("GCC diagnostic pop") \
> +    } while (false)
> +#endif

Wow, very nice. You should use this to reduce all the manual use of #pragma GCC diagnostic and #pragma clang diagnostic throughout the project.

Unfortunately, I think you are going to need two separate definitions here for Clang and GCC, IGNORE_WARNING_GCC and IGNORE_WARNING_CLANG, or you'll just wind up introducing new warnings about unknown pragmas when the wrong compiler is used. At least I'm pretty sure GCC complains if it doesn't recognize the warning, and it definitely complains if it sees #pragma clang.

[Guillaume Emont]

(In reply to Michael Catanzaro from comment #5)
> Comment on attachment 347180 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347180&action=review
> 
> I'm not qualified to review the uses of this macro, but here are my thoughts
> on the macro itself:
> 
> > Source/JavaScriptCore/runtime/JSBigInt.cpp:1318
> > +// offsetOfData() makes sure that its return value is aligned to the size of
> > +// Digit, so even though we cast to char* for pointer arithmetics, the cast to
> > +// Digit* is properly aligned, though the compiler doesn't know about it,
> > +// therefore we disable this warning.
> 
> This should be aligned four spaces to the right.
Right, sorry for the oversight.

> 
> > Source/WTF/wtf/Compiler.h:400
> > +#if !defined(IGNORE_WARNING) && COMPILER(GCC_OR_CLANG)
> > +#define _WEBKIT_IGNORE_WARNING_STRINGIFY(a) #a
> > +#define IGNORE_WARNING(warning) do {\
> > +    _Pragma("GCC diagnostic push") \
> > +    _Pragma(_WEBKIT_IGNORE_WARNING_STRINGIFY(GCC diagnostic ignored warning)) \
> > +    } while (false)
> > +
> > +#define IGNORE_WARNING_END do {\
> > +    _Pragma("GCC diagnostic pop") \
> > +    } while (false)
> > +#endif
> 
> Wow, very nice. You should use this to reduce all the manual use of #pragma
> GCC diagnostic and #pragma clang diagnostic throughout the project.
I agree. I also think that should be a separate patch, WDYT?

> 
> Unfortunately, I think you are going to need two separate definitions here
> for Clang and GCC, IGNORE_WARNING_GCC and IGNORE_WARNING_CLANG, or you'll
> just wind up introducing new warnings about unknown pragmas when the wrong
> compiler is used. At least I'm pretty sure GCC complains if it doesn't
> recognize the warning, and it definitely complains if it sees #pragma clang.

Apparently, clang is OK with at least some gcc-formatted pragma ignores. For proof, the bots are happy with the ones in this patch (though they might not be necessary), there is also older existing code that does that (see e.g. JavaScriptCore/assembler/LinkBuffer.h), so I think it's not a problem for clang. Yet grepping through the code, there are ignores that are gcc specific or clang specific, so my proposal would be to have:
 - IGNORE_WARNING_GCC / IGNORE_WARNING_GCC_END, with same definition as IGNORE_WARNING(_END) in my patch but only defined for GCC.
 - IGNORE_WARNING_CLANG / IGNORE_WARNING_CLANG_END, with clang pragmas, only defined for clang
 - IGNORE_WARNING_GCC_OR_CLANG / IGNORE_WARNING_GCC_OR_CLANG_END with same definition as IGNORE_WARNING(_END), defined for both clang and GCC

We might be able to live without the third one if we can prove that clang doesn't get confused if we use both the GCC and CLANG macros together.

[Adrian Perez]

Comment on attachment 347180 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347180&action=review

This is a nice patch, and I like it that you have added comments documenting
why it's safe to disable the compiler warning for the tricky cases. (Informal r+!)

> Source/JavaScriptCore/ChangeLog:28
> +        Exception * cast to a JSObject *. It is therefore safe to cast it back

Probabl your editor has messed a bit here with the star signs “*” used
as bullets and it may have reflowed the text without removing them, and
now there's too many inline stars in this paragraph.

Please remove the excess stars :-)

>>> Source/WTF/wtf/Compiler.h:400
>>> +#endif
>> 
>> Wow, very nice. You should use this to reduce all the manual use of #pragma GCC diagnostic and #pragma clang diagnostic throughout the project.
>> 
>> Unfortunately, I think you are going to need two separate definitions here for Clang and GCC, IGNORE_WARNING_GCC and IGNORE_WARNING_CLANG, or you'll just wind up introducing new warnings about unknown pragmas when the wrong compiler is used. At least I'm pretty sure GCC complains if it doesn't recognize the warning, and it definitely complains if it sees #pragma clang.
> 
> I agree. I also think that should be a separate patch, WDYT?

I would be fine with this being solved in a follow-up patch, just don't
forget about it ;-)

[Michael Catanzaro]

I agree with splitting the macros out to a separate patch, since that's of general interest to many WebKit developers. I would try to get more feedback on it too, e.g. on webkit-dev@.

Remember clang will accept GCC pragmas, but not vice-versa.

I think clang will warn if it sees a GCC-specific warning (not positive) or a warning specific to an older version of clang (also not positive).

GCC will definitely warn if it sees a clang pragma, or a clang-specific warning, or even a warning specific to older versions of GCC.

[Guillaume Emont]

Comment on attachment 347180 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=347180&action=review

>> Source/JavaScriptCore/ChangeLog:28
>> +        Exception * cast to a JSObject *. It is therefore safe to cast it back
> 
> Probabl your editor has messed a bit here with the star signs “*” used
> as bullets and it may have reflowed the text without removing them, and
> now there's too many inline stars in this paragraph.
> 
> Please remove the excess stars :-)

These stars are here on purpose and have a meaning as I am talking about pointer types "Exception *" and "JSObject *".

These are not the excess stars you are looking for.

[Guillaume Emont]

(In reply to Michael Catanzaro from comment #8)
> I agree with splitting the macros out to a separate patch, since that's of
> general interest to many WebKit developers. I would try to get more feedback
> on it too, e.g. on webkit-dev@.
> 
> Remember clang will accept GCC pragmas, but not vice-versa.

Sure.

> 
> I think clang will warn if it sees a GCC-specific warning (not positive) or
> a warning specific to an older version of clang (also not positive).
> 
> GCC will definitely warn if it sees a clang pragma, or a clang-specific
> warning, or even a warning specific to older versions of GCC.


Do you have a specific change to my proposal to recommend in the light of your two assertions?

In spite of this, nobody seems to be complaining about the existing pragmas in the code, so I think the cases where these complain are either rare or remaining outside of our current use, or the complaints are with versions nobody cares about maybe? Anyway, if I add the macros I proposed previously and replace the manual warning disables in the code, that should be a no-op. Then it's up to future users to use the macro carefully, just as when you use pragmas directly.

[Michael Catanzaro]

(In reply to Guillaume Emont from comment #10) 
> Do you have a specific change to my proposal to recommend in the light of
> your two assertions?

Nope.

> In spite of this, nobody seems to be complaining about the existing pragmas
> in the code, so I think the cases where these complain are either rare or
> remaining outside of our current use, or the complaints are with versions
> nobody cares about maybe? Anyway, if I add the macros I proposed previously
> and replace the manual warning disables in the code, that should be a no-op.
> Then it's up to future users to use the macro carefully, just as when you
> use pragmas directly.

Yes, if your macros can replace the existing #pragma usage without introducing warnings, then they're good IMO.

[Adrian Perez]

(In reply to Guillaume Emont from comment #9)
> Comment on attachment 347180 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=347180&action=review
> 
> >> Source/JavaScriptCore/ChangeLog:28
> >> +        Exception * cast to a JSObject *. It is therefore safe to cast it back
> > 
> > Probabl your editor has messed a bit here with the star signs “*” used
> > as bullets and it may have reflowed the text without removing them, and
> > now there's too many inline stars in this paragraph.
> > 
> > Please remove the excess stars :-)
> 
> These stars are here on purpose and have a meaning as I am talking about
> pointer types "Exception *" and "JSObject *".
> 
> These are not the excess stars you are looking for.

Wow, now that you mention it… you're right, but it's terribly hard to
read. I would either write “Exception* cast to a JSObject*” (no spaces
after the type name, so the star is part of the type name — same as it
is written in the code), or even “This is a Exception pointer cast to
a JSObject pointer“.

[Guillaume Emont]

Created attachment 348927 [details]
Patch

New version of the patch making use of the macros introduced in the patch for #188996. Won't work without that patch previously applied.

[Mark Lam]

Comment on attachment 348927 [details]
Patch

Please fix all the build failures.

[Guillaume Emont]

(In reply to Mark Lam from comment #14)
> Comment on attachment 348927 [details]
> Patch
> 
> Please fix all the build failures.

As said in my previous comment, the patch depends on the patch of #188996 being applied (I'm actually surprised the patch applies on ToT at all), though I am providing it to allow for early discussion. I'll provide a rebased patch once the fix of #188996 is landed, or an alternative if for some reason we don't want the approach proposed there.

[Mark Lam]

(In reply to Guillaume Emont from comment #15)
> (In reply to Mark Lam from comment #14)
> > Comment on attachment 348927 [details]
> > Patch
> > 
> > Please fix all the build failures.
> 
> As said in my previous comment, the patch depends on the patch of #188996
> being applied (I'm actually surprised the patch applies on ToT at all),
> though I am providing it to allow for early discussion. I'll provide a
> rebased patch once the fix of #188996 is landed, or an alternative if for
> some reason we don't want the approach proposed there.

ok.

[Guillaume Emont]

Created attachment 349562 [details]
Patch

New version rebased after landing #188996.

[Guillaume Emont]

Created attachment 349756 [details]
Patch

New version fixing bmalloc/ChangeLog entry

[Mark Lam]

Comment on attachment 349756 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=349756&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp:140
> +        // assuming memory is not malformed, it originately pointed to a value

/originately/originally/

> Source/JavaScriptCore/interpreter/Interpreter.cpp:814
> +        IGNORE_CAST_ALIGN_WARNINGS_BEGIN
>          EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
> +        IGNORE_CAST_ALIGN_WARNINGS_END

Instead of doing this, can you just change prepareForExecution() to return a std::optional<Exception*>?  That should make the code cleaner and better document what will actually be returned.  That also removes the need for all these reinterpret_casts.  I think the JSObject* return value is a holdover from ancient times.

> Source/bmalloc/bmalloc/IsoPageInlines.h:197
> +        // the cast below is safe on these platforms as long as
> +        // both Config::objectSize and the alignment of this are multiples of
> +        // the required alignment of FreeCell
> +        static_assert(!(Config::objectSize % alignof(FreeCell)), "Config::objectSize should respect alignment of FreeCell");
> +        static_assert(!(alignof(IsoPage<Config>) % alignof(FreeCell)), "Alignment of IsoPage<Config> should match that of FreeCell");

Shouldn't these be always true?  I think we can get rid of the #if CPU(ARM) || CPU(MIPS) here and below.  You can also remove the comment.  The assertions are sufficient to document the invariant / requirement.

[Guillaume Emont]

(In reply to Mark Lam from comment #19)
> Comment on attachment 349756 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=349756&action=review
> 
> > Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp:140
> > +        // assuming memory is not malformed, it originately pointed to a value
> 
> /originately/originally/

Sorry, I don't know why, I keep on convincing myself that this is a word ;).

> 
> > Source/JavaScriptCore/interpreter/Interpreter.cpp:814
> > +        IGNORE_CAST_ALIGN_WARNINGS_BEGIN
> >          EXCEPTION_ASSERT(throwScope.exception() == reinterpret_cast<Exception*>(error));
> > +        IGNORE_CAST_ALIGN_WARNINGS_END
> 
> Instead of doing this, can you just change prepareForExecution() to return a
> std::optional<Exception*>?  That should make the code cleaner and better
> document what will actually be returned.  That also removes the need for all
> these reinterpret_casts.  I think the JSObject* return value is a holdover
> from ancient times.

I'd be happy to do so. I did not do it *originally* because I assumed (wrongly I guess, from your request) that there was a reason-I-don't-understand for prepareForExecution() to be that way and didn't want to risk breaking something.

> 
> > Source/bmalloc/bmalloc/IsoPageInlines.h:197
> > +        // the cast below is safe on these platforms as long as
> > +        // both Config::objectSize and the alignment of this are multiples of
> > +        // the required alignment of FreeCell
> > +        static_assert(!(Config::objectSize % alignof(FreeCell)), "Config::objectSize should respect alignment of FreeCell");
> > +        static_assert(!(alignof(IsoPage<Config>) % alignof(FreeCell)), "Alignment of IsoPage<Config> should match that of FreeCell");
> 
> Shouldn't these be always true?  I think we can get rid of the #if CPU(ARM)
> || CPU(MIPS) here and below.  You can also remove the comment.  The
> assertions are sufficient to document the invariant / requirement.

Right. I will do that.

[Guillaume Emont]

Created attachment 350104 [details]
Patch

New version addressing Mark's comments.

[Guillaume Emont]

(In reply to Guillaume Emont from comment #21)
> Created attachment 350104 [details]
> Patch
> 
> New version addressing Mark's comments.

One thing of which I'm not entirely certain is whether I should just change prepareForExecution(), which I did because it felt like the minimal change; or whether we should make prepareForExecutionImpl() return an std::optional<Exception*> as well.

[Guillaume Emont]

Ping reviewer

[Yusuke Suzuki]

Why not using bitwise_cast<> instead of reinterpret_cast<>?

[Mark Lam]

Comment on attachment 350104 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=350104&action=review

Looks good but let's fix the ScriptExecutable::prepareForExecutionImpl() part as well.

> Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp:141
> +        // of the required size, which should be properly aligned on platforms

nit: I would say "which should already be"

> Source/JavaScriptCore/bytecode/CodeBlock.h:1064
> +    JSObject* exception = prepareForExecutionImpl(vm, function, scope, kind, resultCodeBlock);
> +    if (exception) {
> +        IGNORE_CAST_ALIGN_WARNINGS_BEGIN
> +        return std::optional<Exception *>(reinterpret_cast<Exception*>(exception));
> +        IGNORE_CAST_ALIGN_WARNINGS_END
> +    } else
> +        return std::nullopt;

Let's just make ScriptExecutable::prepareForExecutionImpl() return a std::optional<Exception*> as well.  I don't see a good reason for doing this half way.

[Guillaume Emont]

Created attachment 351282 [details]
Patch

New version: change return type of prepareForExecutionImpl() as well, use bitwise_cast in it, and address the comment in MacroAssemblerPrinter.cpp.

[Mark Lam]

Comment on attachment 351282 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351282&action=review

r=me.  The patch looks correct though I think it would be better if we use static_casts in instead of bitwise_casts.

> Source/JavaScriptCore/runtime/ScriptExecutable.cpp:349
> -        return throwException(&state, throwScope, createError(&state, "Forced Failure"_s));
> +        return std::optional<Exception *>(bitwise_cast<Exception*>(throwException(&state, throwScope, createError(&state, "Forced Failure"_s))));

I actually disagree with using bitwise_cast here because we're casting from a pointer to a pointer.  Hence, they are guaranteed to be of the same bit length.  In fact, I think a static cast would be more meaningful because it enforces that there's a "is a" relationship between the source (JSObject*) and the result (Exception*).  The times when we should use bitwise_cast are when casting between the bit value of non-pointer types (e.g. integers) and pointers.

Are you sure that this is where Yusuke is asking to apply the bitwise_cast?

> Source/JavaScriptCore/runtime/ScriptExecutable.cpp:357
> +        return std::optional<Exception *>(bitwise_cast<Exception*>(exception));

Ditto.

[Mark Lam]

Comment on attachment 351282 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351282&action=review

> Source/JavaScriptCore/assembler/MacroAssemblerPrinter.cpp:145
>              auto p = reinterpret_cast<int16_t*>(ptr);

I think this is the place that Yusuke is suggesting that the bitwise_cast be used.  In so doing, I think you can do away with the IGNORE_CAST_ALIGN_WARNINGS_BEGIN.

[Guillaume Emont]

Created attachment 351289 [details]
Patch

Use bitwise_cast in printMemory() and static_cast in prepareForExecutionImpl

[Mark Lam]

Comment on attachment 351289 [details]
Patch

r- for now.  This patch cane be improved i.e. there's no need for the IGNORE_CAST_ALIGN_WARNINGS macros at all.

[Mark Lam]

@Guillaume, I want you to understand why bitwise_cast was suggested as an alternative to IGNORE_CAST_ALIGN_WARNINGS: using bitwise_cast means we want the code to just treat the pointer bits as the new type, and that the author already knows that the bits will work correctly as the new type.  This is why we don't need to disable the warnings.

However, you will find that bmalloc (which sits below WTF) will have no knowledge of bitwise_cast yet (which is defined in WTF).  bmalloc needs its own copy of bitwise_cast.  I suggest copying the bitwise_cast template to bamlloc's Algorithm.h and then using it in the code instead of the IGNORE_CAST_ALIGN_WARNINGS macro.

[Guillaume Emont]

Created attachment 351974 [details]
Patch

Patch reworked without using IGNORE_CAST_ALIGN_WARNINGS.* and instead focusing on using the appropriate cast in each case.

[EWS Watchlist]

Attachment 351974 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/Algorithm.h:189:  bitwise_cast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 351974 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=351974&action=review

r=me if EWS bots are green.

> Source/bmalloc/bmalloc/Algorithm.h:186
> + * Copied from WTF/wtf/StdLibExtras.h

No need to use C style comments.  Can you switch this to a C++ comment?

[Guillaume Emont]

Created attachment 352124 [details]
Patch

Patch for landing.

[WebKit Commit Bot]

Comment on attachment 352124 [details]
Patch

Rejecting attachment 352124 [details] from commit-queue.

Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'validate-changelog', '--check-oops', '--non-interactive', 352124, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit

ChangeLog entry in Source/JavaScriptCore/ChangeLog contains OOPS!.

Full output: https://webkit-queues.webkit.org/results/9545323

[EWS Watchlist]

Attachment 352124 [details] did not pass style-queue:


ERROR: Source/bmalloc/bmalloc/Algorithm.h:187:  bitwise_cast is incorrectly named. Don't use underscores in your identifier names.  [readability/naming/underscores] [4]
Total errors found: 1 in 15 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Mark Lam]

Comment on attachment 352124 [details]
Patch

Next time, when landing after a review, you should fill in the name of the reviewer manually before cq+ing.

[Mark Lam]

(In reply to Mark Lam from comment #38)
> Comment on attachment 352124 [details]
> Patch
> 
> Next time, when landing after a review, you should fill in the name of the
> reviewer manually before cq+ing.

I mean if the patch doesn't have a r+ from a reviewer.  I took care of it for you this time.

[Guillaume Emont]

(In reply to Mark Lam from comment #38)
> Comment on attachment 352124 [details]
> Patch
> 
> Next time, when landing after a review, you should fill in the name of the
> reviewer manually before cq+ing.

Oops. Sorry about that.

[WebKit Commit Bot]

Comment on attachment 352124 [details]
Patch

Clearing flags on attachment: 352124

Committed r237063: <https://trac.webkit.org/changeset/237063>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/45215159>

[Ryan Haddad]

(In reply to WebKit Commit Bot from comment #41)
> Committed r237063: <https://trac.webkit.org/changeset/237063>

This change appears to have caused layout test fast/dom/Window/window-postmessage-clone-deep-array.html to fail on macOS Debug WK1:

--- /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/fast/dom/Window/window-postmessage-clone-deep-array-expected.txt
+++ /Volumes/Data/slave/sierra-debug-tests-wk1/build/layout-test-results/fast/dom/Window/window-postmessage-clone-deep-array-actual.txt
@@ -1,5 +1,5 @@
+CONSOLE MESSAGE: line 34: RangeError: Maximum call stack size exceeded.
 Tests that we support cloning deep(ish) arrays.
 
-PASS: eventData is [object Array](default toString threw RangeError: Maximum call stack size exceeded.) of type object
 PASS: eventData is done of type string
 

https://build.webkit.org/results/Apple%20Sierra%20Debug%20WK1%20(Tests)/r237063%20(9929)/results.html

[Ryan Haddad]

(In reply to Ryan Haddad from comment #44)
> (In reply to WebKit Commit Bot from comment #41)
> > Committed r237063: <https://trac.webkit.org/changeset/237063>
> 
> This change appears to have caused layout test
> fast/dom/Window/window-postmessage-clone-deep-array.html to fail on macOS
> Debug WK1

Correction: It also affects macOS and iOS Debug WK2 configurations.

https://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=fast%2Fdom%2FWindow%2Fwindow-postmessage-clone-deep-array.html

[Ryan Haddad]

Reverted r237063 for reason:

Caused layout test fast/dom/Window/window-postmessage-clone-deep-array.html to fail on macOS and iOS Debug bots.

Committed r237080: <https://trac.webkit.org/changeset/237080>

[Guillaume Emont]

*** This bug has been marked as a duplicate of bug 195049 ***