This is very easy to reproduce, just compile the sample code below (C or
C++, it doesn't matter) with /O2 or /Ox option.
Just adding a #pragma function(strcat) or compiling with /O1 or /Od removes
the bug.

I didn't try other version of Visual C++ than the one from Visual Studio.NET
2003 Entreprise Edition (french version, from MDSN subscription).

Conditions to make this bug happen:
1) an inlined strcat (intrisic) and
2) first parameter of the inlined strcat is a not-inlinable function call
returning the destination string pointer and
3) second parameter of the inlined strcat isn't a literal constant string
(the inlined strcat must have to do an inlined strlen on it, the optimizer
musn't be able to find a constant length).

// BEGINNING OF SAMPLE
#define WIN32_LEAN_AND_MEAN
#include <windows.h>
#include <stdio.h>
#include <string.h>

// making the variables below global prevent optimizations that "hide" the
bug
char sz1[256] = "abc";
char sz2[] = ".ghi";

int main (int argc, char **argv)
{
// lstrcat (below) instead of a second strcat to prevent its inlinning,
because
// the lstrcat replaces a more complex function from a DLL of mine.
// The code generated for the inlined strcat is realy very bad !
strcat(lstrcat(sz1, "def"), sz2); // sz1 is (far) large enough for that
// should print "abcdef.ghi" but the lstrcat make an exception !
printf("%s\n", sz1);
return 0;
}

// END OF SAMPLE

The generated code (with my comments) is plainly wrong:
_main PROC NEAR
; Line 15
; THE INSTRUCTION BELOW APPEAR TOO SOON
push OFFSET FLAT:??_C@_03BHEEIFFN@def?$AA@ ; push second
parameter of lstrcat ("def")
mov eax, OFFSET FLAT:_sz2 ; get second parameter of strcat
; THE INSTRUCTION BELOW APPEAR TOO SOON TOO
push OFFSET FLAT:_sz1 ; push first parameter of lstrcat (sz1)
; inlined strlen(sz2)
mov edx, eax
$L20160:
mov cl, BYTE PTR [eax]
inc eax
test cl, cl
jne SHORT $L20160
; OOPS, WRONG ! backup ebx/esi AFTER pushing parameters for lstrcat
and before
; calling lstrcat !
push ebx
push esi
sub eax, edx ; end of the inlined strlen(sz2)
; STILL WRONG ! same reason as above
push edi
;THIS (WAS ABOVE) SHOULD BE HERE: push OFFSET
FLAT:??_C@_03BHEEIFFN@def?$AA@
mov esi, edx ; backup strlen(sz2) value
;THIS (WAS ABOVE) SHOULD BE HERE: push OFFSET FLAT:_sz1
mov ebx, eax ; backup sz2 pointer
; lstrcat below is COMPLETELY LOST with the random values pushed
from edi and esi !
; => exception
call DWORD PTR __imp__lstrcatA@8
; inlined strcat, skip existing chars in the (never) returned
pointer to sz1 from lstrcat
dec eax
$L20161:
mov cl, BYTE PTR [eax+1]
inc eax
test cl, cl
jne SHORT $L20161
; and copy sz2 at the end of sz1
mov ecx, ebx
shr ecx, 2
mov edi, eax
rep movsd
mov ecx, ebx
and ecx, 3
; Line 17
push OFFSET FLAT:_sz1
push OFFSET FLAT:??_C@_03OFAPEBGM@?$CFs?6?$AA@
rep movsb
call _printf
add esp, 8
pop edi
pop esi
; Line 18
xor eax, eax
pop ebx
; Line 19
ret 0
_main ENDP

Re: Bug when generating some inlined (intrisic) strcat in vc 7.1 by Carl

Carl
Fri Apr 23 08:58:08 CDT 2004

Christophe Bouchon wrote:
> This is very easy to reproduce, just compile the sample code below (C
> or C++, it doesn't matter) with /O2 or /Ox option.
> Just adding a #pragma function(strcat) or compiling with /O1 or /Od
> removes the bug.

Thanks for the simple repro case!

This bug has been fixed in Whidbey. If it's causing you problems in
production code, compile with /O1 (which frequently results in faster code
than /O2 anway), or contact product support - there might be a QFE/hotfix
available.

-cd





Re: Bug when generating some inlined (intrisic) strcat in vc 7.1 by Christophe

Christophe
Fri Apr 23 10:16:10 CDT 2004

This isn't serious to expect all developpers will change their compilation
options in all their projects for such a basic bug in the compiler ! And I'm
sure most developpers prefer to have a fix for vc 7.1 instead of a fix in
Whidbey that will include many other bugs because it will have many new
features (for genericity and other extensions of the MSIL 2.0 for exemple,
and last missing bits for 100% conformance with C++ standard). It is
unnacceptable to have to deliver software products compiled by an unreliable
compiler to clients.

Browsing Microsoft newsgroups before submitting this bug, I found references
to several other bugs "fixed in Whidbey". Perhaps it's time for Visual
Studio.NET 2003 Service Pack 1 !

"Carl Daniel [VC++ MVP]" <cpdaniel_remove_this_and_nospam@mvps.org.nospam> a
écrit dans le message de news:ua$ODsTKEHA.3916@TK2MSFTNGP10.phx.gbl...
> Christophe Bouchon wrote:
> > This is very easy to reproduce, just compile the sample code below (C
> > or C++, it doesn't matter) with /O2 or /Ox option.
> > Just adding a #pragma function(strcat) or compiling with /O1 or /Od
> > removes the bug.
>
> Thanks for the simple repro case!
>
> This bug has been fixed in Whidbey. If it's causing you problems in
> production code, compile with /O1 (which frequently results in faster code
> than /O2 anway), or contact product support - there might be a QFE/hotfix
> available.
>
> -cd
>
>
>
>



Re: Bug when generating some inlined (intrisic) strcat in vc 7.1 by Carl

Carl
Fri Apr 23 10:28:26 CDT 2004

Christophe Bouchon wrote:
> This isn't serious to expect all developpers will change their
> compilation options in all their projects for such a basic bug in the
> compiler ! And I'm sure most developpers prefer to have a fix for vc
> 7.1 instead of a fix in Whidbey that will include many other bugs
> because it will have many new features (for genericity and other
> extensions of the MSIL 2.0 for exemple, and last missing bits for
> 100% conformance with C++ standard). It is unnacceptable to have to
> deliver software products compiled by an unreliable compiler to
> clients.

Then I guess you'll never deliver any products, since all compilers are
unreliable. Each compiler has it's own set of shortcomings that you
discover over time and learn to work around. Several of the VC++ MVPs have
recommended for years that /O1 should be the default since it's frequently
as fast or faster than /O2 and has proven to be less buggy. /Oi (which is
part of /O2) is particularly problematic since in many cases the library
(non-instrinsic) versions of the functions are actually faster.

> Browsing Microsoft newsgroups before submitting this bug, I found
> references to several other bugs "fixed in Whidbey". Perhaps it's
> time for Visual Studio.NET 2003 Service Pack 1 !

Perhaps it is. One thing to understand though: Service packs by definition
contain a collection of QFEs. Unless someone calls PSS and requests a QFE
for a particular bug, it's pretty much guaranteed to NOT be fixed in a
subsequent service pack.

-cd



Re: Bug when generating some inlined (intrisic) strcat in vc 7.1 by Christophe

Christophe
Fri Apr 23 16:59:36 CDT 2004

Yes, I know very well that almost every piece of software always have some
bugs, even some nobody know or will ever know about, what I was taking about
was reactivity on very basic bugs (I think you will agree strcat is a basic
enough and often called function, and /O2 is the default option for Release
projects). When a bug like that is discovered in one of our products, I must
fix it ASAP, most of the time in less than a week, except if the fix is too
dangerous for the stability of other parts of the code.

On another side, I would be very happy to see /Oi not defaulted except for
the most obvious cases (_rot[lr] and _abs because they always generate a
single assembler instruction, plus the arguments evaluation that are also
required when calling the runtime functions anyway). I did tests some years
ago (with VC4) that demonstrated that the run-time optimized string and
memory functions were far better than the inlined ones (and too large to be
inlined), more than compensating the call overhead by their efficiency (the
"fast find any 0 byte contained in a 32 bits word" used for string functions
was really good).

I'll try the PSS (if it can help for the creation of an MSVC SP, it's even
better...).

Thank you for yours answers.

"Carl Daniel [VC++ MVP]" <cpdaniel_remove_this_and_nospam@mvps.org.nospam> a
écrit dans le message de news:O7FjgeUKEHA.3216@tk2msftngp13.phx.gbl...
> Christophe Bouchon wrote:
> > This isn't serious to expect all developpers will change their
> > compilation options in all their projects for such a basic bug in the
> > compiler ! And I'm sure most developpers prefer to have a fix for vc
> > 7.1 instead of a fix in Whidbey that will include many other bugs
> > because it will have many new features (for genericity and other
> > extensions of the MSIL 2.0 for exemple, and last missing bits for
> > 100% conformance with C++ standard). It is unnacceptable to have to
> > deliver software products compiled by an unreliable compiler to
> > clients.
>
> Then I guess you'll never deliver any products, since all compilers are
> unreliable. Each compiler has it's own set of shortcomings that you
> discover over time and learn to work around. Several of the VC++ MVPs
have
> recommended for years that /O1 should be the default since it's frequently
> as fast or faster than /O2 and has proven to be less buggy. /Oi (which is
> part of /O2) is particularly problematic since in many cases the library
> (non-instrinsic) versions of the functions are actually faster.
>
> > Browsing Microsoft newsgroups before submitting this bug, I found
> > references to several other bugs "fixed in Whidbey". Perhaps it's
> > time for Visual Studio.NET 2003 Service Pack 1 !
>
> Perhaps it is. One thing to understand though: Service packs by
definition
> contain a collection of QFEs. Unless someone calls PSS and requests a QFE
> for a particular bug, it's pretty much guaranteed to NOT be fixed in a
> subsequent service pack.
>
> -cd
>
>