This is an archive of the discontinued LLVM Phabricator instance.

[ARM] Fix SJLJ exception handling when manually chosen on a platform where it isn't default
ClosedPublic

Authored by mstorsjo on Sep 25 2017, 1:00 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

mstorsjo created this revision.Sep 25 2017, 1:00 PM
compnerd requested changes to this revision.Sep 26 2017, 1:53 PM

@t.p.northover should absolutely look at this. The problem here is that the MCAsmInfo is sometimes not set up early enough which results in the exception model not being correct. We need to wire this up better so that the information is not pulled from MCAsmInfo but passed through properly.

This revision now requires changes to proceed.Sep 26 2017, 1:53 PM
mstorsjo updated this revision to Diff 116964.Sep 28 2017, 4:15 AM
mstorsjo edited edge metadata.

Reverted most of the logic back to the original form, but added a check of Options.ExceptionModel instead (which afaik always is available). If this is set to SjLj, we override UseSjLjEH, and if getMCAsmInfo() is available we verify that it matches just like before.

compnerd accepted this revision.Sep 28 2017, 8:25 AM
compnerd added inline comments.
lib/Target/ARM/ARMSubtarget.cpp
153–155 ↗(On Diff #116964)

I think that I would prefer:

UsesSjLjEH = (isTargetDarwin() && !isTargetWatchABI()) || Options.ExceptionModel == ExceptionHandling::SjLj;
This revision is now accepted and ready to land.Sep 28 2017, 8:25 AM
mstorsjo added inline comments.Sep 28 2017, 9:23 AM
lib/Target/ARM/ARMSubtarget.cpp
153–155 ↗(On Diff #116964)

Sounds good. Or perhaps even UsesSjLjEH = (isTargetDarwin() && !isTargetWatchABI() && Options.ExceptionModel == ExceptionHandling::None) || Options.ExceptionModel == ExceptionHandling::SjLj; to allow disabling it on iOS as well.

This revision was automatically updated to reflect the committed changes.