[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