Merge branch 'fix-and-run-tests-with-asan'

Fix issues found by address/leak sanitizers in the unit tests and add Travis CI build running tests built with ASAN to ensure they don't reappear in the future. See https://github.com/wxWidgets/wxWidgets/pull/2086
2020-10-19 21:16:27 +02:00
parent fec33079ed a2923a6fad
commit 4abb816f70
14 changed files with 151 additions and 27 deletions
--- a/.travis.yml
+++ b/.travis.yml
@@ -36,8 +36,12 @@ jobs:
          name: wxGTK 3 CMake Ubuntu 18.04
        - dist: focal
          compiler: gcc
-          env: wxCONFIGURE_FLAGS="--disable-compat30 --disable-optimise --disable-unicode" wxSKIP_SAMPLES=1
+          env: wxGTK_VERSION=2 wxCONFIGURE_FLAGS="--disable-compat30 --disable-unicode" wxSKIP_SAMPLES=1
          name: wxGTK ANSI Ubuntu 20.04
+        - dist: focal
+          compiler: gcc
+          env: wxGTK_VERSION=3 wxCONFIGURE_FLAGS="--disable-compat30 --disable-sys-libs" wxSKIP_SAMPLES=1 wxUSE_ASAN=1
+          name: wxGTK Ubuntu 20.04 with ASAN
        - os: osx
          osx_image: xcode7.3
          compiler: clang
@@ -78,7 +82,7 @@ jobs:
          name: wxQt Ubuntu 18.04
        - os: linux
          arch: arm64
-          env: wxCONFIGURE_FLAGS="--disable-sys-libs" wxLXC=1
+          env: wxGTK_VERSION=3 wxCONFIGURE_FLAGS="--disable-sys-libs" wxLXC=1
          name: wxGTK ARM64
        - os: linux
          arch: ppc64le
@@ -94,7 +98,7 @@ jobs:
    allow_failures:
        - os: linux
          arch: arm64
-          env: wxCONFIGURE_FLAGS="--disable-sys-libs" wxLXC=1
+          env: wxGTK_VERSION=3 wxCONFIGURE_FLAGS="--disable-sys-libs" wxLXC=1
          name: wxGTK ARM64
        - os: linux
          arch: ppc64le
--- a/build/tools/before_install.sh
+++ b/build/tools/before_install.sh
@@ -22,9 +22,12 @@ case $(uname -s) in
                        3)  libtoolkit_dev=libgtk-3-dev
                            extra_deps='libwebkit2gtk-4.0-dev libwebkitgtk-3.0-dev'
                            ;;
-                        *)  libtoolkit_dev=libgtk2.0-dev
+                        2)  libtoolkit_dev=libgtk2.0-dev
                            extra_deps='libwebkitgtk-dev'
                            ;;
+                        *)  echo 'Please specify wxGTK_VERSION explicitly.' >&2
+                            exit 1
+                            ;;
                    esac

                    extra_deps="$extra_deps \
@@ -42,6 +45,24 @@ case $(uname -s) in
            done

            $SUDO apt-get install -y $libtoolkit_dev $pkg_install
+
+            if [ "$wxUSE_ASAN" = 1 ]; then
+                codename=$(lsb_release --codename --short)
+                # Enable the `-dbgsym` repositories.
+                echo "deb http://ddebs.ubuntu.com ${codename} main restricted universe multiverse
+                deb http://ddebs.ubuntu.com ${codename}-updates main restricted universe multiverse
+                deb http://ddebs.ubuntu.com ${codename}-proposed main restricted universe multiverse" | \
+                $SUDO tee --append /etc/apt/sources.list.d/ddebs.list
+
+                # Import the debug symbol archive signing key from the Ubuntu server.
+                # Note that this command works only on Ubuntu 18.04 LTS and newer.
+                $SUDO apt-get install -y ubuntu-dbgsym-keyring
+
+                $SUDO apt-get update
+
+                # Install the symbols to allow LSAN suppression list to work.
+                $SUDO apt-get install -y libfontconfig1-dbgsym
+            fi
        fi
        ;;

--- a/build/tools/travis-ci.sh
+++ b/build/tools/travis-ci.sh
@@ -8,6 +8,10 @@ wxPROC_COUNT=`getconf _NPROCESSORS_ONLN`
 ((wxPROC_COUNT++))
 wxBUILD_ARGS="-j$wxPROC_COUNT"

+# Setting this variable suppresses "Error retrieving accessibility bus address"
+# messages from WebKit tests that we're not interested in.
+export NO_AT_BRIDGE=1
+
 case $wxTOOLSET in
    cmake)
        if [ -z $wxCMAKE_TESTS ]; then wxCMAKE_TESTS=CONSOLE_ONLY; fi
