Fix XrefScanUtilFinder for 32 bit games#222
Fix XrefScanUtilFinder for 32 bit games#222Dustin21335 wants to merge 5 commits intoBepInEx:masterfrom
Conversation
It tried to convert IPRelativeMemoryAddress to IntPtr and that caused OverflowExceptions on 32 bit games when the value exceeded int.MaxValue.
| instruction.Op1Kind == OpKind.Memory && instruction.IsIPRelativeMemoryOperand) | ||
| { | ||
| var movTarget = (IntPtr)instruction.IPRelativeMemoryAddress; | ||
| var movTarget = new IntPtr(unchecked((long)instruction.IPRelativeMemoryAddress)); |
There was a problem hiding this comment.
Why does this fix the problem?
There was a problem hiding this comment.
Exactly. Why does this not throw too?
There was a problem hiding this comment.
It converts the ulong to a long with unchecked wrapping, then creates a new IntPtr
https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/statements/checked-and-unchecked
There was a problem hiding this comment.
The only ways I see that not throwing is either:
- The
ulongvalue is less than or equal toint.MaxValue. - The
ulongvalue is greater thanulong.MaxValue + int.MinValue.
Is it guaranteed to be in these ranges?
There was a problem hiding this comment.
Oh yea your right I apologize. I believe this would work without throwing
IntPtr movTarget;
if (Environment.Is64BitProcess) movTarget = address <= long.MaxValue ? new IntPtr((long)address) : new IntPtr(unchecked((long)address));
else movTarget = address <= int.MaxValue ? new IntPtr((int)address) : new IntPtr(unchecked((int)address));
| if (Environment.Is64BitProcess) return address <= long.MaxValue ? new IntPtr((long)address) : new IntPtr(unchecked((long)address)); | ||
| else return address <= int.MaxValue ? new IntPtr((int)address) : new IntPtr(unchecked((int)address)); |
There was a problem hiding this comment.
This is just return unchecked((IntPtr)address);
However, dropping the upper 32 bits seems like a massive bug. Did you actually verify that this works?
There was a problem hiding this comment.
I'm extremely hesitant to merge this kind of change. I want a review by one of the other maintainers.
There was a problem hiding this comment.
Alright thank you that's fine, let me know if you need anything from me. My apologies I was bad at explaining
Co-authored-by: Jeremy Pritts <49847914+ds5678@users.noreply.github.com>
Co-authored-by: Jeremy Pritts <49847914+ds5678@users.noreply.github.com>
Co-authored-by: Jeremy Pritts <49847914+ds5678@users.noreply.github.com>
|
Hey, did all the requested changes go through? I think I accepted them all but it still says 1 requested change |
It appears so. |
|
Alright thank you |
2 participants
It tried to convert IPRelativeMemoryAddress to IntPtr and that caused OverflowException on 32 bit games when the value exceeded int.MaxValue.