Aggressive optimization's bug in iteration statements

Started by frankie, September 02, 2018, 10:44:44 PM

Previous topic - Next topic

frankie

The following code demonstrate the bug.
The comments inside explain the issue in detail.
The bug is particularly insidious because not easily detectable especially by beginners.


/*
* This sample demonstrate an aggressive optimization bug.
* Run the code as is and see the problem.
*
* To workaround you can:
*  - Use no optimization
*  - Use a dummy function that use intermediate symbols
*    forcing compiler to don't remove code
*  - Compile with debug info's. (Really strange, the
*    compiler optimize diffrently with debug on!).
*/
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/*
* Define the following symbol to add a function call inside the loop.
* This will make work the code.
*/
//#define USEFNC 1

typedef struct
{
char *base;
char *p;
char *end;
} DATA;

char text[] = "This\n is a sample\n to be used\n to demonstrate\n a bug\n of aggressive optimization\n";

// Dummy function to trick the compiler and force it to emit correct code
#ifdef USEFNC
void DoNothing(int c)
{
}
#endif

// The 'optimized' function that behave so badly
char * ReadLine(char * restrict dst, int max, DATA * restrict pData)
{
if (pData->p >= pData->end)
return NULL;

int i = 0;
while((i < (max-1)) && (pData->p < pData->end))
{
#ifdef USEFNC
DoNothing(*pData->p);
#endif
dst[i] = *pData->p;
pData->p++;
if (dst[i++] == '\n')
break;
}
dst[i] = '\0';

return dst;
}

#define LINELEN 512

int main(int argc, char *argtv[])
{
DATA Data = {0};

Data.base = text;
Data.p    = text;
Data.end  = text + strlen(text);

char line[LINELEN];

while (ReadLine(line, LINELEN, &Data))
printf("%s", line);

return 0;

}
"It is better to be hated for what you are than to be loved for what you are not." - Andre Gide

TimoVJL

Bug might be here, as RCX was not incremented:; pData->p++;
lea r11, [rcx+01h]
mov [r8+08h], r11
; inc rcx ; add this to fix that bug

public ReadLine
.code
;ReadLine:
;char * ReadLine(char * restrict dst, int max, DATA * restrict pData)
;rcx dst rdx max r8 pData
ReadLine proc
push rbx
mov rax, rcx ; dst
mov rcx, [r8+08h] ; pData->p
mov r9, [r8+10h] ; pData->end
; if (pData->p >= pData->end)
cmp rcx, r9
; return 0;
jnb @52
; int i = 0;
xor r10d, r10d
; while((i < (max-1)) && (pData->p < pData->end)) {
jmp @3A
; dst[i] = *pData->p;
@16: movsxd r11, r10d
mov bl, [rcx]
mov [rax+r11*1], bl
; pData->p++;
lea r11, [rcx+01h]
mov [r8+08h], r11
; inc rcx ; add this to fix that bug
; if (dst[i++] == '\n')
add r10d, 01h
lea r11d, [r10d-01h]
movsxd r11, r11d
cmp byte ptr [rax+r11*1], 0Ah
jz @49
@3A: lea r11d, [edx-01h]
; (i < (max-1))
cmp r10d, r11d
jnl @49
; (pData->p < pData->end)
cmp rcx, r9
jb @16
; break;
; }
; dst[i] = '\0';
@49: movsxd rdx, r10d
mov byte ptr [rax+rdx*1], 0
jmp @54
@52: xor eax, eax
; return dst;
@54: pop rbx
ret
ReadLine endp
end
May the source be with you

Pelle

For now, you can drop the restrict qualifier on the pointer to the DATA struct in ReadLine().

(An early optimizer pass didn't anticipate a restrict pointer to point to a struct with more pointers, allowing later passes to mess things up).

Will be fixed some day, but that day isn't right now...
/Pelle