[Runtime] Move some of the changes back to int. by mandel-macaque · Pull Request #7529 · dotnet/macios

mandel-macaque · 2019-12-02T20:42:15Z

Some of the fixes done for the warnings have breaking changes. Move back
to int and ensure that we do not have any compilation errors (we are
using -Werror).

Fixes: #7509

Some of the fixes done for the warnings have breaking changes. Move back to int and ensure that we do not have any compilation errors (we are using -Werror). Fixes: dotnet#7509

spouliot · 2019-12-02T21:06:48Z

 				*(NSObject **) arg = obj;
 			} else {
-				exception_gchandle = xamarin_get_exception_for_parameter (8030, 0, "Unable to marshal the out/ref parameter", sel, method, p, i, false);
+				exception_gchandle = xamarin_get_exception_for_parameter (8030, 0, "Unable to marshal the out/ref parameter", sel, method, p, (int) i, false);


would not this be simpler (less changes) if int i was used ?

Surprisingly, no.

monojenkins · 2019-12-02T23:01:05Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

1 tests failed, 87 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

rolfbjarne · 2019-12-03T13:26:05Z

https://github.com/xamarin/maccore/issues/2072 is still happening, so this is not complete. My first guess would be that it's the changes in trampolines-arm64.m that is causing https://github.com/xamarin/maccore/issues/2072.

monojenkins · 2019-12-03T22:00:15Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

1 tests failed, 87 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

mandel-macaque · 2019-12-03T23:41:25Z

@rolfbjarne these results do look more promising.

rolfbjarne · 2019-12-04T14:59:13Z

@mandel-macaque so that didn't fix https://github.com/xamarin/maccore/issues/2072 😒

…p on the canary.

monojenkins · 2019-12-10T21:31:42Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

3 tests failed, 85 tests passed.

Failed tests

monotouch-test/iOS Unified 64-bits - simulator/Debug: Failed
monotouch-test/watchOS 32-bits - simulator/Debug: Crashed
monotouch-test/watchOS 32-bits - simulator/Release (all optimizations): TimedOut

…t to step on the canary." This reverts commit a40c128.

monojenkins · 2019-12-11T21:38:29Z

Build failure
✅ Build succeeded
✅ API Diff (from stable)
✅ API Diff (from PR only) (no change)
✅ Generator Diff (no change)
🔥 Test run failed 🔥

Test results

1 tests failed, 87 tests passed.

Failed tests

monotouch-test/watchOS 32-bits - simulator/Debug: Crashed

spouliot · 2019-12-13T23:00:47Z

@monojenkins backport to d16-5

monojenkins · 2019-12-13T23:01:18Z

@spouliot backporting to d16-5 failed, the patch results in conflicts:

Applying: [Runtime] Move some of the changes back to int.
Using index info to reconstruct a base tree...
M	runtime/delegates.t4
M	runtime/monotouch-debug.h
M	runtime/monotouch-debug.m
M	runtime/monotouch-main.m
M	runtime/runtime.m
M	runtime/trampolines-invoke.m
M	runtime/trampolines.m
M	runtime/xamarin-support.m
M	runtime/xamarin/runtime.h
M	runtime/xamarin/trampolines.h
M	src/ObjCRuntime/Runtime.cs
.git/rebase-apply/patch:262: trailing whitespace.
int 
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging src/ObjCRuntime/Runtime.cs
Auto-merging runtime/xamarin/trampolines.h
Auto-merging runtime/xamarin/runtime.h
Auto-merging runtime/xamarin-support.m
Auto-merging runtime/trampolines.m
Auto-merging runtime/trampolines-invoke.m
Auto-merging runtime/runtime.m
Auto-merging runtime/monotouch-main.m
Auto-merging runtime/monotouch-debug.m
Auto-merging runtime/monotouch-debug.h
Auto-merging runtime/delegates.t4
Applying: Undo changes in the trampolines arm64 code.
Applying: Remove stack protection until the assembly/code is updated not to step on the canary.
Using index info to reconstruct a base tree...
M	Make.config
Falling back to patching base and 3-way merge...
Auto-merging Make.config
CONFLICT (content): Merge conflict in Make.config
error: Failed to merge in the changes.
Patch failed at 0003 Remove stack protection until the assembly/code is updated not to step on the canary.

Please backport manually!

Some of the fixes done for the warnings have breaking changes. Move back to int and ensure that we do not have any compilation errors (we are using -Werror). Fixes: dotnet#7509

Some of the fixes done for the warnings have breaking changes. Move back to int and ensure that we do not have any compilation errors (we are using -Werror). Fixes: #7509

[Runtime] Move some of the changes back to int.

352a48e

Some of the fixes done for the warnings have breaking changes. Move back to int and ensure that we do not have any compilation errors (we are using -Werror). Fixes: dotnet#7509

mandel-macaque requested a review from rolfbjarne as a code owner December 2, 2019 20:42

VincentDondain approved these changes Dec 2, 2019

View reviewed changes

rolfbjarne approved these changes Dec 2, 2019

View reviewed changes

spouliot approved these changes Dec 2, 2019

View reviewed changes

ghost approved these changes Dec 2, 2019

View reviewed changes

Undo changes in the trampolines arm64 code.

2f29e7e

stephen-hawley approved these changes Dec 4, 2019

View reviewed changes

Remove stack protection until the assembly/code is updated not to ste…

a40c128

…p on the canary.

mandel-macaque mentioned this pull request Dec 10, 2019

[Runtime] ARM64 trampolines crashed with the fstack-protector-strong flag enabled. #7564

Closed

mandel-macaque added 2 commits December 11, 2019 13:44

Merge branch 'master' into undo-long

3e2ba69

Revert "Remove stack protection until the assembly/code is updated no…

0d7b9c9

…t to step on the canary." This reverts commit a40c128.

spouliot approved these changes Dec 12, 2019

View reviewed changes

mandel-macaque merged commit 3f0985e into dotnet:master Dec 12, 2019

Uh oh!

Conversation

mandel-macaque commented Dec 2, 2019

Uh oh!

spouliot Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

mandel-macaque Dec 2, 2019

Choose a reason for hiding this comment

Uh oh!

monojenkins commented Dec 2, 2019

Test results

Failed tests

Uh oh!

rolfbjarne commented Dec 3, 2019

Uh oh!

monojenkins commented Dec 3, 2019

Test results

Failed tests

Uh oh!

mandel-macaque commented Dec 3, 2019

Uh oh!

rolfbjarne commented Dec 4, 2019

Uh oh!

monojenkins commented Dec 10, 2019

Test results

Failed tests

Uh oh!

monojenkins commented Dec 11, 2019

Test results

Failed tests

Uh oh!

spouliot commented Dec 13, 2019

Uh oh!

monojenkins commented Dec 13, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants