News:

Simutrans.com Portal
Our Simutrans site. You can find everything about Simutrans from here.

Crash when deleting bridge next to tunnel entrance

Started by freddyhayward, February 24, 2021, 07:39:14 AM

Previous topic - Next topic

0 Members and 1 Guest are viewing this topic.

freddyhayward

1. Setup terrain so that bridge can be built next to portal:

2. Build bridge:

3. Build tunnel:

4. Enter sliced underground view:

5. Delete tunnel segment next to portal:

6. Delete bridge, game will crash:

On an extended debug build, the crash occurs on the last line of this code snippet from brueckenbauer.cc and the code looks the same as standard.
grund_t *prev;
// if we have a ramp, then only check the higher end!
if(  gr->get_neighbour( prev, wegtyp, ribi_t::nesw[i])  &&  prev->get_hoehe() > gr->get_hoehe()  ) {
if(  weg_t *w = prev->get_weg( wegtyp )  ) {
// now remove ribi (or full way)
w->set_ribi( (~ribi_t::backward( ribi_t::nesw[i] )) & w->get_ribi_unmasked() );
if(  w->get_ribi_unmasked() == 0  ) {
// nowthing left => then remove completel
prev->remove_everything_from_way( player, wegtyp, bridge_ribi ); // removes stop and signals correctly
prev->weg_entfernen( wegtyp, true );
if(  prev->get_typ() == grund_t::monorailboden  ) {

prissi

Standard does not crash. Also bridges and tunnels are different in extended, since they contain their own ways.

freddyhayward

Quote from: prissi on February 24, 2021, 01:49:23 PMStandard does not crash. Also bridges and tunnels are different in extended, since they contain their own ways.
Standard does crash, it crashed in my testing, and 2 other users on discord were able to reproduce.

[C] Ranran

Unfortunately I can't reproduce this with extended. (´・ω・`)


Can you reproduce the crash at the same coordinates (237,12) in demo.sve?
If so, there may be a problem that cannot be reproduced on windows...

Roboron


freddyhayward

Quote from: Ranran on February 24, 2021, 10:27:21 PM
Unfortunately I can't reproduce this with extended. (´・ω・`)


Can you reproduce the crash at the same coordinates (237,12) in demo.sve?
If so, there may be a problem that cannot be reproduced on windows...
Yes, I could. Can you try uploading the save?

prissi

It seems very hard to reproduce this. I could only crash it once in pak128. Reloading the same game I could no longer crash it. I failed to procude a crash in pak64 at all, and in pak128 I needed three tries and then could not reproduce it.

Simutrans crashed at the same position, but this does not make sense either. Because if prev is NULL (like in my single crash), then the calls to prev->remove_everything_from_way and prev->weg_entfernen should have failed as well.

freddyhayward

Quote from: prissi on February 25, 2021, 06:43:51 AM
It seems very hard to reproduce this. I could only crash it once in pak128. Reloading the same game I could no longer crash it. I failed to procude a crash in pak64 at all, and in pak128 I needed three tries and then could not reproduce it.

Simutrans crashed at the same position, but this does not make sense either. Because if prev is NULL (like in my single crash), then the calls to prev->remove_everything_from_way and prev->weg_entfernen should have failed as well.
Can you upload a pak64 save that does NOT crash?

freddyhayward

I have attached a minimal pak64 reproduction case here. All it takes to crash is deleting the bridge. Please let me know if it doesn't crash everytime.

freddyhayward

and ranran, can you please test this pak128.britain-ex save?

[C] Ranran

#10
Quote from: freddyhayward on February 25, 2021, 08:31:50 AMand ranran, can you please test this pak128.britain-ex save?
Yeah, I can certainly see that it causes a crash.


But what is the difference from the bridge in the foreground in the image above? Destroying it will not cause a crash.
Similarly, the two bridges built in demo.save do not cause a crash
Is it because they are at 0 altitude?


EDIT:
0 Altitude doesn't seem to matter. If I build a bridge and tunnel there as well after removing the tunnel built by freddy, the crash cannot be reproduced.
The bridge I built is blessed.

ceeac

Seems like a use-after-free to me. Running Standard with -fsanitize=undefined gives this:

bauer/brueckenbauer.cc:1155:14: runtime error: member call on address 0x00000382d528 which does not point to an object of type 'grund_t'
0x00000382d528: note: object has a possibly invalid vptr: abs(offset to top) too big
00 00 00 02  48 d6 82 03 00 00 00 00  00 00 00 00 00 00 00 00  00 00 00 00 5d 1d 00 00  02 00 03 00
              ^~~~~~~~~~~~~~~~~~~~~~~
              possibly invalid vptr
    #0 0x4596d1 in bridge_builder_t::remove(player_t*, koord3d, waytype_t) /home/ceeac/Projects/code/simutrans/bauer/brueckenbauer.cc:1155:14
    #1 0x13e8fc3 in tool_remover_t::tool_remover_intern(player_t*, koord3d, signed char, char const*&) /home/ceeac/Projects/code/simutrans/simtool.cc:578:9
    #2 0x13eebb1 in tool_remover_t::work(player_t*, koord3d) /home/ceeac/Projects/code/simutrans/simtool.cc:779:6
    #3 0x1521aba in karte_t::call_work(tool_t*, player_t*, koord3d, bool&) /home/ceeac/Projects/code/simutrans/simworld.cc:6947:16
    #4 0x135eff4 in interaction_t::interactive_event(event_t const&) /home/ceeac/Projects/code/simutrans/siminteraction.cc:239:18
    #5 0x1361e13 in interaction_t::process_event(event_t&) /home/ceeac/Projects/code/simutrans/siminteraction.cc:417:2
    #6 0x1361f30 in interaction_t::check_events() /home/ceeac/Projects/code/simutrans/siminteraction.cc:439:7
    #7 0x1553a3e in karte_t::interactive(unsigned int) /home/ceeac/Projects/code/simutrans/simworld.cc:7258:17
    #8 0x1383144 in simu_main(int, char**) /home/ceeac/Projects/code/simutrans/simmain.cc:1537:9
    #9 0x16f57a0 in sysmain(int, char**) /home/ceeac/Projects/code/simutrans/sys/simsys.cc:1125:9
    #10 0x17ddb39 in main /home/ceeac/Projects/code/simutrans/sys/simsys_s2.cc:810:9
    #11 0x7fbc745ca0b2 in __libc_start_main /build/glibc-eX1tMB/glibc-2.31/csu/../csu/libc-start.c:308:16
    #12 0x41464d in _start (/media/ceeac/Projects/code/simutrans/build/client/sim+0x41464d)


I believe the following code from grund_t::remove_everything_from_way (line 2086) causes the bug (note the last comment):

// make tunnel portals to normal ground
if (get_typ()==tunnelboden  &&  (flags&has_way1)==0) {
// remove remaining objs
obj_loesche_alle( player );
// set to normal ground
welt->access(here)->kartenboden_setzen( new boden_t( pos, slope ) );
// now this is already deleted !
}


However, I can also reproduce the bug when deleting tunnels via tunnel portals without a bridge nearby, so I'm not sure if this is the actual root cause.
The bug seems to be quite old - I can also reproduce it with r8630 from 2018 which is the oldest revision I can get to compile.

freddyhayward

Quote from: Ranran on February 25, 2021, 09:08:54 AM
Yeah, I can certainly see that it causes a crash.


But what is the difference from the bridge in the foreground in the image above? Destroying it will not cause a crash.
Similarly, the two bridges built in demo.save do not cause a crash
Is it because they are at 0 altitude?


EDIT:
0 Altitude doesn't seem to matter. If I build a bridge and tunnel there as well after removing the tunnel built by freddy, the crash cannot be reproduced.
I cannot access that save - but it's possible that you did not delete the tunnel section next to the portal.

freddyhayward

Quote from: ceeac on February 25, 2021, 09:10:21 AMHowever, I can also reproduce the bug when deleting tunnels via tunnel portals without a bridge nearby, so I'm not sure if this is the actual root cause.
I'm not sure what this means, can you upload a minimal reproduction save?

[C] Ranran

Quote from: freddyhayward on February 25, 2021, 09:18:36 AMI cannot access that save - but it's possible that you did not delete the tunnel section next to the portal.
Ahh, you are right. Crash when deleting the tunnel behind the portal and then deleting the bridge.

Dwachs

One solution could be to change grund_t::remove_everything_from_way to be static and get the grund_t* parameter as reference. If the grund gets deleted the pointer is set to NULL. In this way all places in the code that use this routine and are prone to this error needs to be altered for good.
Parsley, sage, rosemary, and maggikraut.

prissi

Inside a tunnel, the tunnelbauer deletes also stuff himself. Seems like there is also a good chance of crashing. r9650 should fix this.

Oh sorry, I submitted just before I saw Dwachs comment. I actually removed the deletion of ground from it and left it to the bruecken and tunnelbauer routines.