Conversation
|
Tagging subscribers to this area: @JulieLeeMSFT, @dotnet/jit-contrib |
There was a problem hiding this comment.
Pull request overview
This PR targets CodeQL/static-analysis findings in the CoreCLR ilasm tool by addressing integer type correctness and potential unaligned UTF-16 access patterns in the parser/helpers.
Changes:
- Use an unsigned 16-bit loop index when iterating metadata streams during PE file creation.
- Avoid unaligned
WCHAR*dereferences inSymWby reading viamemcpy. - Add/adjust alignment handling for Unicode parsing helpers (aligned buffers; enforce even-byte lengths for UTF-16 token copies).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/coreclr/ilasm/writer.cpp |
Updates stream-iteration index type to avoid signed/size issues when walking metadata streams. |
src/coreclr/ilasm/grammar_after.cpp |
Hardens Unicode parsing helpers against unaligned access and odd-length UTF-16 token handling (plus aligned scratch buffers). |
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
|
I think focus should move to the managed ilasm in |
Until that is working, which is a long ways off, we still need to satisfy tooling and all the requirements for the existing code. |
It is just pending test infra support, everything else is in place; all TODOs addressed. |
Co-authored-by: Aaron R Robinson <[email protected]>
Fix two issues that got flagged
1 - Fix a mistyped loop counter.
2 - One of the files frequently casts CHAR*->WCHAR*. Retyping isn't reasonable, because these methods are called through function pointers using char*. These casts are probably safe since they all point into MapViewOfFile, but I add alignment constraints and bailouts if somehow we have an odd pointer. The analyzer still complains however, so I also need an explicit suppression of the warning.
I'm kicking off another analysis pipeline in a bit to make sure the suppressions get picked up