Segfault merging database

[Scimmia22]

When updating from 2.3.1 to current develop branch (matches 2.4 branch), I get a popup "There may be new ingredients and recipes available. Would you like to add these to your database?". If I choose yes, brewtarget segfaults with this trace:

#0  Database::copy<Yeast> (this=this@entry=0x2450510, object=0x1, displayed=displayed@entry=true, keyHash=keyHash@entry=0x2450580)
    at /tmp/brewtarget/src/brewtarget/src/database.h:790
#1  0x0000000000530acd in Database::newYeast (this=0x2450510, other=<optimized out>)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:1509
#2  0x0000000000525cde in Database::updateDatabase (this=this@entry=0x2450510, filename=...)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:4926
#3  0x00000000005456b5 in Database::load (this=this@entry=0x2450510)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:351
#4  0x0000000000545871 in Database::Database (this=0x2450510)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:106
#5  0x00000000005459d5 in Database::instance ()
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:654
#6  0x0000000000509d89 in Brewtarget::initialize (userDirectory=...)
    at /tmp/brewtarget/src/brewtarget/src/brewtarget.cpp:435
#7  0x000000000050ad47 in Brewtarget::run (userDirectory=...)
    at /tmp/brewtarget/src/brewtarget/src/brewtarget.cpp:522
#8  0x00000000004c107e in main (argc=<optimized out>, argv=<optimized out>)
    at /tmp/brewtarget/src/brewtarget/src/main.cpp:70

[mikfire]

Thanks for the report. I will dig into this later tonight.

[mikfire]

I am unable to replicate this.

Could you make your database.sqlite file available via dropbox or something similar?

[Scimmia22]

email OK for the link? I see a gmail in the git log.

[mikfire]

That will be fine.

Mik
On Jun 28, 2016 10:21 AM, "Doug Newgard" [email protected] wrote:

email OK? I see a gmail in the git log.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#291 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AKxgF7BZe0Wcf68Ekif1-IF3d7OztbGXks5qQS3wgaJpZM4I_PXc
.

[mikfire]

I was afraid of that. I still cannot replicate the problem. The interesting bit in the stack trace is

0x0000000000530acd in Database::newYeast (this=0x2450510, **other=<optimized out>**)

It implies a compiler got a little aggressive in its optimizations.

What OS and distro are you compiling on? gcc and qt versions would be helpful as well.

Mik

[Scimmia22]

This is on Arch Linux, fully up to date (Qt 5.7, gcc 6.1). I'll try it with -O0 when I get home in a few hours. Arch defaults to -O2.

[mikfire]

Our builds, depending on a few flags, go for -O3.

I will spin up an arch vm and see if I can see anything interesting.

Mik
On Jun 29, 2016 16:04, "Doug Newgard" [email protected] wrote:

This is on Arch Linux, fully up to date (Qt 5.7, gcc 6.1). I'll try it
with -O0 when I get home in a few hours. Arch defaults to -O2.

—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub
#291 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AKxgF2nMPTnjwSR6hq5AJqbWYOvf2k_eks5qQs_AgaJpZM4I_PXc
.

[Scimmia22]

Doesn't seem to be optimization related, -O0 give me this bt:

#0  0x00000000005f7573 in Database::copy<Yeast> (this=0xe73220, object=0x1, displayed=true, keyHash=0xe73290)
    at /tmp/brewtarget/src/brewtarget/src/database.h:790
#1  0x00000000005a62b0 in Database::newYeast (this=0xe73220, other=0x1)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:1509
#2  0x00000000005d4615 in Database::updateDatabase (this=0xe73220, filename=...)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:4926
#3  0x0000000000599e4e in Database::load (this=0xe73220)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:351
#4  0x00000000005977d1 in Database::Database (this=0xe73220)
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:106
#5  0x000000000059d0cd in Database::instance ()
    at /tmp/brewtarget/src/brewtarget/src/database.cpp:654
#6  0x00000000005822a9 in Brewtarget::initialize (userDirectory=...)
    at /tmp/brewtarget/src/brewtarget/src/brewtarget.cpp:435
#7  0x00000000005829fc in Brewtarget::run (userDirectory=...)
    at /tmp/brewtarget/src/brewtarget/src/brewtarget.cpp:522
#8  0x00000000005365aa in main (argc=1, argv=0x7fff248afcb8)
    at /tmp/brewtarget/src/brewtarget/src/main.cpp:70

[Scimmia22]

Just did a build with gcc 5.4.0, same segfault

[mikfire]

It may be a Qt 5.7 thing. I am still on 5.6. I am building an arch vm instance today and tomorrow to see what I can see.

What I find interesting is that both times "other" assumes some odd values -- optimized out in the first trace, a static 0x1 in the second. That should be a pointer, not NULL or 1. This is causing the object->metaObject()->className() call in copy() to fail.

@rocketman768, can you explain what this line does:

    oldid = (this->*(tp.newElement))()->_key;

It is way beyond my primitive understanding of C to parse that pointer magic.

Mik

[rocketman768]

oldid = (this->*(tp.newElement))()->_key;

tp.newElement is a function pointer to a function which is supposed to create a new element for the given table. The this->* part is because it because of the fact that the function is not just a plain old function pointer, but a function pointer to a Database method, and hence needs a this parameter (the syntax just happens to be awful). The last () is just the call of the function. It returns some sort of BeerXMLElement* so you can dereference its _key.

[rocketman768]

Is it because we no longer have newYeast(void) but rather newYeast(Yeast* other=0) forced into a function pointer of the former type?

typedef struct
{
   QString tableName; // Name of the table.
   QStringList propName; // List of BeerXML column names.
   BeerXMLElement* (Database::*newElement)(void); // Function to make a new ingredient in this table.
} TableParams;

[mikfire]

Thanks for the explanation. Your question seems a very reasonable one, and leads immediately to a test. I would still be at a loss as to why it works fine on ubuntu 16.04 and gentoo.

I've got archOS somewhat installed. Now it will be picking my way through the build to figure out what pieces I need or don't need. I should be able to test this in a bit.

[mikfire]

I still cannot reproduce on Arch OS. Even using your database.

The only thing I can think of now is for me to generate a few patches send them to @Scimmia22 for testing.

[Scimmia22]

I'm willing to test, but if nobody can reproduce this, could it be something with my system? If so, it's likely not worth it.

[rocketman768]

All segfaults are serious and wrong.
On Mon, Jul 4, 2016 at 9:40 AM Doug Newgard [email protected]
wrote:

I'm willing to test, but if nobody can reproduce this, could it be
something with my system? If so, it's likely not worth it.

—
You are receiving this because you were mentioned.

Reply to this email directly, view it on GitHub
#291 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AAt7rnN3fN2TqlR8pysshusQoe8ngIsnks5qSTeFgaJpZM4I_PXc
.

[Scimmia22]

In that case, just let me know what you need to debug or if you have patches to try.

[mikfire]

Ok. I was able to track this down, finally. Thanks everybody for your patience with me. It isn't specific to your system, @Scimmia22.

I realized I had to install the new database in /usr/local/share/brewtarget before the merge happened correctly. Once I did that, brewtarget crashed every time.

Yes, as @rocketman768 suggested I had to change the function signature. I then ran into another bug that I think has been resident for a while, so had to fix that.

I will be commit a branch tonight that seems to address the issues.

[mikfire]

Try that, please.

[Scimmia22]

That seems to do the job. No segfault.

[mikfire]

Ok. The solution isn't quite as clean as I would like, but mr #293 seems to do the work.