@@ -17,6 +21,11 @@ case $wxTOOLSET in
        fi
        cmake --version

+        if [ "$wxUSE_ASAN" = 1 ]; then
+            echo "ASAN currently isn't supported in CMake builds"
+            exit 1
+        fi
+
        echo 'travis_fold:start:configure'
        echo 'Configuring...'
        mkdir build_cmake
@@ -57,7 +66,24 @@ case $wxTOOLSET in
    *)
        echo 'travis_fold:start:configure'
        echo 'Configuring...'
-        ./configure --disable-optimise --disable-debug_info $wxCONFIGURE_FLAGS || rc=$?
+
+        wxCONFIGURE_OPTIONS="--disable-optimise $wxCONFIGURE_FLAGS"
+        if [ -n "$wxGTK_VERSION" ]; then
+            wxCONFIGURE_OPTIONS="--with-gtk=$wxGTK_VERSION $wxCONFIGURE_OPTIONS"
+        fi
+
+        if [ "$wxUSE_ASAN" = 1 ]; then
+            export LSAN_OPTIONS=suppressions=$(pwd)/misc/suppressions/lsan
+
+            wxASAN_CFLAGS="-fsanitize=address -fno-omit-frame-pointer"
+            wxASAN_CXXFLAGS=$wxASAN_CFLAGS
+            wxASAN_LDFLAGS="-fsanitize=address"
+
+            ./configure $wxCONFIGURE_OPTIONS --enable-debug "CFLAGS=$wxASAN_CFLAGS" "CXXFLAGS=$wxASAN_CXXFLAGS" "LDFLAGS=$wxASAN_LDFLAGS" || rc=$?
+        else
+            ./configure $wxCONFIGURE_OPTIONS --disable-debug_info || rc=$?
+        fi
+
        if [ -n "$rc" ]; then
            echo '*** Configuring failed, contents of config.log follows: ***'
            echo '-----------------------------------------------------------'
--- a/include/wx/gtk/private/webkit.h
+++ b/include/wx/gtk/private/webkit.h
@@ -29,7 +29,8 @@ public:

    ~wxWebKitJavascriptResult()
    {
-        webkit_javascript_result_unref(m_jsresult);
+        if ( m_jsresult != NULL )
+            webkit_javascript_result_unref(m_jsresult);
    }

    operator WebKitJavascriptResult *() const { return m_jsresult; }
--- a/include/wx/sizer.h
+++ b/include/wx/sizer.h
@@ -321,6 +321,10 @@ public:
    // Enable deleting the SizerItem without destroying the contained sizer.
    void DetachSizer() { m_sizer = NULL; }

+    // Enable deleting the SizerItem without resetting the sizer in the
+    // contained window.
+    void DetachWindow() { m_window = NULL; m_kind = Item_None; }
+
    virtual wxSize GetSize() const;
    virtual wxSize CalcMin();
    virtual void SetDimension( const wxPoint& pos, const wxSize& size );
--- a/interface/wx/any.h
+++ b/interface/wx/any.h
@@ -84,6 +84,25 @@ public:
                 cannot be converted to a specific data type, wxAny will then
                 hold and manage reference to wxVariantData* similar to how
                 wxVariant does.
+
+                 Note that objects constructed from list-valued variants
+                 require the list to be explicitly cleared using `WX_CLEAR_LIST`
+                 to avoid leaking memory. This unfortunate behaviour will not
+                 be changed to prevent breaking the existing code relying on it.
+
+                 @code
+                 wxVariant vList;
+                 vList.NullList();
+                 vList.Append(15);
+                 vList.Append("abc");
+
+                 // Create wxAny from the list variant.
+                 wxAny any = wxAny(vList);
+
+                 // Clear the list to avoid the memory leak.
+                 wxAnyList anyList = any.As<wxAnyList>();
+                 WX_CLEAR_LIST(wxAnyList, anyList);
+                 @endcode
    */
    wxAny(const wxVariant& variant);

--- a/misc/suppressions/lsan
+++ b/misc/suppressions/lsan
@@ -0,0 +1,10 @@
+# Leak sanitizer suppressions for wx, use it by setting
+# LSAN_OPTIONS=suppressions=<path-to-this-file>
+
+# Known leaks in libfontconfig.so.1: note that you must have its debug symbols
+# installed for these suppressions to work.
+leak:FcConfigValues
+leak:FcLangSetCreate
+leak:FcPatternObjectInsertElt
+leak:FcValueListCreate
+leak:FcValueSave
--- a/src/common/sizer.cpp
+++ b/src/common/sizer.cpp
@@ -32,6 +32,7 @@
 #include "wx/vector.h"
 #include "wx/listimpl.cpp"
 #include "wx/private/window.h"
+#include "wx/scopedptr.h"


 //---------------------------------------------------------------------------
@@ -673,7 +674,35 @@ wxSizer::~wxSizer()

 wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item )
 {
-    m_children.Insert( index, item );
+    // The helper class that solves two problems when
+    // wxWindowBase::SetContainingSizer() throws:
+    // 1. Avoid leaking memory using the scoped pointer to the sizer item.
+    // 2. Disassociate the window from the sizer item to not reset the
+    //    containing sizer for the window in the items destructor.
+    class ContainingSizerGuard
+    {
+    public:
+        explicit ContainingSizerGuard( wxSizerItem *item )
+            : m_item(item)
+        {
+        }
+
+        ~ContainingSizerGuard()
+        {
+            if ( m_item )
+                m_item->DetachWindow();
+        }
+
+        wxSizerItem* Release()
+        {
+            return m_item.release();
+        }
+
+    private:
+        wxScopedPtr<wxSizerItem> m_item;
+    };
+
+    ContainingSizerGuard guard( item );

    if ( item->GetWindow() )
        item->GetWindow()->SetContainingSizer( this );
@@ -681,7 +710,9 @@ wxSizerItem* wxSizer::DoInsert( size_t index, wxSizerItem *item )
    if ( item->GetSizer() )
        item->GetSizer()->SetContainingWindow( m_containingWindow );

-    return item;
+    m_children.Insert( index, item );
+
+    return guard.Release();
 }

 void wxSizer::SetContainingWindow(wxWindow *win)
@@ -1449,6 +1480,9 @@ wxGridSizer::wxGridSizer( int rows, int cols, const wxSize& gap )

 wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item)
 {
+    // Ensure that the item will be deleted in case of exception.
+    wxScopedPtr<wxSizerItem> scopedItem( item );
+
    // if only the number of columns or the number of rows is specified for a
    // sizer, arbitrarily many items can be added to it but if both of them are
    // fixed, then the sizer can't have more than that many items -- check for
@@ -1489,7 +1523,7 @@ wxSizerItem *wxGridSizer::DoInsert(size_t index, wxSizerItem *item)
        );
    }

-    return wxSizer::DoInsert(index, item);
+    return wxSizer::DoInsert( index, scopedItem.release() );
 }

 int wxGridSizer::CalcRowsCols(int& nrows, int& ncols) const
--- a/tests/any/anytest.cpp
+++ b/tests/any/anytest.cpp
@@ -652,6 +652,8 @@ void wxAnyTestCase::wxVariantConversions()
    CPPUNIT_ASSERT(variant.GetCount() == 2);
    CPPUNIT_ASSERT(variant[0].GetLong() == 15);
    CPPUNIT_ASSERT(variant[1].GetString() == "abc");
+    // Avoid the memory leak.
+    WX_CLEAR_LIST(wxAnyList, anyList);

    any = wxAny(vCustomType);
    CPPUNIT_ASSERT(wxANY_CHECK_TYPE(any, wxVariantData*));
--- a/tests/filesys/filesystest.cpp
+++ b/tests/filesys/filesystest.cpp
@@ -22,6 +22,7 @@
 #if wxUSE_FILESYSTEM

 #include "wx/fs_mem.h"
+#include "wx/scopedptr.h"

 // ----------------------------------------------------------------------------
 // helpers
@@ -186,16 +187,16 @@ TEST_CASE("wxFileSystem::MemoryFSHandler", "[filesys][memoryfshandler][find]")
        AutoMemoryFSHandler()
            : m_handler(new wxMemoryFSHandler())
        {
-            wxFileSystem::AddHandler(m_handler);
+            wxFileSystem::AddHandler(m_handler.get());
        }

        ~AutoMemoryFSHandler()
        {
-            wxFileSystem::RemoveHandler(m_handler);
+            wxFileSystem::RemoveHandler(m_handler.get());
        }

    private:
-        wxMemoryFSHandler* const m_handler;
+        wxScopedPtr<wxMemoryFSHandler> const m_handler;
    } autoMemoryFSHandler;

    wxMemoryFSHandler::AddFile("foo.txt", "foo contents");
--- a/tests/html/htmlparser.cpp
+++ b/tests/html/htmlparser.cpp
@@ -20,15 +20,13 @@
 #endif // WX_PRECOMP

 #include "wx/html/winpars.h"
