Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Aug 13, 2025

This PR fixes undefined behavior caused by misaligned memory writes in the CoreCLR dynamic helper code generation for both i386 and amd64 architectures.

Problem

The code in src/coreclr/vm/i386/cgenx86.cpp and src/coreclr/vm/amd64/cgenamd64.cpp was performing direct 32-bit, 16-bit, and 64-bit memory writes using pointer dereferences like *(INT32 *)p = value, *(UINT16 *)p = value, and *(TADDR *)p = value. When the pointer p is not aligned to the appropriate byte boundaries (4-byte for INT32, 2-byte for UINT16, 8-byte for TADDR on AMD64), this causes undefined behavior that can be detected by clang's undefined behavior sanitizer (-fsanitize=undefined).

Solution

Replaced all misaligned memory access patterns with the appropriate SET_UNALIGNED_* macros that are specifically designed to handle potentially unaligned memory access safely:

i386 (cgenx86.cpp):

  • 21 instances of *(INT32 *)p = valueSET_UNALIGNED_32(p, value)
  • 1 instance of *(UINT16 *)p = valueSET_UNALIGNED_16(p, value)
  • 8 additional instances in CreateDictionaryLookupHelper:
    • 5 instances of *(UINT16 *)p = valueSET_UNALIGNED_16(p, value)
    • 3 instances of *(UINT32 *)p = valueSET_UNALIGNED_32(p, value)

amd64 (cgenamd64.cpp):

  • 22 instances of *(UINT16 *)p = valueSET_UNALIGNED_16(p, value)
  • 13 instances of *(UINT32 *)p = valueSET_UNALIGNED_32(p, value)
  • 10 instances of *(INT32 *)p = valueSET_UNALIGNED_32(p, value)
  • 12 instances of *(TADDR *)p = valueSET_UNALIGNED_64(p, value)
  • 4 instances of *(UINT64 UNALIGNED *)p = valueSET_UNALIGNED_64(p, value)

These macros are defined in the CoreCLR PAL headers and use the UNALIGNED attribute to ensure safe memory access regardless of alignment.

Testing

  • CoreCLR builds successfully with the changes on both architectures
  • Changes are minimal and surgical, only affecting the specific undefined behavior issue
  • All functionality is preserved as the macros provide the same behavior but without undefined behavior

Fixes #118602.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Copilot AI changed the title [WIP] Misaligned memory writes at src/coreclr/vm/i386/cgenx86.cpp Fix misaligned memory writes in i386 dynamic helper code generation Aug 13, 2025
Copilot finished work on behalf of jkotas August 13, 2025 15:01
Copilot AI requested a review from jkotas August 13, 2025 15:01
@jkotas
Copy link
Member

jkotas commented Aug 13, 2025

@copilot Could you please apply similar fixes in cgenamd64.cpp as well?

Copy link
Contributor Author

Copilot AI commented Aug 13, 2025

@copilot Could you please apply similar fixes in cgenamd64.cpp as well?

Applied similar fixes to cgenamd64.cpp in commit dd5087a. Replaced 58 instances of misaligned memory writes with appropriate SET_UNALIGNED_* macros, following the same pattern as the i386 fixes. CoreCLR builds successfully with the changes.

Copilot AI changed the title Fix misaligned memory writes in i386 dynamic helper code generation Fix misaligned memory writes in i386 and amd64 dynamic helper code generation Aug 13, 2025
Copilot finished work on behalf of jkotas August 13, 2025 15:53
Copilot AI requested a review from jkotas August 13, 2025 15:53
@MichalPetryka
Copy link
Contributor

When I did a regex (\*[\s]*\(.*\*[\s]*\)) search for this pattern, I got 3056 occurances in src dir alone (didn't search in the rest) so seems like there are lots more.

@jkotas
Copy link
Member

jkotas commented Aug 13, 2025

When I did a regex (*[\s](.*[\s]*)) search for this pattern

Majority of hits found by this pattern are false positives.

Copilot AI requested a review from jkotas August 13, 2025 16:35
Copilot finished work on behalf of jkotas August 13, 2025 16:35
Copilot finished work on behalf of jkotas August 13, 2025 17:18
@jkotas jkotas marked this pull request as ready for review August 13, 2025 18:03
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR fixes undefined behavior in CoreCLR's dynamic helper code generation by replacing misaligned memory writes with safe unaligned access macros. The changes address potential crashes and undefined behavior that can be detected by sanitizers when the memory pointer is not properly aligned for the write operation size.

Key changes:

  • Replace direct pointer dereferences with SET_UNALIGNED_* macros in i386 and amd64 code generation
  • Ensure safe memory access regardless of alignment for 16-bit, 32-bit, and 64-bit writes
  • Preserve all existing functionality while eliminating undefined behavior

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/coreclr/vm/i386/cgenx86.cpp Replace 30 instances of misaligned memory writes with SET_UNALIGNED_16 and SET_UNALIGNED_32 macros
src/coreclr/vm/amd64/cgenamd64.cpp Replace 61 instances of misaligned memory writes with SET_UNALIGNED_16, SET_UNALIGNED_32, and SET_UNALIGNED_64 macros

@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @mangod9
See info in area-owners.md if you want to be subscribed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Misaligned memory writes at src/coreclr/vm/i386/cgenx86.cpp

4 participants