Perhaps a bug

[Grincheux]

Normaly when I do :

_iNewHeight = __iNewHeight - _iStatusHeight - WINDOW_HEIGHT ;
this is equivalent to
_iNewHeight = __iNewHeight - (_iStatusHeight + WINDOW_HEIGHT) ;

With the compiler this is false.

Code: [Select]

LRESULT Edi_OnSize(HWND __hWnd,int __iNewWidth,int __iNewHeight)
{
int _iStatusHeight, _iNewWidth, _iNewHeight ;


RECT _Rc ;


MoveWindow(hStatus,0,0,__iNewWidth,22,TRUE) ,


GetWindowRect(hStatus,&_Rc) ;
_iStatusHeight = _Rc.bottom - _Rc.top ;


// _iNewHeight = __iNewHeight - (_iStatusHeight + WINDOW_HEIGHT) ;
_iNewHeight = __iNewHeight - _iStatusHeight - WINDOW_HEIGHT ;
_iNewWidth = __iNewWidth - WINDOW_WIDTH ;


MoveWindow(hTop,0,0,_iNewWidth,_iNewHeight,TRUE) ;
MoveWindow(hBottom,0,_iNewHeight,_iNewWidth,WINDOW_HEIGHT,TRUE) ;
MoveWindow(hRight,_iNewWidth,0,WINDOW_WIDTH,__iNewHeight - _iStatusHeight,TRUE) ;


return (0) ;
}


[frankie]

Hello Grincheux,
I adjusted your post because was not readable (there was a font modification for 2 pixels height).
Going back to your post no this is not a bug at all.
On mathematical side the equivalence you wrote is absolutely correct.
On the compiler side is a little bit different. The compiler doesn't try to 'simplify' the functions you pass to it, but simply code them as they are applying simple algebraic rules on values grouping as specified by the braces you put in your formula.
Then in the optimization phase it can change some steps to make code better or faster, but only reasoning on opcodes basis.
The 2 version you got are both optimized (see the use of the faster lea instruction), but they always follow pedantically what you wrote:

• Get an initial variable, subtract a variable, subtract the other variable
• Sum 2 variables then subtract the result from variable 1

[jj2007]

This works as expected. Are you sure the problem is not elsewhere?

Code: [Select]

#include <stdio.h>

int main(void) {
  int v0=200, v1=100, v2=50;
  printf("Result twice minus =\t%i\n", v0-v1-v2);
  printf("Result with brackets=\t%i\n", v0-(v1+v2));
}

[frankie]

JJ
Grincheux don't say that the result is wrong, but that the execution flow is different (you should be confortable at assembler  )
And this, as explained above, is not a bug.

[jj2007]

OK, misunderstanding - sorry.

[Grincheux]

It was the debug version with no optimization.


The result is WROG, FALSE...


I don't get the same result twice and the first is really false.
I was examining my code during two days before deciding to make computation by the hand.
Like this I realized that with the first formula it was worng and with the second the result is OK.


I did use optimization because the different results can be explained with them and not without optimization.
This the reason I told that (in an other post) it would be interesting to have a build and a release config.

[frankie]

It was the debug version with no optimization.


The result is WROG, FALSE...


I don't get the same result twice and the first is really false.
I was examining my code during two days before deciding to make computation by the hand.
Like this I realized that with the first formula it was worng and with the second the result is OK.


I did use optimization because the different results can be explained with them and not without optimization.
This the reason I told that (in an other post) it would be interesting to have a build and a release config.

Ah Ok I see it now 
The problem is with an intermediate result where ax register is stored at [rsp+34], in one case is stored a value = _iStatusHeight +250, in the other case is stored _iStatusHeight - 250 (250 is the value of WINDOW_HEIGHT I suppose). 
Let me know please if, as I suppose, _iNewHeight  is correctly calculated, and the problem is elsewhere where the value stored in [rsp+34] is reused. Can you look through disassembly, and post it, to localize where [rsp+34] is used? This is very important to understand which optimization triggers the error.

[Grincheux]

Yes 250 is the value of WINDOW_HEIGHT.


I recreate the bug and I post.

[Grincheux]

Here is a complete debugging session and the compiler and the linker options dialog boxes
there are 3 archives RAR files

[Grincheux]

Last archive

[Grincheux]