+#include "wx/scopedptr.h"

 // Test that parsing invalid HTML simply fails but doesn't crash for example.
 TEST_CASE("wxHtmlParser::ParseInvalid", "[html][parser][error]")
 {
    class NullParser : public wxHtmlWinParser
    {
-    public:
-        virtual wxObject *GetProduct() wxOVERRIDE { return NULL; }
-
    protected:
        virtual void AddText(const wxString& WXUNUSED(txt)) wxOVERRIDE { }
    };
@@ -37,17 +35,17 @@ TEST_CASE("wxHtmlParser::ParseInvalid", "[html][parser][error]")
    wxMemoryDC dc;
    p.SetDC(&dc);

-    p.Parse("<");
-    p.Parse("<foo");
-    p.Parse("<!--");
-    p.Parse("<!---");
+    delete p.Parse("<");
+    delete p.Parse("<foo");
+    delete p.Parse("<!--");
+    delete p.Parse("<!---");
 }

 TEST_CASE("wxHtmlCell::Detach", "[html][cell]")
 {
    wxMemoryDC dc;

-    wxHtmlContainerCell* const top = new wxHtmlContainerCell(NULL);
+    wxScopedPtr<wxHtmlContainerCell> const top(new wxHtmlContainerCell(NULL));
    wxHtmlContainerCell* const cont = new wxHtmlContainerCell(NULL);
    wxHtmlCell* const cell1 = new wxHtmlWordCell("Hello", dc);
    wxHtmlCell* const cell2 = new wxHtmlColourCell(*wxRED);
--- a/tests/mbconv/mbconvtest.cpp
+++ b/tests/mbconv/mbconvtest.cpp
@@ -1147,7 +1147,7 @@ void MBConvTestCase::TestDecoder(
    // make sure the correct output length was calculated
    WX_ASSERT_EQUAL_MESSAGE
    (
-        ("while converting \"%s\"", multiBuffer),
+        ("while converting \"%s\"", inputCopy),
        wideChars,
        outputWritten
    );
--- a/tests/menu/accelentry.cpp
+++ b/tests/menu/accelentry.cpp
@@ -17,6 +17,7 @@
 #endif // WX_PRECOMP

 #include "wx/accel.h"
+#include "wx/scopedptr.h"

 namespace
 {
@@ -35,11 +36,11 @@ void CheckAccelEntry(const wxAcceleratorEntry& accel, int keycode, int flags)
 */
 TEST_CASE( "wxAcceleratorEntry::Create", "[accelentry]" )
 {
-    wxAcceleratorEntry* pa;
+    wxScopedPtr<wxAcceleratorEntry> pa;

    SECTION( "Correct behavior" )
    {
-        pa = wxAcceleratorEntry::Create("Foo\tCtrl+Z");
+        pa.reset( wxAcceleratorEntry::Create("Foo\tCtrl+Z") );

        CHECK( pa );
        CHECK( pa->IsOk() );
@@ -48,21 +49,21 @@ TEST_CASE( "wxAcceleratorEntry::Create", "[accelentry]" )

    SECTION( "Tab missing" )
    {
-        pa = wxAcceleratorEntry::Create("Shift-Q");
+        pa.reset( wxAcceleratorEntry::Create("Shift-Q") );

        CHECK( !pa );
    }

    SECTION( "No accelerator key specified" )
    {
-        pa = wxAcceleratorEntry::Create("bloordyblop");
+        pa.reset( wxAcceleratorEntry::Create("bloordyblop") );

        CHECK( !pa );
    }

    SECTION( "Display name parsing" )
    {
-        pa = wxAcceleratorEntry::Create("Test\tBackSpace");
+        pa.reset( wxAcceleratorEntry::Create("Test\tBackSpace") );

        CHECK( pa );
        CHECK( pa->IsOk() );
--- a/tests/menu/menu.cpp
+++ b/tests/menu/menu.cpp
@@ -20,6 +20,7 @@
 #endif // WX_PRECOMP

 #include "wx/menu.h"
+#include "wx/scopedptr.h"
 #include "wx/translation.h"
 #include "wx/uiaction.h"

@@ -624,7 +625,9 @@ namespace

 void VerifyAccelAssigned( wxString labelText, int keycode )
 {
-    wxAcceleratorEntry* entry = wxAcceleratorEntry::Create( labelText );
+    const wxScopedPtr<wxAcceleratorEntry> entry(
+        wxAcceleratorEntry::Create( labelText )
+    );

    CHECK( entry );
    CHECK( entry->GetKeyCode() == keycode );