-
Notifications
You must be signed in to change notification settings - Fork 5k
Write barrier without any RWX pages #114982
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
bfd57cd
02b9ea6
273b640
17ad508
4669265
a24a67e
a858993
1c4c79a
999b466
547fb96
9fd2921
9c2e247
4601ff8
ff8a695
a882fbf
f5a07a7
45a866b
516656e
2b791c6
ecc06a7
e8c9a34
e992b86
db5d24f
f3f9320
c35cbbd
0fa2885
89422b6
fd2cc83
4dd7420
5ab9e76
9467e25
4e1ee53
3383a68
6ca2479
435c220
2950b1f
247ae5f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
// This file is used to allow sharing of assembly code between NativeAOT and CoreCLR, which have different conventions about how to ensure that constants offsets are accessible | ||
|
||
#include "AsmOffsets.inc" | ||
#include <unixasmmacros.inc> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
// This file is used to allow sharing of assembly code between NativeAOT and CoreCLR, which have different conventions about how to ensure that constants offsets are accessible | ||
|
||
// TODO: Implement |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,6 @@ | ||
; Licensed to the .NET Foundation under one or more agreements. | ||
; The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
; This file is used to allow sharing of assembly code between NativeAOT and CoreCLR, which have different conventions about how to ensure that constants offsets are accessible | ||
|
||
include AsmMacros.inc |
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
// This file is used to allow sharing of assembly code between NativeAOT and CoreCLR, which have different conventions about how to ensure that constants offsets are accessible | ||
|
||
#include <unixasmmacros.inc> | ||
#include "AsmOffsets.inc" |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
// Licensed to the .NET Foundation under one or more agreements. | ||
// The .NET Foundation licenses this file to you under the MIT license. | ||
|
||
// This file is used to allow sharing of assembly code between NativeAOT and CoreCLR, which have different conventions about how to ensure that constants offsets are accessible | ||
|
||
#include <unixasmmacros.inc> | ||
#include "AsmOffsets.inc" |
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -2,7 +2,15 @@ | |||||||||||||||||
// The .NET Foundation licenses this file to you under the MIT license. | ||||||||||||||||||
|
||||||||||||||||||
.intel_syntax noprefix | ||||||||||||||||||
#include <unixasmmacros.inc> | ||||||||||||||||||
#include "AsmMacros_Shared.h" | ||||||||||||||||||
|
||||||||||||||||||
#if defined(__APPLE__) | ||||||||||||||||||
// Currently the build is failing without this due to an issue if the first method in the assembly file has an alternate entry at the start of the file. | ||||||||||||||||||
// Fix, but adding an empty, unused method | ||||||||||||||||||
LEAF_ENTRY RhpWriteBarriersDoNotFailToBuild, _TEXT | ||||||||||||||||||
ret | ||||||||||||||||||
LEAF_END RhpWriteBarriersDoNotFailToBuild, _TEXT | ||||||||||||||||||
#endif | ||||||||||||||||||
|
||||||||||||||||||
#ifdef WRITE_BARRIER_CHECK | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -261,7 +269,15 @@ LEAF_END RhpCheckedXchg, _TEXT | |||||||||||||||||
LEAF_ENTRY RhpByRefAssignRef, _TEXT | ||||||||||||||||||
ALTERNATE_ENTRY RhpByRefAssignRefAVLocation1 | ||||||||||||||||||
mov rcx, [rsi] | ||||||||||||||||||
#ifdef TARGET_APPLE | ||||||||||||||||||
// Apple's linker has issues which break unwind info if | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the manifestation of the bad unwind info? If it is just when single stepping in lldb, do we care? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The manifestation is that if we AV on this location, we are unable to unwind to the managed frame, so we failfast the process instead of producing a NullReferenceException which can be caught. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be better to use the same manual uwnind routine in regular CoreCLR as we use in NAOT to avoid this problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is another approach where we don't use libunwind for unwinding these entries, instead we use our own unwinder which is special cased to know what to do. (This is what NativeAot does.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (Unlike NativeAOT, CoreCLR implements the Apple compact unwinding through a modified copy of the llvm-libunwind code; libunwind should not even come into play for this since it's covered by the compact codes.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I was doing fixes of the osx-arm64 NativeAOT port I found a bug in llvm-libunwind comparison of the boundary comparison for IP addresses and function start/end (cb6fb60#diff-fa7a2ccbf98ccd844bfc21e48f2bc700137c2903cf97d487fa011e16b1f9b3c0). I looked at the custom unwinding code in CoreCLR and there are similar comparisons that look sketchy: runtime/src/coreclr/pal/src/exception/remote-unwind.cpp Lines 1034 to 1037 in c201ad1
runtime/src/coreclr/pal/src/exception/remote-unwind.cpp Lines 1064 to 1067 in c201ad1
(in both cases the condition should presumably be ip >= funcEnd , not ip > funcEnd ; shouldn't really cause an error for alt_entry though..)
|
||||||||||||||||||
// an ALTERNATE_ENTRY is present in the middle of a function see https://github.com/dotnet/runtime/pull/114982#discussion_r2083272768 | ||||||||||||||||||
.cfi_endproc | ||||||||||||||||||
#endif | ||||||||||||||||||
ALTERNATE_ENTRY RhpByRefAssignRefAVLocation2 | ||||||||||||||||||
#ifdef TARGET_APPLE | ||||||||||||||||||
.cfi_startproc | ||||||||||||||||||
#endif | ||||||||||||||||||
mov [rdi], rcx | ||||||||||||||||||
|
||||||||||||||||||
// Check whether the writes were even into the heap. If not there's no card update required. | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this not failing in NativeAOT build today?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that I look at it, this probably isn't necessary in the amd64 write barrier since it wasn't complaining before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This leads me to wonder if maybe the issue on MacOS arm64 with having an alternate entry as the first symbol in the object file was that since our alternate entry macro doesn't set the thumb bit, the offset for the alternate entry was before that of the normal function symbol for the first symbol in the object file. Time for an experiment. Sigh.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thumb bit is only relevant for arm32 so that should not matter here.
Also, the issue with first symbol in object file is known ld64 bug, see #114982 (comment).