1514592 - Non-SEH builds complain about RpcExceptionCode

Status: RESOLVED FIXED

(Core :: IPC: MSCOM, enhancement)

Description

[(Away)]

When building with clang-cl with HAVE_SEH_EXCEPTIONS disabled (since clang-cl doesn't yet support them for aarch64), the compiler complains that RpcExceptionCode can't be called outside of an __except block.

Comment 1

[(Away)]

Attachment: Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Comment on attachment 9031748 [details] [diff] [review]
Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS

Review of attachment 9031748 [details] [diff] [review]:
-----------------------------------------------------------------

Sorry, I changed my mind on this. When HAVE_SEH_EXCEPTIONS is not defined, can you please assign RPC_X_INVALID_BUFFER to mStatus before returning, thus guaranteeing an error state?

Comment 3

[(No longer employed by Mozilla) Aaron Klotz]

(In reply to Aaron Klotz [:aklotz] from comment #2)
> Comment on attachment 9031748 [details] [diff] [review]
> Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS
> 
> Review of attachment 9031748 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Sorry, I changed my mind on this. When HAVE_SEH_EXCEPTIONS is not defined,
> can you please assign RPC_X_INVALID_BUFFER to mStatus before returning, thus
> guaranteeing an error state?

(In the StructToStream case, at least. In the StructFromStream case, we return false which should be enough.)

Comment 4

[Jacek Caban]

(In reply to Aaron Klotz [:aklotz] from comment #2)
> Sorry, I changed my mind on this. When HAVE_SEH_EXCEPTIONS is not defined,
> can you please assign RPC_X_INVALID_BUFFER to mStatus before returning, thus
> guaranteeing an error state?

Note that if HAVE_SEH_EXCEPTIONS is not defined, except block will never be actually executed, so it doesn't really matter (in a patch fixing the same problem in bug 9024304 I #ifdefed the whole except block to make that more explicit).

Comment 5

[(Away)]

Attachment: Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS

Here's Jacek's suggestion, which I quite like. But if you'd still prefer that I go with your approach, let me know.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d14ab4c9b5b
Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS. r=aklotz

Comment 7

[Oana Pop-Rus]

https://hg.mozilla.org/mozilla-central/rev/0d14ab4c9b5b

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 9034198 [details] [diff] [review]
Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for Tor compilation

User impact if declined: Tor will have to carry the patch

Fix Landed on Version: 66.0a1 / 20190108101613

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): Only affects non-standard (mingw) Windows builds

String or UUID changes made by this patch:

Comment 9

[Mike de Boer [:mikedeboer]]

https://hg.mozilla.org/projects/cedar/rev/0d14ab4c9b5b31c4466252774a68539320f46f52
Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS. r=aklotz

Comment 10

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9034198 [details] [diff] [review]
Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS

Tor compilation fix which is NPOTB for official Firefox builds. Approved for 60.5.0esr.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-esr60/rev/a09b2a626b9c