help needed to find a bug in zorba (or gcc 4.9)

[Martin Gieseking]

Hi,

I've tried to fix the broken zorba package in rawhide for a couple of 
weeks now but, unfortunately, without much success. The upstream 
developers don't seem to be able to find the cause for the issue either.

The problem is that the package fails to build with gcc 4.9.0 (all 
archs) because the generated zorba binary segfaults for some queries due 
to accessing already freed memory. The issue only occurs with optimized 
builds (-O1, -O2, -O3) using gcc 4.9.0. With gcc 4.8.x the binary and 
thus the whole package build and work correctly. Therefore, it might 
also be possible that there's a bug in gcc's optimizer, but I'm not sure.

valgrind and gcc's address sanitizer report the code sections where the 
error occurs but when stepping through them with a debugger, I'm unable 
to understand what's actually going on there. It looks as if the 
affected code should work properly. So I got stuck now.

It would be great if someone could help to track down the issue in order 
to keep the package available in Fedora.

Here is the latest SRPM:
http://mgieseki.fedorapeople.org/review/zorba-3.0.0-4.fc21.src.rpm

The corresponding bug tickets can be found here:
https://bugzilla.redhat.com/show_bug.cgi?id=1095292
https://bugs.launchpad.net/bugs/1317976

Thanks,
Martin

[Petr Spacek]

On 10.6.2014 14:16, Martin Gieseking wrote:
>  Hi,
>
>  I've tried to fix the broken zorba package in rawhide for a couple of weeks
>  now but, unfortunately, without much success. The upstream developers don't
>  seem to be able to find the cause for the issue either.
>
>  The problem is that the package fails to build with gcc 4.9.0 (all archs)
>  because the generated zorba binary segfaults for some queries due to accessing
>  already freed memory. The issue only occurs with optimized builds (-O1, -O2,
>  -O3) using gcc 4.9.0. With gcc 4.8.x the binary and thus the whole package
>  build and work correctly. Therefore, it might also be possible that there's a
>  bug in gcc's optimizer, but I'm not sure.
>
>  valgrind and gcc's address sanitizer report the code sections where the error
>  occurs but when stepping through them with a debugger, I'm unable to
>  understand what's actually going on there. It looks as if the affected code
>  should work properly. So I got stuck now.
>
>  It would be great if someone could help to track down the issue in order to
>  keep the package available in Fedora.
>
>  Here is the latest SRPM:
>  http://mgieseki.fedorapeople.org/review/zorba-3.0.0-4.fc21.src.rpm
>
>  The corresponding bug tickets can be found here:
>  https://bugzilla.redhat.com/show_bug.cgi?id=1095292
>  https://bugs.launchpad.net/bugs/1317976
>
>  Thanks,
>  Martin 
Does it happen if you compile it *with* -fno-delete-null-pointer-checks switch?

That one caused a lot of grief with BIND 9.

-- 
Petr^2 Spacek

[Martin Gieseking]

Am 10.06.2014 14:24, schrieb Petr Spacek:
>  On 10.6.2014 14:16, Martin Gieseking wrote:
> > The problem is that the package fails to build with gcc 4.9.0 (all archs)
> > because the generated zorba binary segfaults for some queries due to
> > accessing
> > already freed memory. The issue only occurs with optimized builds
> > (-O1, -O2,
> > -O3) using gcc 4.9.0. With gcc 4.8.x the binary and thus the whole
> > package
> > build and work correctly. Therefore, it might also be possible that
> > there's a
> > bug in gcc's optimizer, but I'm not sure. 

>  Does it happen if you compile it *with*
> -fno-delete-null-pointer-checks
>  switch?
>
>  That one caused a lot of grief with BIND 9. 

Yes, unfortunately, it does. I just rebuilt zorba with the suggested 
option but still get a buggy binary that segfaults while building the 
package.

Martin

[Jerry James]

On Tue, Jun 10, 2014 at 6:16 AM, Martin Gieseking
<martin.gieseking(a)uos.de&gt; wrote:
>  Hi,
>
>  I've tried to fix the broken zorba package in rawhide for a couple of weeks
>  now but, unfortunately, without much success. The upstream developers don't
>  seem to be able to find the cause for the issue either.
>
>  The problem is that the package fails to build with gcc 4.9.0 (all archs)
>  because the generated zorba binary segfaults for some queries due to
>  accessing already freed memory. The issue only occurs with optimized builds
>  (-O1, -O2, -O3) using gcc 4.9.0. With gcc 4.8.x the binary and thus the
>  whole package build and work correctly. Therefore, it might also be possible
>  that there's a bug in gcc's optimizer, but I'm not sure.
>
>  valgrind and gcc's address sanitizer report the code sections where the
>  error occurs but when stepping through them with a debugger, I'm unable to
>  understand what's actually going on there. It looks as if the affected code
>  should work properly. So I got stuck now.
>
>  It would be great if someone could help to track down the issue in order to
>  keep the package available in Fedora. 
Here's the first problem pointed out by valgrind:
- class Store (src/store/naive/store.h) has a public member "zstring
theEmptyNs"
- that object is set to a string that is also added to "StringPool
*theNamespacePool" inside Store::init() (src/store/naive/store.cpp)
- when the ZorbaImpl destructor runs on the singleton ZorbaImpl
object, it starts this call chain:
  o shutdownInternal(false)
  o StoreManager::shutdownStore(&GENV_STORE)
  o SimpleStore::shutdown(false)
  o Store::shutdown(false)
- Since theNamespacePool is non-NULL, we do this:
  theEmptyNs.~zstring();
  theXmlSchemaNs.~zstring();
  delete theNamespacePool;
  theNamespacePool = NULL;

We deleted theEmptyNs ... but left it sitting in theNamespacePool.  So
when theNamespacePool's destructor runs, it examines that string,
leading to the crash.  The same thing happens with theXmlSchemaNs.
The fix is to remove those strings from the StringPool instead of
explicitly deallocating them, and then let the Store destructor
actually delete the two strings, like so:

--- src/store/naive/store.cpp.orig 2013-11-06 00:20:44.000000000 -0700
+++ src/store/naive/store.cpp 2014-06-10 12:00:00.000000000 -0600
@@ -333,8 +333,8 @@

     if (theNamespacePool != NULL)
     {
-      theEmptyNs.~zstring();
-      theXmlSchemaNs.~zstring();
+      theNamespacePool->erase(theEmptyNs);
+      theNamespacePool->erase(theXmlSchemaNs);

       delete theNamespacePool;
       theNamespacePool = NULL;

Unfortunately, it appears that that is not the only bug.  Valgrind
shows at least two more bugs, both also tied into SimpleStore and
Store somehow, but I'm out of time to look at them.

Off topic: the check for unicode/coll.h (ZORBA_HAVE_COLL_H) is failing
spuriously because CHECK_INCLUDE_FILES is used where
CHECK_INCLUDE_FILE_CXX should be used.  One fix is to do this in
%prep:

# Fix detection of unicode/coll.h
sed -i 's,\(CHECK_INCLUDE_FILE\)S\( ("unicode/coll.h"\),\1_CXX\2,'
CMakeLists.txt

Good luck and regards,
-- 
Jerry James
http://www.jamezone.org/

[Martin Gieseking]

Am 10.06.2014 20:44, schrieb Jerry James:
>  Here's the first problem pointed out by valgrind:
>  - class Store (src/store/naive/store.h) has a public member "zstring
> theEmptyNs"
>  - that object is set to a string that is also added to "StringPool
>  *theNamespacePool" inside Store::init() (src/store/naive/store.cpp)
>  - when the ZorbaImpl destructor runs on the singleton ZorbaImpl
>  object, it starts this call chain:
>    o shutdownInternal(false)
>    o StoreManager::shutdownStore(&GENV_STORE)
>    o SimpleStore::shutdown(false)
>    o Store::shutdown(false)
>  - Since theNamespacePool is non-NULL, we do this:
>    theEmptyNs.~zstring();
>    theXmlSchemaNs.~zstring();
>    delete theNamespacePool;
>    theNamespacePool = NULL;
>  
>  We deleted theEmptyNs ... but left it sitting in theNamespacePool.  So
>  when theNamespacePool's destructor runs, it examines that string,
>  leading to the crash.  The same thing happens with theXmlSchemaNs.
>  The fix is to remove those strings from the StringPool instead of
>  explicitly deallocating them, and then let the Store destructor
>  actually delete the two strings, like so: 

Jerry,

thank you for taking the time to look into the code and for tracking
down the first issue. However, it's the same one I already fixed with
patch 4 (zorba-namespace-pool.patch) present in the latest SRPM [1]
linked in my initial email. It has also been applied upstream already.


>  Unfortunately, it appears that that is not the only bug.  Valgrind
>  shows at least two more bugs, both also tied into SimpleStore and
>  Store somehow, but I'm out of time to look at them. 
Yes, the remaining bugs are hard to isolate. They always occur in
conjunction with the zstring/rstring objects. I just can't find the
location where the memory gets corrupted so that the access to their
data fields fail...


>  Off topic: the check for unicode/coll.h (ZORBA_HAVE_COLL_H) is
> failing
>  spuriously because CHECK_INCLUDE_FILES is used where
>  CHECK_INCLUDE_FILE_CXX should be used.  One fix is to do this in
>  %prep:
>  # Fix detection of unicode/coll.h
>  sed -i 's,\(CHECK_INCLUDE_FILE\)S\( ("unicode/coll.h"\),\1_CXX\2,'
>  CMakeLists.txt 
Also thank you for this hint. I'm going to add the fix the bug and
report it upstream.

Martin

[1] http://mgieseki.fedorapeople.org/review/zorba-3.0.0-4.fc21.src.rpm

[Petr Spacek]

On 10.6.2014 21:47, Martin Gieseking wrote:
>  Am 10.06.2014 20:44, schrieb Jerry James:
> > Here's the first problem pointed out by valgrind:
> > - class Store (src/store/naive/store.h) has a public member "zstring
> theEmptyNs"
> > - that object is set to a string that is also added to "StringPool
> > *theNamespacePool" inside Store::init() (src/store/naive/store.cpp)
> > - when the ZorbaImpl destructor runs on the singleton ZorbaImpl
> > object, it starts this call chain:
> >    o shutdownInternal(false)
> >    o StoreManager::shutdownStore(&GENV_STORE)
> >    o SimpleStore::shutdown(false)
> >    o Store::shutdown(false)
> > - Since theNamespacePool is non-NULL, we do this:
> >    theEmptyNs.~zstring();
> >    theXmlSchemaNs.~zstring();
> >    delete theNamespacePool;
> >    theNamespacePool = NULL;
> >
> > We deleted theEmptyNs ... but left it sitting in theNamespacePool.  So
> > when theNamespacePool's destructor runs, it examines that string,
> > leading to the crash.  The same thing happens with theXmlSchemaNs.
> > The fix is to remove those strings from the StringPool instead of
> > explicitly deallocating them, and then let the Store destructor
> > actually delete the two strings, like so:
>
>
>  Jerry,
>
>  thank you for taking the time to look into the code and for tracking
>  down the first issue. However, it's the same one I already fixed with
>  patch 4 (zorba-namespace-pool.patch) present in the latest SRPM [1]
>  linked in my initial email. It has also been applied upstream already.
>
>
> > Unfortunately, it appears that that is not the only bug.  Valgrind
> > shows at least two more bugs, both also tied into SimpleStore and
> > Store somehow, but I'm out of time to look at them.
>
>  Yes, the remaining bugs are hard to isolate. They always occur in
>  conjunction with the zstring/rstring objects. I just can't find the
>  location where the memory gets corrupted so that the access to their
>  data fields fail... 
Maybe you can try Valgrind & gdbserver:

http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.gdbs...

I'm sorry if you already tried that :-)

Petr^2 Spacek

> > Off topic: the check for unicode/coll.h (ZORBA_HAVE_COLL_H) is
> failing
> > spuriously because CHECK_INCLUDE_FILES is used where
> > CHECK_INCLUDE_FILE_CXX should be used.  One fix is to do this in
> > %prep:
> > # Fix detection of unicode/coll.h
> > sed -i 's,\(CHECK_INCLUDE_FILE\)S\( ("unicode/coll.h"\),\1_CXX\2,'
> > CMakeLists.txt
>
>  Also thank you for this hint. I'm going to add the fix the bug and
>  report it upstream.
>
>  Martin
>
>  [1] http://mgieseki.fedorapeople.org/review/zorba-3.0.0-4.fc21.src.rpm

[Martin Gieseking]

Am 11.06.2014 08:32, schrieb Petr Spacek:
> >> Unfortunately, it appears that that is not the only bug. 
> Valgrind
> >> shows at least two more bugs, both also tied into SimpleStore and
> >> Store somehow, but I'm out of time to look at them.
> >
> > Yes, the remaining bugs are hard to isolate. They always occur in
> > conjunction with the zstring/rstring objects. I just can't find the
> > location where the memory gets corrupted so that the access to their
> > data fields fail...
>
>  Maybe you can try Valgrind & gdbserver:
>
>  http://valgrind.org/docs/manual/manual-core-adv.html#manual-core-adv.gdbs...
>
>
>  I'm sorry if you already tried that :-) 
Thanks, Petr. Yes, I already tried that too, but without much success so 
far.

Martin