Closed Bug 1514592 Opened Last year Closed 11 months ago

Non-SEH builds complain about RpcExceptionCode

Categories

(Core :: IPC: MSCOM, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla66
Tracking Status
firefox-esr60 --- fixed
firefox66 --- fixed

People

(Reporter: dmajor, Assigned: dmajor)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Assignee: nobody → dmajor
Attachment #9031748 - Flags: review?(aklotz)
Attachment #9031748 - Flags: review?(aklotz) → review+
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?
Attachment #9031748 - Flags: review+ → review-
(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.)
(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).
Here's Jacek's suggestion, which I quite like. But if you'd still prefer that I go with your approach, let me know.
Attachment #9031748 - Attachment is obsolete: true
Attachment #9034198 - Flags: review?(aklotz)
Attachment #9034198 - Flags: review?(aklotz) → review+
Pushed by dmajor@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0d14ab4c9b5b
Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS. r=aklotz
Status: NEW → RESOLVED
Closed: 11 months ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla66

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:

Attachment #9034198 - Flags: approval-mozilla-esr60?
https://hg.mozilla.org/projects/cedar/rev/0d14ab4c9b5b31c4466252774a68539320f46f52
Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS. r=aklotz

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.

Attachment #9034198 - Flags: approval-mozilla-esr60? → approval-mozilla-esr60+
You need to log in before you can comment on or make changes to this bug.