Skip to content

Use explicit arithmetic checks in the amd64 watchOS simulator.#20639

Merged
imhameed merged 1 commit intomono:masterfrom
imhameed:gh-mono-20632-dont-use-sigfpe-for-watchos
Dec 9, 2020
Merged

Use explicit arithmetic checks in the amd64 watchOS simulator.#20639
imhameed merged 1 commit intomono:masterfrom
imhameed:gh-mono-20632-dont-use-sigfpe-for-watchos

Conversation

@imhameed
Copy link
Contributor

@imhameed imhameed commented Dec 8, 2020 •
edited
Loading

Fixes #20632.

@monojenkins monojenkins mentioned this pull request Dec 8, 2020
Merged
@dalexsoto
Copy link
Member

dalexsoto commented Dec 8, 2020

Thanks a lot @imhameed! Can we get this back ported to 2020-02?

@imhameed
Copy link
Contributor Author

imhameed commented Dec 8, 2020

Yeah; I'll do that once this makes it through. (I haven't actually reproduced the failure or verified that the fix actually works yet, so I'll need a little time to roll my iDevice setup forward.)

@dalexsoto
Copy link
Member

dalexsoto commented Dec 8, 2020

Gotcha, thanks! Let me know if I can help somehow

@SamMonoRT
Copy link
Contributor

SamMonoRT commented Dec 8, 2020

Any idea why we didn't catch this earlier ? @dalexsoto

@lambdageek
Copy link
Member

lambdageek commented Dec 8, 2020

@SamMonoRT for the 32-bit simulator it didn't happen because MONO_ARCH_NEED_DIV_CHECK is defined unconditionally

mono/mono/mini/mini-x86.h

Line 194 in 0e3d75d

#define MONO_ARCH_NEED_DIV_CHECK 1

@dalexsoto
Copy link
Member

dalexsoto commented Dec 8, 2020

@SamMonoRT Because this is new for WatchOS 7.0+ and I did not catch it here #20553 because this was merged in xamarin-macios in a way that the bcl test triggered for a previous commit that did not contain this fix so we caught it on a subsequent commit

@imhameed
Copy link
Contributor Author

imhameed commented Dec 9, 2020

@monojenkins build failed

@dalexsoto
Copy link
Member

dalexsoto commented Dec 9, 2020

@imhameed any chance we can have this change pushed into a no shipping branch inside mono repo so the CI generates the binaries? this way I can help to test the change

@imhameed imhameed merged commit bcd4791 into mono:master Dec 9, 2020
@imhameed
Copy link
Contributor Author

imhameed commented Dec 9, 2020

@monojenkins backport to 2020-02

@monojenkins monojenkins mentioned this pull request Dec 9, 2020
Merged
imhameed added a commit that referenced this pull request Dec 10, 2020
@monojenkins @imhameed
Use explicit arithmetic checks in the amd64 watchOS simulator. (#20647)
5e9cb6d 
Fixes #20632.

Backport of #20639.

Co-authored-by: Imran Hameed <imhameed@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SamMonoRT SamMonoRT Awaiting requested review from SamMonoRT

2 more reviewers

@vargaz vargaz vargaz approved these changes

@lambdageek lambdageek lambdageek approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

SIGFPE signal is not being handled when running on x86_64 WatchOS 7.0+ Simulators

5 participants

@imhameed @dalexsoto @SamMonoRT @lambdageek @vargaz