Here is a complete debugging session and the compiler and the linker options dialog boxes
there are 3 archives RAR files

[Grincheux]

This is my sexy editor

[frankie]

The problem is in the instruction:

Code: [Select]

lea eax[eax+16]First of all I could not understand why is there  I really don't understand why that value is added to the result?
Because the instruction is executed on both conditions you'll get adifference of 32 between the two results 
For me the real question is why 16 is added? Do you know if there is any macro around in your code? Or is it a compiler dangling value?
Anyway the bug is confirmed for me. 

P.S. Please check if that 16 exists olso with no optimizations.

[Grincheux]

There is no optimisation. See my hard copy.

   GetWindowRect(hStatus,&_Rc) ;
   _iStatusHeight = _Rc.bottom - _Rc.top ;

   _iNewHeight = __iNewHeight - (_iStatusHeight + WINDOW_HEIGHT) ;

_iStatusHeight = 23
__iNewHeight = HIWORD(__lParam) // WM_SIZE
IMAGE_HEIGHT = 250

Vars name with one undescore are locales variables
Vars name with two underscores are parameters

#define   GWL_USERDATA   -21
#define   WM_EDITOR      WM_USER + 1
#define   MAX_NAME      31
#define   MAX_FILES      500
#define   TABCTRL_HEIGHT   22
#define   IMAGE_WIDTH      300
#define   IMAGE_HEIGHT   250
#define   WINDOW_WIDTH   IMAGE_WIDTH
#define   WINDOW_HEIGHT   IMAGE_HEIGHT + TABCTRL_HEIGHT



static LRESULT CALLBACK EdiWndProc(HWND __hWnd,UINT __Msg,WPARAM __wParam,LPARAM __lParam)
{
   switch (__Msg)
   {
      case   WM_PAINT :         return (Edi_OnPaint(__hWnd)) ;
      case   WM_COMMAND :      return (Edi_OnCommand(__hWnd,__wParam,__lParam)) ;
      case   WM_NOTIFY :         return (Edi_OnNotify(__hWnd,(LPNMHDR) __lParam)) ;
      case   WM_SIZE :         return (Edi_OnSize(__hWnd,LOWORD(__lParam),HIWORD(__lParam))) ;
      case   WM_CLOSE :         return (Edi_OnWmClose(__hWnd)) ;
      case   WM_DESTROY :      return (Edi_OnDestroy(__hWnd)) ;
      case   WM_CREATE :         return (Edi_OnCreate(__hWnd)) ;
   }


   return (DefWindowProc(__hWnd,__Msg,__wParam,__lParam)) ;
}

[Grincheux]

Here is the full file (Edi.c)

Code: [Select]

#define WIN32_LEAN_AND_MEAN  /* speed up compilations */
#include <windows.h>
#include <windowsx.h>
#include <shlwapi.h>
#include <commdlg.h>
#include <commctrl.h>
#include <tchar.h>
#include <Richedit.h>
#include <Uxtheme.h>
#include "libgfl.h"
#include "libgfle.h"
#include "Edi-Cnst.h"
#include "Edi-Struc.h"
#include "Edi-Rsc.h"
#include "Edi-Vars.h"
#include "Edi-Fns.h"
#include "Edi-Ladies.h"

LRESULT Edi_OnCreate(HWND __hWnd)
{
   int         _nHeight ;
   HDC         _hDC ;

   SetWindowTheme(__hWnd,L"Explorer",NULL) ;

   Ofn.hwndOwner = NULL ;

   _hDC = GetDC(__hWnd) ;
   _nHeight = -MulDiv(10,GetDeviceCaps(_hDC,LOGPIXELSY),72) ;
   ReleaseDC(__hWnd,_hDC) ;

   hFont_00 = CreateFont(_nHeight,0,0,0,FW_EXTRALIGHT,0,0,0,ANSI_CHARSET,OUT_TT_ONLY_PRECIS,CLIP_DEFAULT_PRECIS,CLEARTYPE_QUALITY,VARIABLE_PITCH|FF_DONTCARE,"Tahoma");


   hStatus = CreateWindow(STATUSCLASSNAME,NULL,WS_CHILD|WS_VISIBLE|SBARS_SIZEGRIP,0,0,0,0,__hWnd,(HMENU) 0x1962,hInstance,NULL);
   SendMessage(hStatus,SB_SETPARTS,(WPARAM)(sizeof(Statwidths)/sizeof(int)),(LPARAM) Statwidths) ;
   SendMessage(hStatus,(UINT) SB_SETTEXT,(WPARAM) 0,(LPARAM) (LPSTR) "Welcome") ;
   SendMessage(hStatus,WM_SETFONT,(WPARAM) hFont_00,TRUE) ;

   hTop = CreateWindow(szClassTop,NULL,WS_CHILD|WS_VISIBLE,0,0,0,0,__hWnd,(HMENU) 0x1963,hInstance,NULL);
   hBottom = CreateWindow(szClassBottom,NULL,WS_CHILD|WS_VISIBLE,0,0,0,0,__hWnd,(HMENU) 0x1964,hInstance,NULL);

   return (0) ;
}

