[Libreoffice-commits] core.git: unoxml/source

Stephan Bergmann sbergman at redhat.com
Fri Apr 7 08:21:35 UTC 2017


 unoxml/source/dom/documentbuilder.cxx |   32 +++++++++++++++++++++-----------
 1 file changed, 21 insertions(+), 11 deletions(-)

New commits:
commit d78040aadb6a8e3eeee1652b62a06d6e7e9b28ce
Author: Stephan Bergmann <sbergman at redhat.com>
Date:   Fri Apr 7 10:17:49 2017 +0200

    ASan stack-use-after-scope
    
    ...as reported during CppunitTest_extensions_test_update (see below).  I don't
    see a reason why those pContext should be shared_ptr, so better make them
    unique_ptr to guarantee that xmlFreeParserCtxt is called with c still in scope
    (in CDocumentBuilder::parse).
    
    > ==10883==ERROR: AddressSanitizer: stack-use-after-scope on address 0x2b111da94b68 at pc 0x2b116e55a200 bp 0x7fff7228b0f0 sp 0x7fff7228b0e8
    > READ of size 8 at 0x2b111da94b68 thread T0
    >     #0 0x2b116e55a1ff in com::sun::star::uno::BaseReference::is() const include/com/sun/star/uno/Reference.h:94:27
    >     #1 0x2b116e699756 in DOM::xmlIO_close_func(void*) unoxml/source/dom/documentbuilder.cxx:214:33
    >     #2 0x2b11300cf646 in xmlFreeParserInputBuffer__internal_alias workdir/UnpackedTarball/xml2/xmlIO.c:2575:2
    >     #3 0x2b112fdd2706 in xmlFreeInputStream__internal_alias workdir/UnpackedTarball/xml2/parserInternals.c:1341:9
    >     #4 0x2b112fddebf3 in xmlFreeParserCtxt__internal_alias workdir/UnpackedTarball/xml2/parserInternals.c:1774:9
    >     #5 0x2b116e6aaddc in std::_Sp_counted_deleter<_xmlParserCtxt*, void (*)(_xmlParserCtxt*), std::allocator<void>, (__gnu_cxx::_Lock_policy)2>::_M_dispose() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:464:9
    >     #6 0x2b116e65370a in std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:150:6
    >     #7 0x2b116e653494 in std::__shared_count<(__gnu_cxx::_Lock_policy)2>::~__shared_count() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:662:11
    >     #8 0x2b116e6a78b1 in std::__shared_ptr<_xmlParserCtxt, (__gnu_cxx::_Lock_policy)2>::~__shared_ptr() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr_base.h:928:31
    >     #9 0x2b116e69f01d in std::shared_ptr<_xmlParserCtxt>::~shared_ptr() /usr/lib/gcc/x86_64-redhat-linux/6.3.1/../../../../include/c++/6.3.1/bits/shared_ptr.h:93:11
    >     #10 0x2b116e696aca in DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx:336:5
    >     #11 0x2b116e699fe2 in non-virtual thunk to DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx
    >     #12 0x2b116df6449a in (anonymous namespace)::UpdateInformationProvider::getUpdateInformationEnumeration(com::sun::star::uno::Sequence<rtl::OUString> const&, rtl::OUString const&) extensions/source/update/feed/updatefeed.cxx:589:83
    >     #13 0x2b116df682ea in non-virtual thunk to (anonymous namespace)::UpdateInformationProvider::getUpdateInformationEnumeration(com::sun::star::uno::Sequence<rtl::OUString> const&, rtl::OUString const&) extensions/source/update/feed/updatefeed.cxx
    >     #14 0x2b115d9a73eb in testupdate::Test::testGetUpdateInformationEnumeration() extensions/qa/update/test_update.cxx:57:26
    >     #15 0x2b115d9baf1b in CppUnit::TestCaller<testupdate::Test>::runTest() workdir/UnpackedTarball/cppunit/include/cppunit/TestCaller.h:166:6
    >     #16 0x2b111904ad8b in CppUnit::TestCaseMethodFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/TestCase.cpp:32:5
    >     #17 0x2b11322dab0f in (anonymous namespace)::Protector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) test/source/vclbootstrapprotector.cxx:39:14
    >     #18 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
    >     #19 0x2b1128c6214f in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unotest/source/cpp/unobootstrapprotector/unobootstrapprotector.cxx:89:12
    >     #20 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
    >     #21 0x2b1124efc351 in (anonymous namespace)::Prot::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) unotest/source/cpp/unoexceptionprotector/unoexceptionprotector.cxx:63:16
    >     #22 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
    >     #23 0x2b1118f87350 in CppUnit::DefaultProtector::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) workdir/UnpackedTarball/cppunit/src/cppunit/DefaultProtector.cpp:15:12
    >     #24 0x2b11190093ce in CppUnit::ProtectorChain::ProtectFunctor::operator()() const workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:20:25
    >     #25 0x2b1119005e70 in CppUnit::ProtectorChain::protect(CppUnit::Functor const&, CppUnit::ProtectorContext const&) workdir/UnpackedTarball/cppunit/src/cppunit/ProtectorChain.cpp:77:18
    >     #26 0x2b11190c50f5 in CppUnit::TestResult::protect(CppUnit::Functor const&, CppUnit::Test*, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) workdir/UnpackedTarball/cppunit/src/cppunit/TestResult.cpp:181:28
    >     #27 0x2b1119048fa4 in CppUnit::TestCase::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestCase.cpp:91:13
    >     #28 0x2b111904d7a7 in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:64:30
    >     #29 0x2b111904c819 in CppUnit::TestComposite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:23:3
    >     #30 0x2b111904d7a7 in CppUnit::TestComposite::doRunChildTests(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:64:30
    >     #31 0x2b111904c819 in CppUnit::TestComposite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestComposite.cpp:23:3
    >     #32 0x2b11191035c9 in CppUnit::TestRunner::WrappingSuite::run(CppUnit::TestResult*) workdir/UnpackedTarball/cppunit/src/cppunit/TestRunner.cpp:47:27
    >     #33 0x2b11190c340d in CppUnit::TestResult::runTest(CppUnit::Test*) workdir/UnpackedTarball/cppunit/src/cppunit/TestResult.cpp:148:9
    >     #34 0x2b111910489b in CppUnit::TestRunner::run(CppUnit::TestResult&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&) workdir/UnpackedTarball/cppunit/src/cppunit/TestRunner.cpp:96:14
    >     #35 0x532fb4 in (anonymous namespace)::ProtectedFixtureFunctor::run() const sal/cppunittester/cppunittester.cxx:306:20
    >     #36 0x52e7c3 in sal_main() sal/cppunittester/cppunittester.cxx:456:20
    >     #37 0x52cb6f in main sal/cppunittester/cppunittester.cxx:363:1
    >     #38 0x2b111acb5400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    >     #39 0x438019 in _start (workdir/LinkTarget/Executable/cppunittester+0x438019)
    >
    > Address 0x2b111da94b68 is located in stack of thread T0 at offset 104 in frame
    >     #0 0x2b116e69553f in DOM::CDocumentBuilder::parse(com::sun::star::uno::Reference<com::sun::star::io::XInputStream> const&) unoxml/source/dom/documentbuilder.cxx:303
    >
    >   This frame has 4 object(s):
    >     [32, 40) 'g' (line 308)
    >     [64, 80) 'pContext' (line 310)
    >     [96, 120) 'c' (line 321) <== Memory access at offset 104 is inside this variable
    >     [160, 168) 'ref.tmp' (line 333)
    
    Change-Id: I3398d54776af4927417e392e55bbca740d4121af

