On Fri, Sep 05, 2008 at 10:44:50PM +0100, Rob Shearman wrote: > 2008/9/5 Francois Gouget <[EMAIL PROTECTED]>: > > I have a few other concerns here: > > * Why do we need a macro here? I thought it was so that > > __builtin_object_size() could do its work, but the strcpy() functions > > above in the patch have no associated macro and they call > > __builtin_object_size() on their parameter which would seem to be > > poinless. So I must be misunderstanding something here.
The strcpy functions there are inline, so we can modify the inline version. strcpy() itself is replaced by glibc headers (bits/string2.h and string3.h) into either a __builtin_strcpy_chk call, which gcc knows how to handle. (in earlier glibc/gcc versions with an additional macro wrapper). > > * It's customary to add some extra parenthesing in such macros: > > (src),(srclen),__builtin_object_size((src),0), Yes, need to fix. > > * The macro itself can cause trouble in some cases. In Winelib > > code it can cause naming collisions with user functions (especially > > with class methods, though it may not be very likely here), which > > would especially be an issue with strcpy*(). Maybe it should be > > protected by something like a WINE_NO_OVERFLOW_CHECK macro? So far we have no problems in Wine sourcecode. I could easily limit it to be __WINESRC__ only. > > * The macro can also cause trouble in case its parameters have > > side-effects, like x++ or similar (though the __builtin_object_size() > > mentions returning -1 in case or side-effects). This could impact > > Wine too. Need to fix, yes. > I also have some questions: > * Is there any runtime overhead of using __builtin_object_size and/or > the __alloc_size__ attribute? There is no runtime overhead. __alloc_size__ is applied at compile time only for constant size allocations (and carried along with the pointer inside GCC during compile). __builtin_object_size is evaluated at compile time too. If it cannot be determined, it will evaluate to -1. > * If not and it's a compile time only thing, why can't these buffer > overruns be detected at compile time instead of runtime (which > obviously depends on test coverage)? If you have a known-size destination buffer, but a variable source buffer this cannot be detected. If both are known-size at compile time, a compilation abort should be possible. Hmm, parts of the MBtoWC check at least could be made to fail at compile time, like the bug I fixed some days ago. This patch needs to go back to the drawing board ;) Ciao, Marcus