The International Simutrans Forum

Simutrans Extended => Simutrans-Extended bug reports => Simutrans-Extended development => Simutrans-Extended closed bug reports => Topic started by: Rollmaterial on May 05, 2018, 10:25:11 PM

Title: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Rollmaterial on May 05, 2018, 10:25:11 PM
When I attempt to modify route P Elsfont - Cabbourne on the Bridgewater-Brunel server, the server crashes and the client disconnects and keeps running.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 05, 2018, 10:47:28 PM
It is very odd that this should crash only the server. I have not been able to reproduce this in single player mode (on the assumption that any modification whatsoever is sufficient; if this is not so, I will need to know the specifics of the modification necessary to reproduce this reliably).
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Rollmaterial on May 05, 2018, 10:50:47 PM
I removed all stops before Corn Barngrave and rerouted the line to Embury.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Ves on May 05, 2018, 11:07:34 PM
Ive noted something else when I try to modify a line with range restricted vehicles on the servergame:
Most of them get the "out of range" sign and the warning window pops up, but looking in the warning window the numbers are way of:

(https://farm1.staticflickr.com/823/41197833174_0d995efc03_o_d.png)

It could be in family with the problem raized by Rollmaterial, since it looks like there is some kind of overflow or similar happening (note the name of the halt)

No matter what stop I click in the schedule (even stops that are well within its actual range), this happens.
If I send it to depot, which also are within range, provokes the same result.

savegame:
https://github.com/VictorErik/saved-games/blob/master/master/bug%20-%20out%20of%20range%20overflow.sve (https://github.com/VictorErik/saved-games/blob/master/master/bug%20-%20out%20of%20range%20overflow.sve)

On game startup, problem convoy is located at (37,49)
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Rollmaterial on May 05, 2018, 11:13:09 PM
That's basically what happens to the client. In my case the server crashed.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 06, 2018, 12:00:45 AM
Thank you for the reports. I am still not entirely sure what is causing the crash, but I have fixed a calculation issue and possible integer overflow issue, which should hopefully help. I should be grateful if anyone could let me know whether this issue is abated or resolved in the next nightly build.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Ves on May 06, 2018, 01:27:49 AM
I have not tested the next nighty build (as it has not been created yet!), but I captured another variant of this bug, but this time on the road:

The vehicles completely spam the message board with out of range messages!
I have attached a savegame, you should quickly be overwhelmed by the messages.....

https://github.com/VictorErik/saved-games/blob/master/master/bug%20-%20vehicles%20out%20of%20range%20message%20spam.sve (https://github.com/VictorErik/saved-games/blob/master/master/bug%20-%20vehicles%20out%20of%20range%20message%20spam.sve)
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Rollmaterial on May 06, 2018, 03:09:52 PM
Crashed again! On the same line I first removed Elsfont Parish Hall Stop, then closed the line schedule window, then shortly afterwards removed Wake Yendgate Corner Railway Station. The server crashed and the client kept running with convoys spamming out of range errors.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 06, 2018, 04:10:46 PM
I think that I have now fixed this -  I should be grateful if you could re-test with to-morrow's nightly build. I have also made a modification which might fix the crashes, but, since I am not able to recreate them locally, I have not had a chance to test this.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: ACarlotti on May 06, 2018, 06:11:46 PM
I think I can reproduce these crashes, and have part diagnosed the problem (before getting stuck on *why* something is happening).

First, the reproduction: I can consistently crash the game by creating two passenger docks (a long way apart), a shipyard next to the first dock, and then scheduling a skiff wherry to run from the first dock to the second dock (and back). The crash occurs when the wherry tries to route from the first dock to the second, discovers it is too far, and attempts to generate a message.

Now the code details:
The crash occurs while running line 988 in player/simplay.cc

buf.printf( translator::translate("Vehicle %s cannot travel %ikm to %s because that would exceed its range of %ikm by %.2fkm"), cnv->get_name(), distance, name, cnv->get_min_range(), excess);

The backtrace, and a couple of other bits of debug output, are:

(gdb) backtrace
#0  0x00007ffff680b3a6 in __strlen_sse2 () from /usr/lib/libc.so.6
#1  0x00007ffff67c44bd in vfprintf () from /usr/lib/libc.so.6
#2  0x00007ffff67ebbd2 in vsnprintf () from /usr/lib/libc.so.6
#3  0x000055555597e743 in my_vsnprintf(char *, size_t, const char *, typedef __va_list_tag __va_list_tag *) (buf=0x5555595ddb40 "Vehicle (1) Skiff wherry (passengers) cannot travel 1627215616km to sengers).", n=256,
    fmt=0x555555a096d0 "Vehicle %s cannot travel %ikm to %s because that would exceed its range of %ikm by %.2fkm", ap=0x7fffffffb910) at utils/cbuffer_t.cc:360
#4  0x000055555597e8ad in cbuffer_t::vprintf (this=0x7fffffffbaa0, fmt=0x555555a096d0 "Vehicle %s cannot travel %ikm to %s because that would exceed its range of %ikm by %.2fkm", ap=0x7fffffffb950) at utils/cbuffer_t.cc:387
#5  0x000055555597e812 in cbuffer_t::printf (this=0x7fffffffbaa0, fmt=0x555555a096d0 "Vehicle %s cannot travel %ikm to %s because that would exceed its range of %ikm by %.2fkm") at utils/cbuffer_t.cc:369
#6  0x00005555557d81e5 in player_t::report_vehicle_problem (this=0x5555684065f0, cnv=..., ziel=...) at player/simplay.cc:988
#7  0x0000555555896cfb in convoi_t::prepare_for_routing (this=0x55556dae4e30) at simconvoi.cc:1380
#8  0x0000555555899b2d in convoi_t::step (this=0x55556dae4e30) at simconvoi.cc:2094
#9  0x000055555595d2e8 in karte_t::step (this=0x555558fd39f0) at simworld.cc:5467
#10 0x0000555555970bb7 in karte_t::interactive (this=0x555558fd39f0, quit_month=2147483647) at simworld.cc:10458
#11 0x00005555559045aa in simu_main (argc=2, argv=0x7fffffffe988) at simmain.cc:1362
#12 0x00005555559178ca in sysmain (argc=2, argv=0x7fffffffe988) at simsys.cc:825
#13 0x00005555559e2354 in main (argc=2, argv=0x7fffffffe988) at simsys_s2.cc:792
(gdb) frame 6
#6  0x00005555557d81e5 in player_t::report_vehicle_problem (this=0x5555684065f0, cnv=..., ziel=...) at player/simplay.cc:988
988                                             buf.printf( translator::translate("Vehicle %s cannot travel %ikm to %s because that would exceed its range of %ikm by %.2fkm"), cnv->get_name(), distance, name, cnv->get_min_range(), excess);
(gdb) print(buf)
$44 = {capacity = 256, size = 0, buf = 0x5555595ddb40 "Vehicle (1) Skiff wherry (passengers) cannot travel 1627215616km to sengers)."}
(gdb) print(name)
$45 = 0x555560fd5700 "land stop 2 dock"


What seems to be happening is that:
1. The first %s is formatted correctly.
2. The following %i is formatted using the fourth argument instead of the third. In particular, 1627215616 is the value of the least significant 32 bits of the pointer 0x555560fd5700 as a uint32.
3. A segfault occurs while trying to format the following %s. Give the above, it seems likely that it is trying to read one of the remaining arguments (either the integer or the double (previously a float)) as a string, resulting in a buffer overflow.

Incidentally, note that the "sengers)." at the end of buf is what was previously on the stack (I checked beforehand), and appears because the segfault occurred while buf was being written to (so before the terminating null was replaced).

So, that is what is happening. The question is: why?
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: TurfIt on May 06, 2018, 07:30:56 PM
Wrong format specifier - distance is a double trying to be printed with a %i.
Attached can be used to enable checks of the specifiers at compile time (GCC only - no idea if MSVC can do this...), but of course can't check for translations that mangle the specifiers (really really bad design having them in user changeable files...)
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 06, 2018, 10:06:20 PM
Thank you both very much for that: I have now fixed the incorrect format specifiers.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: ACarlotti on May 06, 2018, 10:43:49 PM
Quote from: jamespetts on May 06, 2018, 10:06:20 PM
I have now fixed the incorrect format specifiers.
I disagree. I think you mean "%.2f", not "%f.2".

Having read the linked page, I think what was happening is that the int and pointer arguments are stored in one place, and the floats in another. Then when parsing the format string, it took the first int/pointer as a pointer, the second int/pointer as an int and then the third int/pointer as a pointer, when this data passed in was in fact a pointer (correct), a pointer (wrong, but no crash) and an int (segfault on dereferencing the non-pointer).
https://stackoverflow.com/questions/44571175/why-are-the-int-and-float-passed-in-printf-going-to-the-wrong-positions-in-the-f

(I actually noticed the wrong specifier, but assumed both that it could be valid usage, and that in any case it couldn't be causing the effects I was seeing. So wrong on both count.)
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 06, 2018, 10:48:57 PM
Thank you - syntax error now corrected.

Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: Matthew on May 07, 2018, 02:31:28 AM
The change in line 981 of player/simplay.cc now reads "%.f2fkm", which is different from both line 988 and ACarlotti's advice. I have no idea what is right  ??? but it looks like it might possibly be a typo.
Title: Re: Modifying schedules of lines with range-restricted convoys crashes the server
Post by: jamespetts on May 07, 2018, 11:42:09 AM
Thank you for spotting that: now fixed.