Skip to content

Incorrect up-sizing for non-power-of-2 structs that are not LclVars #10833

@adityamandaleeka

Description

@adityamandaleeka

When loading a struct from an arbitrary location, it is not safe to load any more bytes than the specified size of the struct. For instance, in the case where you're loading a 3-byte struct that's in the last three bytes of a page, the next byte following the struct may be on a page that is unmapped or not readable.

A JIT dump showed this transformation during the morph process (note the OBJ(3) turns into an IND int):

fgMorphTree BB02, stmt 23 (before)
               [000124] --CXG-------              *  CALL      void   Test.Foo
               [000121] ------------ this in x0   +--*  LCL_VAR   ref    V00 this         
               [000123] ---XG------- arg1         \--*  OBJ(3)    struct
               [000122] ------------                 \--*  LCL_VAR   long   V05 loc4         
argSlots=2, preallocatedArgCount=0, nextSlotNum=0, outgoingArgSpaceSize=0

Sorting the arguments:
Deferred argument ('x1'):
               [000123] ---XG+------              *  IND       int   
               [000122] -----+------              \--*  LCL_VAR   long   V05 loc4         
Replaced with placeholder node:
               [000186] ----------L-              *  ARGPLACE  int   
Deferred argument ('x0'):
               [000121] -----+------              *  LCL_VAR   ref    V00 this         
Replaced with placeholder node:
               [000188] ----------L-              *  ARGPLACE  ref   

Debugging fgMorphArgs shows that we’re taking this codepath which is changing the size from 3 to 4:

#if defined(_TARGET_ARM64_) || defined(UNIX_AMD64_ABI)
                    // For ARM64 or AMD64/UX we can pass non-power-of-2 structs in a register.
                    if ((howToPassStruct == SPK_PrimitiveType) && // Passed in a single register
                        !isPow2(originalSize))                    // size is 3,5,6 or 7 bytes
                    {
                        originalSize = genTypeSize(structBaseType);
                    }
#endif //  _TARGET_ARM64_ || UNIX_AMD64_ABI

This branch was modified a couple of months ago in dotnet/coreclr#18358 to remove an additional check in there that was also checking if the source was a LclVar before adjusting the size.

@CarolEidt Can you please take a look and determine whether we need to reintroduce that check or make a better fix?

Metadata

Metadata

Assignees

Labels

arch-arm64arch-x64area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions