RFC: Adding annotations to function declarations to improve static analysis

[Rob Shearman]

Hi all,

This will be talked about in more detail at WineConf tomorrow, but I
just thought I'd throw this out there as a bit of background to any
discussions.

Adding annotations to function declarations allows Prefast to pick up
certain classes of bugs with varying degrees of false positives. In
particular, with patches like the attached applied
byte-count/element-count mismatches can be detected with no false
positives and relatively few false positives for off-by-one errors and
other buffer overruns. Whilst this could be maintained outside of the
main Wine tree it would be more convenient in terms of automation of
Prefast runs if a vanilla Wine tree can be used (i.e. the annotation
patches are in the official tree).

-- 
Rob Shearman

RFC: Adding annotations to function declarations to improve static analysis

[Reece Dunn]

2008/9/28 Rob Shearman <robertshearman(a)gmail.com>:
> Hi all,
>
> This will be talked about in more detail at WineConf tomorrow, but I
> just thought I'd throw this out there as a bit of background to any
> discussions.
>
> Adding annotations to function declarations allows Prefast to pick up
> certain classes of bugs with varying degrees of false positives. In
> particular, with patches like the attached applied
> byte-count/element-count mismatches can be detected with no false
> positives and relatively few false positives for off-by-one errors and
> other buffer overruns. Whilst this could be maintained outside of the
> main Wine tree it would be more convenient in terms of automation of
> Prefast runs if a vanilla Wine tree can be used (i.e. the annotation
> patches are in the official tree).

This is a good idea.

Is it possible to make tools like sparse aware of these annotations? I
know that the kernel devs use it to track kernel vs userland pointer
mis-matches, but don't know that much about the details.

It should then be possible to allow users to configure (if not already
available) the build to use sparse as the designated toolchain. This
may also generate even more warnings, even without the annotations :).

- Reece

RFC: Adding annotations to function declarations to improve static analysis

[Rob Shearman]

2008/9/28 Reece Dunn <msclrhd(a)googlemail.com>:
> 2008/9/28 Rob Shearman <robertshearman(a)gmail.com>:
> > Hi all,
> >
> > This will be talked about in more detail at WineConf tomorrow, but I
> > just thought I'd throw this out there as a bit of background to any
> > discussions.
> >
> > Adding annotations to function declarations allows Prefast to pick up
> > certain classes of bugs with varying degrees of false positives. In
> > particular, with patches like the attached applied
> > byte-count/element-count mismatches can be detected with no false
> > positives and relatively few false positives for off-by-one errors and
> > other buffer overruns. Whilst this could be maintained outside of the
> > main Wine tree it would be more convenient in terms of automation of
> > Prefast runs if a vanilla Wine tree can be used (i.e. the annotation
> > patches are in the official tree).
>
> This is a good idea.
>
> Is it possible to make tools like sparse aware of these annotations? I
> know that the kernel devs use it to track kernel vs userland pointer
> mis-matches, but don't know that much about the details.

Making sure that kernel/user pointers are not mixed up is quite
different to the annotations that I am proposing to add. However,
someone could certainly hack on sparse to make it become a more
advanced static analysis tool.
> It should then be possible to allow users to configure (if not already
> available) the build to use sparse as the designated toolchain. This
> may also generate even more warnings, even without the annotations :).

While I have used sparse on individual source files before, I believe
it would be a challenge to configure allow it to be used from
makefiles.

-- 
Rob Shearman

RFC: Adding annotations to function declarations to improve static analysis

[Marcus Meissner]

On Sun, Sep 28, 2008 at 12:08:22AM +0100, Rob Shearman wrote:
> Hi all,
>
> This will be talked about in more detail at WineConf tomorrow, but I
> just thought I'd throw this out there as a bit of background to any
> discussions.
>
> Adding annotations to function declarations allows Prefast to pick up
> certain classes of bugs with varying degrees of false positives. In
> particular, with patches like the attached applied
> byte-count/element-count mismatches can be detected with no false
> positives and relatively few false positives for off-by-one errors and
> other buffer overruns. Whilst this could be maintained outside of the
> main Wine tree it would be more convenient in terms of automation of
> Prefast runs if a vanilla Wine tree can be used (i.e. the annotation
> patches are in the official tree).

I have done something similar using small inline wrapper functions,
which works reliable for fixed size compiletime passed buffers and
sizes.

These wrappers will emit undefined functions when wrong and so fail
the build. Note the if() will be optimized away completely.

Redefining functions will however not work when programs use the function
names in other contexts, as struct names or similar.

Sample for 2 winver.h functions:


diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c
index c72d05e..a8de2a2 100644
--- a/dlls/kernel32/locale.c
+++ b/dlls/kernel32/locale.c
@@ -2166,6 +2169,7 @@ BOOL WINAPI EnumSystemLocalesW( LOCALE_ENUMPROCW lpfnLocaleEnum, DWORD dwFlags )
  *  Failure: 0. Use GetLastError() to determine the cause.
  *
  */
+#undef VerLanguageNameA
 DWORD WINAPI VerLanguageNameA( DWORD wLang, LPSTR szLang, DWORD nSize )
 {
     return GetLocaleInfoA( MAKELCID(wLang, SORT_DEFAULT), LOCALE_SENGLANGUAGE, szLang, nSize );
@@ -2177,6 +2181,7 @@ DWORD WINAPI VerLanguageNameA( DWORD wLang, LPSTR szLang, DWORD nSize )
  *
  * See VerLanguageNameA.
  */
+#undef VerLanguageNameW
 DWORD WINAPI VerLanguageNameW( DWORD wLang, LPWSTR szLang, DWORD nSize )
 {
     return GetLocaleInfoW( MAKELCID(wLang, SORT_DEFAULT), LOCALE_SENGLANGUAGE, szLang, nSize );
diff --git a/include/winver.h b/include/winver.h
index f7e887b..cb06852 100644
--- a/include/winver.h
+++ b/include/winver.h
@@ -170,6 +170,23 @@ BOOL        WINAPI GetFileVersionInfoW(LPCWSTR,DWORD,DWORD,LPVOID);
 #define     GetFileVersionInfo WINELIB_NAME_AW(GetFileVersionInfo)
 
 /* 20 GETFILEVERSIONINFORAW */
+#if defined(__GNUC__) && (__GNUC__ >= 4) && defined(__OPTIMIZE__)
+#define make_checker(func,type,arglist1,arglist2,namebuf,namebuflen,charsize)	\
+extern void func##_sizeincorrect(void);						\
+static inline type func##_ichk arglist1  {					\
+	int namebuf##len = __builtin_object_size(namebuf, 0);			\
+										\
+	if (__builtin_constant_p(namebuflen) && (namebuf##len != -1) && (namebuflen*charsize > namebuf##len)) \
+		func##_sizeincorrect();						\
+	return func arglist2 ;							\
+}										
+make_checker(VerLanguageNameA,DWORD WINAPI,(DWORD a,LPSTR b,DWORD c),(a,b,c),b,c,1)
+#define VerLanguageNameA VerLanguageNameA_ichk
+make_checker(VerLanguageNameW,DWORD WINAPI,(DWORD a,LPWSTR b,DWORD c),(a,b,c),b,c,2)
+#define VerLanguageNameW VerLanguageNameW_ichk
+
+#endif /* overflow checker */
+
 
 #endif /* RC_INVOKED */
 
-- 
1.5.6