maildev@lists.thunderbird.net

Thunderbird email developers

View all threads

Building C-C TB with gcc-10 and -Werror and -Wformat, etc.

I
ISHIKAWA,chiaki
Wed, Dec 16, 2020 9:18 AM

I have been able to create a local debug build C-C TB with gcc-10.
But caveat emptor.

I apply rather strict checking at compile time and abort compilation.
I think I have applied the following flags to CXXFLAGS and CFLAGS during
compilation.

-Werror -Wsign-compare -Wunused-result -Wunused-variable -Wformat

The configure script whitelists some warnings by adding -Wno-error=...
flags explicitly from what I see the real flags used during build.

With this strict checking I notice many dubious code both in M-C and C-C
(many in ldap code).
But the strict compile-time checking of snprintf() string buffer
overflow of GCC-10 has a bug.

I need to apply the following local patch to create the debug build of
C-C TB locally with the said flags.

HG changeset patch

User ISHIKAWA, Chiaki ishikawa@yk.rim.or.jp

Parent  c989a1970f120c98cba1bb2010b81d87308ea2d7

avoid gcc snprintf format string check bug.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281

diff --git a/calendar/libical/src/libical/icalvalue.c
b/calendar/libical/src/libical/icalvalue.c
--- a/calendar/libical/src/libical/icalvalue.c
+++ b/calendar/libical/src/libical/icalvalue.c
@@ -872,21 +872,28 @@ static char* icalvalue_utcoffset_as_ical
     } else {
     sign = '-';
     }

     h = data/3600;
     m = (data - (h3600))/ 60;
     s = (data - (h
3600) - (m*60));

-    if (s > 0)
-    snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s));
-    else
-    snprintf(str,9,"%c%02d%02d",sign,abs(h),abs(m));

+    if ((0 <= m && m <= 60) && (0 <= s && s <= 60) && (0 <= abs(h) &&
abs(h) <= 24)) {
+        if (s > 0)

  • snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s));
    +        else
    +            snprintf(str,9,"%c%02d%02d",sign,abs(h),abs(m));
    +    } else    {
    +#ifdef DEBUG
    +        icalerror_assert((0 <= m && m <= 60) &&
    +                         (0 <= s && s <= 60) && (0 <= abs(h) && abs(h)
    <=24) ,
    +                         "Minutes & seconds must be in [0, 60) abs(h)
    must be in [0, 24)");
    +#endif
    +    }
         return str;
     }

 static char* icalvalue_string_as_ical_string_r(const icalvalue* value) {

     const char* data;
     char* str = 0;
     icalerror_check_arg_rz( (value!=0),"value");

Withtou the patch, the above code produced a possible buffer overflow
warning and fails with -Werror flag.
Obviously, the compile value propagation somewhere went wrong.
A similar bug was reported early this year against gcc-9, but that seems
to have been fixed.
However, obviously gcc-10 resurfaced the bug in a slightly different place.

Just  thought to share this.

Chiaki

I have been able to create a local debug build C-C TB with gcc-10. But caveat emptor. I apply rather strict checking at compile time and abort compilation. I think I have applied the following flags to CXXFLAGS and CFLAGS during compilation. -Werror -Wsign-compare -Wunused-result -Wunused-variable -Wformat The configure script whitelists some warnings by adding -Wno-error=... flags explicitly from what I see the real flags used during build. With this strict checking I notice many dubious code both in M-C and C-C (many in ldap code). But the strict compile-time checking of snprintf() string buffer overflow of GCC-10 has a bug. I need to apply the following local patch to create the debug build of C-C TB locally with the said flags. # HG changeset patch # User ISHIKAWA, Chiaki <ishikawa@yk.rim.or.jp> # Parent  c989a1970f120c98cba1bb2010b81d87308ea2d7 avoid gcc snprintf format string check bug. https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98281 diff --git a/calendar/libical/src/libical/icalvalue.c b/calendar/libical/src/libical/icalvalue.c --- a/calendar/libical/src/libical/icalvalue.c +++ b/calendar/libical/src/libical/icalvalue.c @@ -872,21 +872,28 @@ static char* icalvalue_utcoffset_as_ical      } else {      sign = '-';      }      h = data/3600;      m = (data - (h*3600))/ 60;      s = (data - (h*3600) - (m*60)); -    if (s > 0) -    snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s)); -    else -    snprintf(str,9,"%c%02d%02d",sign,abs(h),abs(m)); - +    if ((0 <= m && m <= 60) && (0 <= s && s <= 60) && (0 <= abs(h) && abs(h) <= 24)) { +        if (s > 0) + snprintf(str,9,"%c%02d%02d%02d",sign,abs(h),abs(m),abs(s)); +        else +            snprintf(str,9,"%c%02d%02d",sign,abs(h),abs(m)); +    } else    { +#ifdef DEBUG +        icalerror_assert((0 <= m && m <= 60) && +                         (0 <= s && s <= 60) && (0 <= abs(h) && abs(h) <=24) , +                         "Minutes & seconds must be in [0, 60) abs(h) must be in [0, 24)"); +#endif +    }      return str;  }  static char* icalvalue_string_as_ical_string_r(const icalvalue* value) {      const char* data;      char* str = 0;      icalerror_check_arg_rz( (value!=0),"value"); Withtou the patch, the above code produced a possible buffer overflow warning and fails with -Werror flag. Obviously, the compile value propagation somewhere went wrong. A similar bug was reported early this year against gcc-9, but that seems to have been fixed. However, obviously gcc-10 resurfaced the bug in a slightly different place. Just  thought to share this. Chiaki