dive merge seems broken in latest master

[Dirk Hohndel]

Berthold,

This looks like it is yours...

Bisect brings us to this:

commit 8c2383b4952fa22d41745d29484462ed6a67112b
Author: Berthold Stoeger <***@mail.tuwien.ac.at>
Date: Sun Aug 12 22:47:07 2018 -0400

I have too much nitrogen (and too little sleep) to figure out HOW and WHY this breaks things,
but on a trip with multiple dive computers, if I try to download (or plain import) the dives from
my second DC, Subsurface crashes. In process_imported_dive(); there appears to be garbage
int the dive_table.dives[] and that gets dereferenced to calculate the end time of a dive.

/D

[Jan Mulder]

Post by Dirk Hohndel
Berthold,
This looks like it is yours...
commit 8c2383b4952fa22d41745d29484462ed6a67112b
Date:   Sun Aug 12 22:47:07 2018 -0400
I have too much nitrogen (and too little sleep) to figure out HOW and
WHY this breaks things,
but on a trip with multiple dive computers, if I try to download (or
plain import) the dives from
my second DC, Subsurface crashes. In process_imported_dive(); there appears to be garbage
int the dive_table.dives[] and that gets dereferenced to calculate the end time of a dive.

Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

Are you able to produce a simple test XML and test-import XML that
reproduces this?

--jan

[Berthold Stoeger]

Post by Jan Mulder
Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

I can trivially reproduce this by importing a divelog onto itself. Therefore,
this should be easy to fix. I'm heading of to lecture now and will look at
this in the later afternoon.

Berthold

[Jan Mulder]

Post by Berthold Stoeger

Post by Jan Mulder
Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

I can trivially reproduce this by importing a divelog onto itself. Therefore,
this should be easy to fix. I'm heading of to lecture now and will look at
this in the later afternoon.

Yes, this produces a crash indeed. Not 100% sure this the same one.

--jan

[Jan Mulder]

Post by Jan Mulder

Post by Berthold Stoeger

Post by Jan Mulder
Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

I can trivially reproduce this by importing a divelog onto itself. Therefore,
this should be easy to fix. I'm heading of to lecture now and will look at
this in the later afternoon.

Yes, this produces a crash indeed. Not 100% sure this the same one.

Tried to make sense of this. The first attempt crashed immediately, but
subsequent sessions did not. In the debugger, I saw that the trips dives
list (double linked list it seems) is corrupted. Then I had a couple of
runs the ended in an endless loop in the import on itself phase, and
finally, all works ...

So, to me, it seems non-deterministic caused by memory corruption
somewhere. Curious what the root cause here, but it does not feel "easy
to fix" to me.

--jan

[Berthold Stoeger]

Post by Jan Mulder

Post by Jan Mulder

Post by Berthold Stoeger

Post by Jan Mulder
Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

I can trivially reproduce this by importing a divelog onto itself. Therefore,
this should be easy to fix. I'm heading of to lecture now and will look at
this in the later afternoon.

Yes, this produces a crash indeed. Not 100% sure this the same one.

Tried to make sense of this. The first attempt crashed immediately, but
subsequent sessions did not. In the debugger, I saw that the trips dives
list (double linked list it seems) is corrupted. Then I had a couple of
runs the ended in an endless loop in the import on itself phase, and
finally, all works ...
So, to me, it seems non-deterministic caused by memory corruption
somewhere. Curious what the root cause here, but it does not feel "easy
to fix" to me.

I'm quite sure that it is easy to fix, but I will only find time later this
afternoon after work. Quite obviously, for example merge_weight_system_info()
is broken. It should read as
res->weight = a->weight;
res->description = copy_string(a->description);
or similar.

Berthold

[Dirk Hohndel]

Post by Berthold Stoeger

Post by Jan Mulder

Post by Jan Mulder

Post by Berthold Stoeger

Post by Jan Mulder
Tried to reproduce things. As I do not have 2 DCs, I fake a second one
just by editing the ssrf XML. I cannot reproduce it (in this way).

I can trivially reproduce this by importing a divelog onto itself. Therefore,
this should be easy to fix. I'm heading of to lecture now and will look at
this in the later afternoon.

Yes, this produces a crash indeed. Not 100% sure this the same one.

Tried to make sense of this. The first attempt crashed immediately, but
subsequent sessions did not. In the debugger, I saw that the trips dives
list (double linked list it seems) is corrupted. Then I had a couple of
runs the ended in an endless loop in the import on itself phase, and
finally, all works ...
So, to me, it seems non-deterministic caused by memory corruption
somewhere. Curious what the root cause here, but it does not feel "easy
to fix" to me.

I'm quite sure that it is easy to fix, but I will only find time later this
afternoon after work. Quite obviously, for example merge_weight_system_info()
is broken. It should read as
res->weight = a->weight;
res->description = copy_string(a->description);
or similar.

My brain wasn't working at all yesterday or I would have given it a better try.
But getting up before 3am, sitting in airplanes for 10 hours, then having a really
bad night's sleep and diving for nearly 5 hours all conspired to have me a long
way away from my personal best :-)

I did bisect at least the crash so that should give a good starting point where
to look.

I have a few minutes this morning, but if Berthold will look at it this afternoon,
then I'll instead look at PRs and whatever else has piled up in the last two
days (so we don't duplicate work).

Thanks!

/D