LRESULT Edi_OnPaint(HWND __hWnd)
{
    PAINTSTRUCT   _Ps ;
    RECT      _Rc ;

    BeginPaint(__hWnd,&_Ps) ;
    GetClientRect(__hWnd,&_Rc) ;
    DrawText(_Ps.hdc,_T("Hello,Windows!"),-1,&_Rc,DT_SINGLELINE|DT_CENTER|DT_VCENTER) ;
    EndPaint(__hWnd,&_Ps) ;

   return (0) ;
}

LRESULT Edi_OnWmClose(HWND __hWnd)
{
   if(Edi_YesNo(__hWnd,LADY_April) == IDYES)
   {
      Edi_Files_CloseAll(__hWnd) ;

      DestroyWindow(__hWnd) ;
      return (0) ;
   }

   return (1) ;
}

LRESULT Edi_OnCommand(HWND __hWnd,WPARAM __wParam,LPARAM __lParam)
{
   switch(LOWORD(__wParam))
   {
      case   IDM_ABOUT :               return (Edi_About(__hWnd)) ;
      case   IDM_FILES_NEW :            return (Edi_Files_New(__hWnd)) ;
      case   IDM_FILES_OPEN :         return (Edi_Files_Open(__hWnd)) ;
      case   IDM_FILES_OPEN_UNDER :      return (Edi_Files_OpenUnder(__hWnd)) ;
      case   IDM_FILES_OPEN_READING :   return (Edi_Files_OpenReading(__hWnd)) ;
      case   IDM_FILES_SAVE :         return (Edi_Files_Save(__hWnd)) ;
      case   IDM_FILES_SAVE_AS :         return (Edi_Files_SaveAs(__hWnd)) ;
      case   IDM_FILES_SAVE_ALL :      return (Edi_Files_SaveAll(__hWnd)) ;
      case   IDM_FILES_CLOSE :         return (Edi_Files_Close(__hWnd)) ;
      case   IDM_FILES_CLOSE_ALL :      return (Edi_Files_CloseAll(__hWnd)) ;
      case   IDM_FILES_EXIT :         return (Edi_Files_Exit(__hWnd)) ;
      case   IDM_PROJECT_OPEN :         return (Edi_Project_Open(__hWnd)) ;
    }

   return (DefWindowProc(__hWnd,WM_COMMAND,__wParam,__lParam)) ;
}

LRESULT Edi_OnDestroy(HWND __hWnd)
{
   DeleteObject(hFont_00) ;
   FreeLibrary(hRichEdit) ;
   Edi_ImageFree(lpLady) ;
   Edi_ImageFree(lpBackGround) ;
   gflLibraryExit() ;

    PostQuitMessage(0) ;

   return (0) ;
}

LRESULT Edi_OnSize(HWND __hWnd,int __iNewWidth,int __iNewHeight)
{
   int      _iStatusHeight, _iNewHeight ;

   RECT   _Rc ;

   MoveWindow(hStatus,0,0,__iNewWidth,22,TRUE) ,

   GetWindowRect(hStatus,&_Rc) ;
   _iStatusHeight = _Rc.bottom - _Rc.top ;

   [b]_iNewHeight = __iNewHeight - (_iStatusHeight + WINDOW_HEIGHT) ;[/b]
or
   [b]_iNewHeight = __iNewHeight - _iStatusHeight - WINDOW_HEIGHT ;[/b]

   MoveWindow(hTop,0,0,__iNewWidth,_iNewHeight,TRUE) ;
   MoveWindow(hBottom,0,_iNewHeight,__iNewWidth,WINDOW_HEIGHT,TRUE) ;

   return (0) ;
}

LRESULT Edi_OnNotify(HWND __hWnd,LPNMHDR __lpNmHdr)
{
   TCITEM   _Tci ;

   switch (__lpNmHdr->code)
   {
      case   TCN_SELCHANGING :   _Tci.mask = TCIF_PARAM ;
                           TabCtrl_GetItem(__lpNmHdr->hwndFrom,TabCtrl_GetCurSel(__lpNmHdr->hwndFrom),&_Tci) ;
                           ShowWindow((HWND) ((LPEDI_WORK) _Tci.lParam)->hEdit,SW_HIDE) ;
                                return (FALSE) ;   // Return FALSE to allow the selection to change.

      case   TCN_SELCHANGE :      _Tci.mask = TCIF_PARAM ;
                           TabCtrl_GetItem(__lpNmHdr->hwndFrom,TabCtrl_GetCurSel(__lpNmHdr->hwndFrom),&_Tci) ;
                           ShowWindow((HWND) ((LPEDI_WORK) _Tci.lParam)->hEdit,SW_SHOW) ;
                           UpdateWindow((HWND) ((LPEDI_WORK) _Tci.lParam)->hEdit) ;
                           PostMessage(__hWnd,WM_EDITOR,0,(LPARAM) _Tci.lParam) ;
                                return (FALSE) ;

      case   NM_CLICK :         _Tci.mask = TCIF_PARAM ;
                           TabCtrl_GetItem(__lpNmHdr->hwndFrom,TabCtrl_GetCurSel(__lpNmHdr->hwndFrom),&_Tci) ;
                           PostMessage(__hWnd,WM_EDITOR,__lpNmHdr->idFrom,(LPARAM) _Tci.lParam) ;
                           return (FALSE) ;
   }

   return (TRUE) ;
}

static LRESULT CALLBACK EdiWndProc(HWND __hWnd,UINT __Msg,WPARAM __wParam,LPARAM __lParam)
{
   switch (__Msg)
   {
      case   WM_PAINT :         return (Edi_OnPaint(__hWnd)) ;
      case   WM_COMMAND :      return (Edi_OnCommand(__hWnd,__wParam,__lParam)) ;
      case   WM_NOTIFY :         return (Edi_OnNotify(__hWnd,(LPNMHDR) __lParam)) ;
      case   WM_SIZE :         return (Edi_OnSize(__hWnd,LOWORD(__lParam),HIWORD(__lParam))) ;
      case   WM_CLOSE :         return (Edi_OnWmClose(__hWnd)) ;
      case   WM_DESTROY :      return (Edi_OnDestroy(__hWnd)) ;
      case   WM_CREATE :         return (Edi_OnCreate(__hWnd)) ;
   }

   return (DefWindowProc(__hWnd,__Msg,__wParam,__lParam)) ;
}

