stdcxx-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Sebor <se...@roguewave.com>
Subject Re: MSVC 7.1 bad code generation
Date Fri, 14 Sep 2007 05:45:17 GMT
Farid Zaripov wrote:
>   The 22.locale.money.put.cpp test fails on MSVC 7.1 (15s build type)
> with buffer overrun error due to bad code generation.

Great detective work! Since our money_base class has no data members
the patch is fine.

Your analysis of the assembly looks right on the money. It seems MSVC
7.1 implementation of the empty base optimization (EBO) has a bug. It
might have something to do with it being second (or not first) in the
list of bases. If you can reproduce it in a simple test case and file
it as an External issue just for the record that would be great.

I assume the bug doesn't exist in MSVC 8.0 so we don't need to worry
about letting Microsoft know about it?

Martin

> 
>   Here the assembly code for moneypunct ctor:
> -------------
>     _EXPLICIT moneypunct (_RWSTD_SIZE_T __refs = 0)
>         : _RW::__rw_facet (__refs), money_base () { }
> 004018C0  push        ebp  
> 004018C1  mov         ebp,esp 
> 004018C3  push        ecx  
> 004018C4  mov         dword ptr [ebp-4],ecx 
> 004018C7  mov         eax,dword ptr [__refs] 
> 004018CA  push        eax  
> 004018CB  mov         ecx,dword ptr [this] 
> 004018CE  call        __rw::__rw_facet::__rw_facet (412E20h) 
> 
> 004018D3  xor         ecx,ecx 
> 004018D5  mov         edx,dword ptr [this] 
> 004018D8  add         edx,38h                       // the sizeof
> (moneypunct) == 0x38
> 004018DB  mov         byte ptr [edx],cl           // here the place of
> the buffer overrun
> 
> 004018DD  mov         eax,dword ptr [this] 
> 004018E0  mov         dword ptr [eax],offset
> std::moneypunct<char,0>::`vftable' (488838h) 
> 004018E6  mov         eax,dword ptr [this] 
> 004018E9  mov         esp,ebp 
> 004018EB  pop         ebp  
> 004018EC  ret         4    
> -------------
> 
>   When I commented the money_base () call the test succeeded and
> assembly code has changed to:
> -------------
>     _EXPLICIT moneypunct (_RWSTD_SIZE_T __refs = 0)
>         : _RW::__rw_facet (__refs)/*, money_base ()*/ { }
> 004018C0  push        ebp  
> 004018C1  mov         ebp,esp 
> 004018C3  push        ecx  
> 004018C4  mov         dword ptr [ebp-4],ecx 
> 004018C7  mov         eax,dword ptr [__refs] 
> 004018CA  push        eax  
> 004018CB  mov         ecx,dword ptr [this] 
> 004018CE  call        __rw::__rw_facet::__rw_facet (412E20h) 
> 004018D3  mov         ecx,dword ptr [this] 
> 004018D6  mov         dword ptr [ecx],offset
> std::moneypunct<char,0>::`vftable' (488838h) 
> 004018DC  mov         eax,dword ptr [this] 
> 004018DF  mov         esp,ebp 
> 004018E1  pop         ebp  
> 004018E2  ret         4    
> -------------
> 
>   Here the same assembly, but in 12s configuration:
> 
> before change:
> -------------
>     const PunctT pun;
> 004018B1  push        1    
> 004018B3  lea         ecx,[esp+0B4h] 
> 004018BA  call        __rw::__rw_facet::__rw_facet (40A770h) 
> 
> 004018BF  mov         byte ptr [esp+0E8h],bl            // 0xE8 - 0xB4
> == 0x34, so here not buffer overrun,
>  
> // but maybe changed last 4-byte member of the __rw_facet
>  
> // (I suppose is _C_pid)
> 
> 004018C6  mov         dword ptr [esp+0B0h],offset
> Punct<char,0>::`vftable' (43A258h) 
> -------------
> 
> after change:
> -------------
>     const PunctT pun;
> 00401891  push        1    
> 00401893  lea         ecx,[esp+0B4h] 
> 0040189A  call        __rw::__rw_facet::__rw_facet (40A720h) 
> 0040189F  mov         dword ptr [esp+0B0h],offset
> Punct<char,0>::`vftable' (43A258h) 
> -------------
> 
>   The money_base is empty class with default ctor and I think we could
> remove explicit invoking
> of the money_base() from the moneypunct ctor initialization list.
> 
>   I have not verified, but I suppose that the same problem might be with
> messages class.
> 
>   The patch is below:
> 
>   ChangeLog:
>   * _messages.h (messages): Removed explicit invoking of the
> messages_base() ctor
>   to avoid buffer overrun due to bad code generation on MSVC 7.1.
>   * _moneypunct.h (moneypunct): Removed explicit invoking of the
> money_base() ctor
>   to avoid buffer overrun due to bad code generation on MSVC 7.1.
> 
> -------------
> Index: loc/_messages.h
> ===================================================================
> --- loc/_messages.h	(revision 575203)
> +++ loc/_messages.h	(working copy)
> @@ -82,7 +82,7 @@
>                           allocator<char_type> > string_type;
>  
>      _EXPLICIT messages (_RWSTD_SIZE_T __refs = 0)
> -        : _RW::__rw_facet (__refs), messages_base () { }
> +        : _RW::__rw_facet (__refs) { }
>  
>  
>      catalog open (const string& __fun, const locale& __loc) const {
> Index: loc/_moneypunct.h
> ===================================================================
> --- loc/_moneypunct.h	(revision 575203)
> +++ loc/_moneypunct.h	(working copy)
> @@ -66,7 +66,7 @@
>      string_type;
>  
>      _EXPLICIT moneypunct (_RWSTD_SIZE_T __refs = 0)
> -        : _RW::__rw_facet (__refs), money_base () { }
> +        : _RW::__rw_facet (__refs) { }
>  
>      char_type decimal_point () const {
>          return do_decimal_point ();
> -------------
> 
> Farid.
> 


Mime
View raw message