Zorba

segfault in parse-xml:parse()

Bug #1024033 reported by mb21 on 2012-07-12

This bug affects 1 person

Affects		Status	Importance	Assigned to	Milestone
	Zorba	Fix Released	Critical	Nicolae Brinza	Zorba 2.7 "Gaia"

Bug Description

parse-xml:parse() results in a segfault when called with:

<opt:options>
<opt:parse-external-parsed-entity opt:skip-root-nodes="0" />
</opt:options>

or with

<opt:options>
<opt:parse-external-parsed-entity opt:skip-root-nodes="1" />
</opt:options>

for that matter.
With only <opt:options /> it works.

Reproduce:

echo "<page />" > acc.xml
zorba -f -q xmlparse.xq

This was tested with the zorba build from revision 10923.

$ gdb --args zorba -f -q xmlparse.xq
[Thread debugging using libthread_db enabled]
<?xml version="1.0" encoding="UTF-8"?>
fetch [0]: xs:string(<page />
)

Program received signal SIGSEGV, Segmentation fault.
0xb54e29de in std::basic_istream<char, std::char_traits<char> >::sentry::sentry(std::basic_istream<char, std::char_traits<char> >&, bool) () from /usr/lib/i386-linux-gnu/libstdc++.so.6

also:

$ valgrind install_dbg/bin/zorba -f -q zorba-files/xmlparse.xq
==21695== Memcheck, a memory error detector
==21695== Copyright (C) 2002-2010, and GNU GPL'd, by Julian Seward et al.
==21695== Using Valgrind-3.6.1 and LibVEX; rerun with -h for copyright info
==21695== Command: /home/tillw/code/zorba/install_dbg/bin/zorba -f -q zorba-files/xmlparse.xq
==21695==
<?xml version="1.0" encoding="UTF-8"?>
fetch [0]: xs:string(<page />
)
==21695== Invalid write of size 4
==21695== at 0x6B0AFD3: std::istream::read(char*, int) (in /usr/lib/i386-linux-gnu/libstdc++.so.6.0.14)
==21695== by 0x4EA5B70: zorba::simplestore::FastXmlLoader::readPacket(std::istream&, char*, long) (loader_fast.cpp:242)
==21695== by 0x4EAAAC1: zorba::simplestore::FragmentXmlLoader::fillBuffer(zorba::FragmentIStream*) (loader_dtd.cpp:161)
==21695== by 0x4EAB3E2: zorba::simplestore::FragmentXmlLoader::loadXml(zorba::rstring<zorba::rstring_classes::rep<zorba::atomic_int,
std::char_traits<char>, std::allocator<char> > > const&, zorba::rstring<zorba::rstring_classes::rep<zorba::atomic_int, std::char_traits<
char>, std::allocator<char> > > const&, std::istream&) (loader_dtd.cpp:260)
==21695== by 0x4ECD245: zorba::simplestore::Store::loadDocument(zorba::rstring<zorba::rstring_classes::rep<zorba::atomic_int, std::ch
ar_traits<char>, std::allocator<char> > > const&, zorba::rstring<zorba::rstring_classes::rep<zorba::atomic_int, std::char_traits<char>,
std::allocator<char> > > const&, std::istream&, zorba::store::LoadProperties const&) (store.cpp:1030)
==21695== by 0x4A96C87: zorba::FnZorbaParseXmlFragmentIterator::nextImpl(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanSt
ate&) const (parse_fragment_impl.cpp:230)
==21695== by 0x489381C: zorba::Batcher<zorba::FnZorbaParseXmlFragmentIterator>::produceNext(zorba::store::ItemHandle<zorba::store::It
em>&, zorba::PlanState&) const (plan_iterator.h:535)
==21695== by 0x4B3D7BA: zorba::PlanIterator::consumeNext(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanIterator const*, z
orba::PlanState&) (plan_iterator.cpp:109)
==21695== by 0x49FA142: zorba::TraceIterator::nextImpl(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanState&) const (error
s_and_diagnostics_impl.cpp:102)
==21695== by 0x4969368: zorba::Batcher<zorba::TraceIterator>::produceNext(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanS
tate&) const (plan_iterator.h:535)
==21695== by 0x4B3D7BA: zorba::PlanIterator::consumeNext(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanIterator const*, z
orba::PlanState&) (plan_iterator.cpp:109)
==21695== by 0x4B823DA: zorba::flwor::FLWORIterator::bindVariable(unsigned long, zorba::flwor::FlworState*, zorba::PlanState&) const
(flwor_iterator.cpp:1216)
==21695== Address 0x7406f1c is 4 bytes inside a block of size 280 free'd
==21695== at 0x4025907: operator delete(void*) (vg_replace_malloc.c:387)
==21695== by 0x6B0215F: std::basic_ifstream<char, std::char_traits<char> >::~basic_ifstream() (in /usr/lib/i386-linux-gnu/libstdc++.s
o.6.0.14)
==21695== by 0x4778C2C: zorba::internal::fileStreamReleaser(std::istream*) (default_url_resolvers.cpp:86)
==21695== by 0x4E697E6: zorba::simplestore::StreamableStringItem::~StreamableStringItem() (atomic_items.h:919)
==21695== by 0x4E69866: zorba::simplestore::StreamableStringItem::~StreamableStringItem() (atomic_items.h:921)
==21695== by 0x4C775ED: zorba::store::Item::free() (in /home/tillw/code/zorba/install_dbg/lib/libzorba_simplestore.so.2.5.0)
==21695== by 0x4E0AE39: zorba::store::Item::removeReference() (item.cpp:172)
==21695== by 0x445768B: zorba::store::ItemHandle<zorba::store::Item>& zorba::store::ItemHandle<zorba::store::Item>::assign<zorba::sto
re::Item>(zorba::store::ItemHandle<zorba::store::Item> const&) (item_handle.h:200)
==21695== by 0x4456971: zorba::store::ItemHandle<zorba::store::Item>::operator=(zorba::store::ItemHandle<zorba::store::Item> const&)
(item_handle.h:143)
==21695== by 0x4A96C9C: zorba::FnZorbaParseXmlFragmentIterator::nextImpl(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanSt
ate&) const (parse_fragment_impl.cpp:230)
==21695== by 0x489381C: zorba::Batcher<zorba::FnZorbaParseXmlFragmentIterator>::produceNext(zorba::store::ItemHandle<zorba::store::It
em>&, zorba::PlanState&) const (plan_iterator.h:535)
==21695== by 0x4B3D7BA: zorba::PlanIterator::consumeNext(zorba::store::ItemHandle<zorba::store::Item>&, zorba::PlanIterator const*, z
orba::PlanState&) (plan_iterator.cpp:109)
==21695==

See original description

Related branches

lp:~nbrinza/zorba/parse-fragment

Merged into lp:zorba at revision 11208

Chris Hillery: Approve on 2013-01-30

Nicolae Brinza: Approve on 2013-01-20

Revision history for this message

mb21 (mauro-bieg) wrote on 2012-07-12:

reproduce Edit (705 bytes, text/plain)

mb21 (mauro-bieg) on 2012-07-12

description:

updated

Revision history for this message

Chris Hillery (ceejatec) wrote on 2012-07-12:

I'll take a look.

Changed in zorba:
status:	New → Confirmed
importance:	Undecided → Critical
assignee:	nobody → Chris Hillery (ceejatec)
milestone:	none → 2.7

Revision history for this message

Chris Hillery (ceejatec) wrote on 2012-07-13:

Nicolae - this is happening because of the following:

    if (result->isStreamable())
    {
      state->theFragmentStream.theStream = &result->getStream();
    }

(lines 195-198 of parse_fragment_impl.cpp). This code assigns the istream to the state, but memory ownership of the stream is still associated with the item (result). Later, this stream is passed to Store.loadDocument() at line 230. So when that item goes out of scope, the stream is freed, and the store is left pointing to a freed istream.

What needs to happen is that state->theFragmentStream needs to have a StreamReleaser field as well. Then, when you assign the value of result->getStream() to theFragmentStream.theStream, you need to also assign the value of result->getStreamReleaser() to theFragmentStream.theStreamReleaser. You also need to call result->setStreamReleaser(nullptr) to complete the transference of memory ownership to theFragmentStream. Finally, in ~FragmentIStream() (or, apparently, actually in FragmentIStream::reset()), if theStreamReleaser is non-null then call it, passing theStream as an argument.

It's not super-clear, but this is how streaming in Zorba works, for better or for worse...

Changed in zorba:
assignee:	Chris Hillery (ceejatec) → Nicolae Brinza (nbrinza)

Revision history for this message

Nicolae Brinza (nbrinza) wrote on 2012-07-31:

Thanks for the hint, Chris. It was quite clear. It makes sense from the point of view of streamable items lifetime management.

It indeed fixed the bug.

Revision history for this message

Chris Hillery (ceejatec) wrote on 2012-07-31:

Nicolae - the branch you associated with this bug is merged, but the same query still crashes in apparently the same fashion with the latest trunk build. Please take another look with high priority.

Revision history for this message

Nicolae Brinza (nbrinza) wrote on 2012-07-31:

Hi Chris,

I'm reusing this old branch -- the parse-fragment branch -- which I've been using for all parse-fragment -related issues. It is indeed merged, but I have pushed a new commit, which has not been merged yet. It cannot be merged as I have not even proposed it. I was waiting to fix the other bug and merge both changes into the trunk.

If you want to test the fix you'll have to check out the branch and build it. The trunk does not yet have the fix.

Zorba Build Bot (zorba-buildbot) on 2012-08-30