JIT: verify containment safety before forming address modes#65048
JIT: verify containment safety before forming address modes#65048AndyAyersMS merged 1 commit intodotnet:mainfrom
Conversation
At least on arm64, where LEAs must be contained. Fixes dotnet#64936.
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsAt least on arm64, where LEAs must be contained. Fixes #64936.
|
|
@EgorBo PTAL My local jit-format runs insist this is how things should format. Seems odd. |
|
/azp run runtime-coreclr libraries-jitstress |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| #else | ||
| const bool isContainable = true; | ||
| #endif | ||
| TryCreateAddrMode(ind->Addr(), isContainable, ind); |
There was a problem hiding this comment.
TryCreateAddrMode is called with true from other places too so I wonder if those need this check as well (or maybe TryCreateAddrMode should just check it inside by itself
There was a problem hiding this comment.
We should be doing the "right" safety checks later when we actually mark the node contained.
So the issue is whether the target can handle a non-contained LEA. arm64 can't handle a non-contained LEA but xarch/arm32 can.
|
Handful of diffs, all regressions (not surprisingly). Many look to be gratuitous, perhaps spurious flags. ;; libraries.pmi
;; Internal.TypeSystem.Interop.ArrayMarshaller:GetElementMarshaller(int):Internal.TypeSystem.Interop.Marshaller:this
;; BEFORE
ldr w14, [x19,#96]
str w14, [x0,#88]
;; AFTER
add x14, x0, #88
ldr w15, [x19,#96]
str w15, [x14]
;; BEFORE
ldrb w14, [x19,#110]
strb w14, [x0,#110]
;; AFTER
add x14, x0, #110
ldrb w15, [x19,#110]
strb w15, [x14]
;; Number:NumberToString(byref,byref,ushort,int,System.Globalization.NumberFormatInfo,bool)
;; BEFORE
ldrh w1, [x1,#12]
strh w1, [x2, w0, UXTW #2]
;; AFTER
ubfiz x3, x0, #1, #32
add x2, x2, x3
ldrh w1, [x1,#12]
strh w1, [x2]Let me look at a few of these in more detail. |
|
Not clear to me if the failure this issue is addressing is still happening. Looks like it is: https://dev.azure.com/dnceng/public/_build/results?buildId=1602009&view=results |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Outerloop had two failures -- one looks like an OOM while jitting that we see from time to time (crossgen2, x86, hugeexpr1). The other looks like a flaky CI issue. Retrying the latter. |
At least on arm64, where LEAs must be contained.
Fixes #64936.