Closed Bug 1514592 Opened 6 years ago Closed 6 years ago

Non-SEH builds complain about RpcExceptionCode

Categories

(Core :: IPC: MSCOM, enhancement)

Product:
Core
Component:
IPC: MSCOM
Type:
enhancement
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla66
Tracking Flags:
Tracking Status
firefox-esr60 --- fixed
firefox66 --- fixed

People

(Reporter: away, Assigned: away)

References

Details

Attachments

(1 file, 1 obsolete file)

Bug 1514592: Don't call RpcExceptionCode if we don't HAVE_SEH_EXCEPTIONS
(deleted), patch
bugzilla
: review+
RyanVM
: approval-mozilla-esr60+
Details | Diff | Splinter Review
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

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

Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox66: --- → fixed
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.

Attachment

General

Creator:
Created:
Updated:
Size: