Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ExpressionSimplifier rewrite some comparisons incorrectly. #883

Open
blindmatrix opened this issue Jun 24, 2020 · 0 comments
Open

ExpressionSimplifier rewrite some comparisons incorrectly. #883

blindmatrix opened this issue Jun 24, 2020 · 0 comments

Comments

@blindmatrix
Copy link
Contributor

The loops while expression should be (wLoc08_35 < 32) (or (ax_24 + 0x01 < 32)) but is instead rewritten as (ax_24 < 65505). This happens in ExpressionSimplifier.cs:358.

(Sidenote: wLoc08_35 was defined as ax_24 + 0x01. It would have been amazing if this was taken into consideration when propagating values. (I think that wLoc08_35 should be treated as a real variable instead of a temporary, but I guess that would require a lot more work when deciding what to keep and what was temporary which would probably also result in a lot of incorrect guesses...). But that's not the point of this issue.)

(Also, there's another issue with the output, but I'm creating a separate issue for that, related to array indexing)

Sample binary (and .dcproject):
RekoIssueArrayIndex-comp.zip

Incorrect output:

Eq_2 Win32CrtStartup()
{
	word16 wLoc08_35 = 0x00;
	if (true)
	{
		do
		{
			g_a403000[(word32) wLoc08_35 * 0x02] = (struct Eq_25) (word16) ((word32) wLoc08_35 * 0x02);
			cup16 ax_24 = (word16) (word32) wLoc08_35;
			wLoc08_35 = ax_24 + 0x01;
		} while (ax_24 < 65505);
	}
	ExitProcess(0x00);
}

Sample program:

volatile unsigned short theArray[32];

extern "C"
int WINAPI mainCRTStartup(void)
{
	for (volatile unsigned short i = 0; i < 32; i++)
		theArray[i] = i * 2;

	ExitProcess(0);
}

Sample assembly:

.text:00401000                 public mainCRTStartup
.text:00401000 mainCRTStartup  proc near
.text:00401000
.text:00401000 i               = word ptr -4
.text:00401000
.text:00401000                 push    ebp
.text:00401001                 mov     ebp, esp
.text:00401003                 push    ecx
.text:00401004                 xor     eax, eax
.text:00401006                 mov     [ebp+i], ax
.text:0040100A                 cmp     [ebp+i], 20h
.text:0040100F                 jnb     short loc_401035
.text:00401011
.text:00401011 loc_401011:                             ; CODE XREF: mainCRTStartup+33�j
.text:00401011                 movzx   eax, [ebp+i]
.text:00401015                 lea     ecx, [eax+eax]
.text:00401018                 movzx   eax, [ebp+i]
.text:0040101C                 mov     theArray[eax*2], cx
.text:00401024                 movzx   eax, [ebp+i]
.text:00401028                 inc     ax
.text:0040102A                 mov     [ebp+i], ax
.text:0040102E                 cmp     [ebp+i], 20h
.text:00401033                 jb      short loc_401011
.text:00401035
.text:00401035 loc_401035:                             ; CODE XREF: mainCRTStartup+F�j
.text:00401035                 push    0               ; uExitCode
.text:00401037                 call    ds:ExitProcess
.text:00401037 mainCRTStartup  endp
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant