[Libreoffice-commits] core.git: 3 commits - config_host.mk.in configure.ac hwpfilter/source include/vcl solenv/gbuild vcl/source

Libreoffice Gerrit user logerrit at kemper.freedesktop.org
Mon Jan 21 08:38:35 UTC 2019


 config_host.mk.in                      |    1 
 configure.ac                           |   57 +++++++++++++++++++++++++++++++++
 hwpfilter/source/hfont.cxx             |    4 +-
 hwpfilter/source/hstyle.cxx            |   13 ++++++-
 include/vcl/pngread.hxx                |    2 -
 solenv/gbuild/platform/com_GCC_defs.mk |    4 ++
 vcl/source/gdi/pngread.cxx             |    2 +
 7 files changed, 80 insertions(+), 3 deletions(-)

New commits:
commit fed7c3deb3f4ec81f78967c2d7f3c4554398cb9d
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jan 18 16:13:57 2019 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Jan 21 09:38:27 2019 +0100

    Slience bogus -Werror=maybe-uninitialized
    
    ...as emitted by at least GCC 8.2 with --enable-optimized, e.g. at:
    
    > In file included from include/rtl/ustring.hxx:37,
    >                  from include/cppuhelper/factory.hxx:26,
    >                  from unoxml/source/rdf/librdf_repository.hxx:24,
    >                  from unoxml/source/rdf/librdf_repository.cxx:20:
    > include/rtl/string.hxx: In static member function ‘static std::shared_ptr<{anonymous}::librdf_TypeConverter::Node> {anonymous}::librdf_TypeConverter::extractNode_NoLock(const com::sun::star::uno::Reference<com::sun::star::rdf::XNode>&)’:
    > include/rtl/string.hxx:294:27: error: ‘*((void*)(& type)+8).rtl::OString::pData’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    >          rtl_string_release( pData );
    >          ~~~~~~~~~~~~~~~~~~^~~~~~~~~
    > unoxml/source/rdf/librdf_repository.cxx:2094:30: note: ‘*((void*)(& type)+8).rtl::OString::pData’ was declared here
    >      boost::optional<OString> type;
    >                               ^~~~
    
    This appears to be a common pattern of false positives with uses of
    boost::optional, common enough to disable the warning globally for affected
    compilers, even if there would also be useful findings by that warning (e.g.,
    <https://gerrit.libreoffice.org/#/c/66619/> "Fix -Werror=maybe-uninitialized").
    
    I didn't bother to file a GCC bug for the reproducer used in configure.ac,
    <https://gcc.gnu.org/bugzilla/buglist.cgi?quicksearch=maybe-uninitialized>
    already shows lots of open bugs in that area, and the documentation at
    <https://gcc.gnu.org/onlinedocs/gcc-8.2.0/gcc/Warning-Options.html> states that
    "GCC may not be able to determine when the code is correct in spite of appearing
    to have an error."
    
    (Clang appears to not support -Wmaybe-uninitialized at all, so exclude it from
    the configure.ac check, to not have the check's failure result in an unsupported
    -Wno-maybe-uninitialized end up on the compiler command line.)
    
    Change-Id: Ifb9ca4c342750eae54f7e1a01506101310484c7e
    Reviewed-on: https://gerrit.libreoffice.org/66621
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/config_host.mk.in b/config_host.mk.in
index 55a4c00c8219..aa64c32b77ab 100644
--- a/config_host.mk.in
+++ b/config_host.mk.in
@@ -244,6 +244,7 @@ export GTK_PRINT_CFLAGS=$(gb_SPACE)@GTK_PRINT_CFLAGS@
 export GTK_PRINT_LIBS=$(gb_SPACE)@GTK_PRINT_LIBS@
 export USING_X11=@USING_X11@
 export HAMCREST_JAR=@HAMCREST_JAR@
+export HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED=@HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED@
 export HAVE_GCC_AVX=@HAVE_GCC_AVX@
 export HAVE_GCC_STACK_PROTECTOR_STRONG=@HAVE_GCC_STACK_PROTECTOR_STRONG@
 export HAVE_GCC_BUILTIN_ATOMIC=@HAVE_GCC_BUILTIN_ATOMIC@
diff --git a/configure.ac b/configure.ac
index fe0d792ba7fe..19f1f5803d5e 100644
--- a/configure.ac
+++ b/configure.ac
@@ -6540,6 +6540,63 @@ AC_COMPILE_IFELSE([AC_LANG_SOURCE([
 CXXFLAGS=$save_CXXFLAGS
 AC_LANG_POP([C++])
 
+dnl At least GCC 8.2 with -O2 (i.e., --enable-optimized) causes a false-positive -Wmaybe-
+dnl uninitialized warning for code like
+dnl
+dnl   OString f();
+dnl   boost::optional<OString> * g(bool b) {
+dnl       boost::optional<OString> o;
+dnl       if (b) o = f();
+dnl       return new boost::optional<OString>(o);
+dnl   }
+dnl
+dnl (as is e.g. present, in a slightly more elaborate form, in
+dnl librdf_TypeConverter::extractNode_NoLock in unoxml/source/rdf/librdf_repository.cxx); the below
+dnl code is meant to be a faithfully stripped-down and self-contained version of the above code:
+HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED=
+if test "$GCC" = yes && test "$COM_IS_CLANG" != TRUE; then
+    AC_MSG_CHECKING([whether $CXX might report false -Werror=maybe-uninitialized])
+    AC_LANG_PUSH([C++])
+    save_CXXFLAGS=$CXXFLAGS
+    CXXFLAGS="$CXXFLAGS $CXXFLAGS_CXX11 -Werror -Wmaybe-uninitialized"
+    if test "$ENABLE_OPTIMIZED" = TRUE; then
+        CXXFLAGS="$CXXFLAGS -O2"
+    else
+        CXXFLAGS="$CXXFLAGS -O0"
+    fi
+    AC_COMPILE_IFELSE([AC_LANG_SOURCE([[
+            #include <new>
+            void f1(int);
+            struct S1 {
+                ~S1() { f1(n); }
+                int n = 0;
+            };
+            struct S2 {
+                S2() {}
+                S2(S2 const & s) { if (s.init) set(*reinterpret_cast<S1 const *>(s.stg)); }
+                ~S2() { if (init) reinterpret_cast<S1 *>(stg)->S1::~S1(); }
+                void set(S1 s) {
+                    new (stg) S1(s);
+                    init = true;
+                }
+                bool init = false;
+                char stg[sizeof (S1)];
+            } ;
+            S1 f2();
+            S2 * f3(bool b) {
+                S2 o;
+                if (b) o.set(f2());
+                return new S2(o);
+            }
+        ]])], [AC_MSG_RESULT([no])], [
+            HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED=TRUE
+            AC_MSG_RESULT([yes])
+        ])
+    CXXFLAGS=$save_CXXFLAGS
+    AC_LANG_POP([C++])
+fi
+AC_SUBST([HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED])
+
 dnl ===================================================================
 dnl system stl sanity tests
 dnl ===================================================================
diff --git a/solenv/gbuild/platform/com_GCC_defs.mk b/solenv/gbuild/platform/com_GCC_defs.mk
index 1048a1375327..14d82ccdc0e2 100644
--- a/solenv/gbuild/platform/com_GCC_defs.mk
+++ b/solenv/gbuild/platform/com_GCC_defs.mk
@@ -79,6 +79,10 @@ gb_CXXFLAGS_COMMON := \
 	-fno-common \
 	-pipe \
 
+ifeq ($(HAVE_BROKEN_GCC_WMAYBE_UNINITIALIZED),TRUE)
+gb_CXXFLAGS_COMMON += -Wno-maybe-uninitialized
+endif
+
 gb_CXXFLAGS_Wundef = -Wno-undef
 
 ifeq ($(ENABLE_GDB_INDEX),TRUE)
commit 26688cf08988ad80f7412d00dba749faa06832ad
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jan 18 16:11:27 2019 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Jan 21 09:38:13 2019 +0100

    Avoid -Werror=stringop-truncation
    
    ...as emitted by at least GCC 8.2 with --enable-optimized.  These are not
    actually problems, as in both cases strncpy(p,...,N-1) is used to copy a
    string into an area p of length N that has initially been zero-initialized, so
    p[N-1] will already contain NUL.  But add the (redundant) p[N-1]=NUL just in
    case, and to silence GCC (where the documentation explicitly recommends this
    additional write as a way to silence the warning).  Unfortunately, in hstyle.cxx
    at least GCC 8.2 with --enable-optimized would still emit the warning, even
    though it uses the recommended way of handling it, so needs to be silenced with
    a #pragma.
    
    Change-Id: Ie41f420c732c2bfb699903ebd21ce1a89dd2b236
    Reviewed-on: https://gerrit.libreoffice.org/66620
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/hwpfilter/source/hfont.cxx b/hwpfilter/source/hfont.cxx
index e5c23a88199b..1a96b1797463 100644
--- a/hwpfilter/source/hfont.cxx
+++ b/hwpfilter/source/hfont.cxx
@@ -47,7 +47,9 @@ void HWPFont::AddFont(int lang, const char *font)
     nfonts = nFonts[lang];
     if (MAXFONTS <= nfonts)
         return;
-    strncpy(fontnames[lang].get() + FONTNAMELEN * nfonts, font, FONTNAMELEN - 1);
+    auto const p = fontnames[lang].get() + FONTNAMELEN * nfonts;
+    strncpy(p, font, FONTNAMELEN - 1);
+    p[FONTNAMELEN - 1] = '\0'; // just in case, even though the array is zero-initialized
     nFonts[lang]++;
 }
 
diff --git a/hwpfilter/source/hstyle.cxx b/hwpfilter/source/hstyle.cxx
index cc3ef07cc987..87fd1efbe978 100644
--- a/hwpfilter/source/hstyle.cxx
+++ b/hwpfilter/source/hstyle.cxx
@@ -66,7 +66,18 @@ void HWPStyle::SetName(int n, char const *name)
     if (n >= 0 && n < nstyles)
     {
         if (name)
-            strncpy(DATA[n].name, name, MAXSTYLENAME);
+        {
+#if defined __GNUC__ && __GNUC__ == 8 && __GNUC_MINOR__ == 2 && !defined __clang__
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wstringop-truncation"
+#endif
+            auto const p = DATA[n].name;
+            strncpy(p, name, MAXSTYLENAME);
+            p[MAXSTYLENAME] = '\0'; // just in case, even though the array is zero-initialized
+#if defined __GNUC__ && __GNUC__ == 8 && __GNUC_MINOR__ == 2 && !defined __clang__
+#pragma GCC diagnostic pop
+#endif
+        }
         else
             DATA[n].name[0] = 0;
     }
commit 3a001a588c43cf8c2c3f6cb6a6796cc1bf8e2683
Author:     Stephan Bergmann <sbergman at redhat.com>
AuthorDate: Fri Jan 18 15:58:58 2019 +0100
Commit:     Stephan Bergmann <sbergman at redhat.com>
CommitDate: Mon Jan 21 09:38:01 2019 +0100

    Fix -Werror=maybe-uninitialized
    
    ...as emitted by at least GCC 8.2 with --enable-optimized:
    
    > In file included from vcl/source/gdi/pngread.cxx:27:
    > include/vcl/pngread.hxx: In member function ‘bool vcl::PNGReaderImpl::ReadNextChunk()’:
    > include/vcl/pngread.hxx:49:12: error: ‘aDummyChunk.vcl::PNGReader::ChunkData::nType’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
    >      struct ChunkData
    >             ^~~~~~~~~
    
    The immediately relevant part is initializing
    
            sal_uInt32 nType = 0;
    
    in the ChunkData ctor, but while at it, also make the Read*Int32 calls in
    PNGReaderImpl::ReadNextChunk more robust.
    
    Change-Id: Id6ad10a4382a8d063fd70c7bac595c3693475ca3
    Reviewed-on: https://gerrit.libreoffice.org/66619
    Tested-by: Jenkins
    Reviewed-by: Stephan Bergmann <sbergman at redhat.com>

diff --git a/include/vcl/pngread.hxx b/include/vcl/pngread.hxx
index 8992709b3349..31592d9f17ac 100644
--- a/include/vcl/pngread.hxx
+++ b/include/vcl/pngread.hxx
@@ -48,7 +48,7 @@ public:
     // retrieve every chunk that resides inside the PNG
     struct ChunkData
     {
-        sal_uInt32 nType;
+        sal_uInt32 nType = 0;
         std::vector<sal_uInt8> aData;
     };
     const std::vector<ChunkData>& GetChunks() const;
diff --git a/vcl/source/gdi/pngread.cxx b/vcl/source/gdi/pngread.cxx
index 78195d5e7c46..b5ac9e430596 100644
--- a/vcl/source/gdi/pngread.cxx
+++ b/vcl/source/gdi/pngread.cxx
@@ -273,6 +273,8 @@ bool PNGReaderImpl::ReadNextChunk()
         PNGReader::ChunkData& rChunkData = *maChunkIter;
 
         // read the chunk header
+        mnChunkLen = 0;
+        mnChunkType = 0;
         mrPNGStream.ReadInt32( mnChunkLen ).ReadUInt32( mnChunkType );
         rChunkData.nType = mnChunkType;
 


More information about the Libreoffice-commits mailing list