int Edi_InitInstance(HINSTANCE __hInstance)
{
   INITCOMMONCONTROLSEX   _Icc ;
   WNDCLASS            _Wc ;

   hInstance = __hInstance;

   SetErrorMode(SEM_FAILCRITICALERRORS) ;

   GetModuleFileName(__hInstance,szProgramName,MAX_PATH) ;
   *(PathFindFileName(lstrcpy(szProgramFolder,szProgramName)) - 1) = '\0' ;

   lstrcpy(szInitialDirectory,szProgramFolder) ;

   _Icc.dwSize = sizeof(INITCOMMONCONTROLSEX) ;
   _Icc.dwICC = ICC_WIN95_CLASSES|ICC_LINK_CLASS|ICC_PROGRESS_CLASS ;

   InitCommonControlsEx(&_Icc) ;

   if(gflLibraryInit() != GFL_NO_ERROR)   return (FALSE) ;

   hRichEdit = LoadLibrary(_T("msftedit.dll")) ;

   memset(&_Wc,0,sizeof(WNDCLASSEX)) ;

   _Wc.lpszClassName   = szClassName ;
   _Wc.style         = CS_VREDRAW|CS_HREDRAW|CS_DROPSHADOW ;
   _Wc.hIcon         = LoadIcon(hInstance,MAKEINTRESOURCE(IDR_ICO_MAIN)) ;
   _Wc.hCursor         = LoadCursor(NULL,IDC_ARROW) ;
   _Wc.hbrBackground   = (HBRUSH)(COLOR_WINDOW + 1) ;
   _Wc.lpszMenuName   = (LPSTR) IDM_MENU_MAIN ;
   _Wc.lpfnWndProc      = EdiWndProc ;
   _Wc.hInstance      = hInstance ;
   _Wc.cbClsExtra      = 256 ;
   _Wc.cbWndExtra      = 256 ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.hIcon         = NULL ;
   _Wc.lpszMenuName   = NULL ;

   _Wc.lpszClassName   = szClassTop ;
   _Wc.lpfnWndProc      = TopWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClassBottom ;
   _Wc.lpfnWndProc      = BottomWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Compile ;
   _Wc.lpfnWndProc      = CompileWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Constantes ;
   _Wc.lpfnWndProc      = ConstantesWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Find ;
   _Wc.lpfnWndProc      = FindWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Functions ;
   _Wc.lpfnWndProc      = FunctionsWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Help ;
   _Wc.lpfnWndProc      = HelpWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Includes ;
   _Wc.lpfnWndProc      = IncludesWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Libraries ;
   _Wc.lpfnWndProc      = LibrariesWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Project ;
   _Wc.lpfnWndProc      = ProjectWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Structures ;
   _Wc.lpfnWndProc      = StructuresWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_Styles ;
   _Wc.lpfnWndProc      = StylesWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   _Wc.lpszClassName   = szClass_ToDo ;
   _Wc.lpfnWndProc      = ToDoWndProc ;

   if(!RegisterClass(&_Wc)) return (FALSE) ;

   memset(&Ofn,0,sizeof(OPENFILENAME)) ;

   Ofn.lStructSize = sizeof(OPENFILENAME) ;
   Ofn.hInstance = hInstance ;
   Ofn.lpstrFilter = "C sources files\0*.c\0C headers\0*.h\0Resources files\0*.rc\0Text files\0*.txt\0Assembler source files\0*.asm\0Assembler Include files\0*.inc\0All files\0*.*\0\0" ;
   Ofn.nFilterIndex = 1 ;
   Ofn.lpstrFile = szFileToOpen ;
   Ofn.nMaxFile = MAX_PATH ;
   Ofn.lpstrFileTitle = szFileTitle ;
   Ofn.nMaxFileTitle = MAX_PATH ;
   Ofn.lpstrInitialDir = szInitialDirectory ;
   Ofn.Flags = OFN_FILEMUSTEXIST|OFN_ENABLESIZING|OFN_EXPLORER|OFN_NONETWORKBUTTON|OFN_PATHMUSTEXIST|OFN_NOREADONLYRETURN|OFN_LONGNAMES|OFN_HIDEREADONLY ;
   Ofn.lpstrDefExt = "c" ;

   return (TRUE) ;
}

int Edi_InitApplication(HINSTANCE __hInstance)
{
   hWndMain = CreateWindowEx(WS_EX_ACCEPTFILES,szClassName,"Edi, a Sample Sexy Editor",
                     WS_OVERLAPPEDWINDOW|WS_VISIBLE,
                     0,0,CW_USEDEFAULT,CW_USEDEFAULT,NULL,NULL,__hInstance,NULL) ;
    if(!hWndMain) return (FALSE) ;

   ShowWindow(hWndMain,SW_MAXIMIZE) ;
   UpdateWindow(hWndMain) ;

   return (TRUE) ;
}

int PASCAL WinMain(HINSTANCE __hInstance,HINSTANCE __hPrevInstance,LPSTR __lpszCmdLine,int __nCmdShow)
{
   MSG      _Msg ;
   HACCEL   _hAccel ;

   if(!Edi_InitInstance(__hInstance))      return (0) ;
   if(!Edi_InitApplication(__hInstance))   return (0) ;

   _hAccel = LoadAccelerators(hInstance,(LPSTR) IDC_ACCEL) ;

   while (GetMessage(&_Msg,NULL,0,0))
   {
      if(!TranslateAccelerator(hWndMain,_hAccel,&_Msg))
      {
         TranslateMessage(&_Msg) ;
         DispatchMessage(&_Msg) ;
      }
   }

   return (_Msg.wParam) ;
}