diff --git a/unoxml/source/dom/documentbuilder.cxx b/unoxml/source/dom/documentbuilder.cxx
index b8f79f6fe9a8..71a6dc6f1785 100644
--- a/unoxml/source/dom/documentbuilder.cxx
+++ b/unoxml/source/dom/documentbuilder.cxx
@@ -299,6 +299,14 @@ namespace DOM
         throw saxex;
     }
 
+    namespace {
+
+    struct XmlFreeParserCtxt {
+        void operator ()(xmlParserCtxt * p) const { xmlFreeParserCtxt(p); }
+    };
+
+    }
+
     Reference< XDocument > SAL_CALL CDocumentBuilder::parse(const Reference< XInputStream >& is)
     {
         if (!is.is()) {
@@ -307,8 +315,17 @@ namespace DOM
 
         ::osl::MutexGuard const g(m_Mutex);
 
-        std::shared_ptr<xmlParserCtxt> const pContext(
-                xmlNewParserCtxt(), xmlFreeParserCtxt);
+        // IO context struct.  Must outlive pContext, as destroying that via
+        // xmlFreeParserCtxt may still access this context_t
+        context_t c;
+        c.pBuilder = this;
+        c.rInputStream = is;
+        // we did not open the stream, thus we do not close it.
+        c.close = false;
+        c.freeOnClose = false;
+
+        std::unique_ptr<xmlParserCtxt, XmlFreeParserCtxt> const pContext(
+                xmlNewParserCtxt());
 
         // register error functions to prevent errors being printed
         // on the console
@@ -317,13 +334,6 @@ namespace DOM
         pContext->sax->warning = warning_func;
         pContext->sax->resolveEntity = resolve_func;
 
-        // IO context struct
-        context_t c;
-        c.pBuilder = this;
-        c.rInputStream = is;
-        // we did not open the stream, thus we do not close it.
-        c.close = false;
-        c.freeOnClose = false;
         xmlDocPtr const pDoc = xmlCtxtReadIO(pContext.get(),
                 xmlIO_read_func, xmlIO_close_func, &c, nullptr, nullptr, 0);
 
@@ -339,8 +349,8 @@ namespace DOM
     {
         ::osl::MutexGuard const g(m_Mutex);
 
-        std::shared_ptr<xmlParserCtxt> const pContext(
-                xmlNewParserCtxt(), xmlFreeParserCtxt);
+        std::unique_ptr<xmlParserCtxt, XmlFreeParserCtxt> const pContext(
+                xmlNewParserCtxt());
         pContext->_private = this;
         pContext->sax->error = error_func;
         pContext->sax->warning = warning_func;


More information about the Libreoffice-commits mailing list