[Bug] marker text becomes undef and will crash the game on selection

[Sirius]

Hey there,
I just found a critical bug crashing the game:
When the terrain under a marker changes, so that the marker is now on another z coordinate, the markers text changes to "undef". When selecting that marker, the game will crash.

You can reproduce this, for example, the following way:
1. look for a natural slope or create one. A natural slope is a slope that will still be a slope when using the "restore natural slope" toll on it.
2. flatten that slope so that you can place a large signalbox on it.
3. Place the signalbox on the flattened tile.
4. Place a marker on one of the signalboxes tiles.
5. Delete the other tile of the signalbox.
    Now the slope will have reset to it's natural slope and the markers text will be "undef".
6. Select the marker.
=> crash

Didn't test with other multitile buildings as I didn't find any others.

[Matthew]

I wonder whether the underlying issue here is the ability to place a marker on top of player buildings? That seems to cause all kinds of weird cases.

But I am not a coder, so just guessing.

[Sirius]

Debug pending...

Code Select Expand

Thread 1 "simutrans-exten" received signal SIGSEGV, Segmentation fault.
0x000000000047014d in grund_t::get_flag (this=0x0, flag=grund_t::has_text) at boden/../obj/../boden/grund.h:228
228             inline bool get_flag(flag_values flag) const {return (flags & flag) != 0;}
(gdb) bt full
#0  0x000000000047014d in grund_t::get_flag (this=0x0, flag=grund_t::has_text) at boden/../obj/../boden/grund.h:228
No locals.
#1  0x0000000000463921 in grund_t::get_text (this=0x0) at boden/grund.cc:124
        result = 0x0
        __PRETTY_FUNCTION__ = "const char* grund_t::get_text() const"
#2  0x000000000057ee5c in label_info_t::label_info_t (this=0x34ed1710, l=0x3b37a340) at gui/label_info.cc:40
        p_name = 0x5a2e970 "Simutrans-Spieler"
        min_width = 290
        gr = 0x0
        p_name = <optimized out>
        min_width = <optimized out>
        gr = <optimized out>
#3  0x00000000004c75a3 in label_t::show_info (this=0x3b37a340) at obj/label.cc:107
        l = 0x3b37a340
#4  0x0000000000771d07 in tool_query_t::work (this=0x5bb2120, player=0x337a5950, pos=...) at simtool.cc:371
        lb = 0x3b37a340
        old_count = 3
        gr = 0x7fff6c0594b8
#5  0x00000000007c4fbe in karte_t::call_work (this=0x44ba4c80, tool=0x5bb2120, player=0x337a5950, pos=..., suspended=@0x7fffffffafaa: false) at simworld.cc:10108
        err = 0x0
#6  0x0000000000750708 in interaction_t::interactive_event (this=0x2f5b4040, ev=...) at siminteraction.cc:240
        suspended = false
        tool = 0x5bb2120
        player = 0x337a5950
        err = 0x0
        pos = {x = 186, y = 240, z = 1 '\001', static invalid = {x = -1, y = -1, z = -1 '\377', static invalid = <same as static member of an already seen type>}}
#7  0x00000000007510eb in interaction_t::process_event (this=0x2f5b4040, ev=...) at siminteraction.cc:417
        left_drag = false
#8  0x00000000007511e4 in interaction_t::check_events (this=0x2f5b4040) at siminteraction.cc:439
        ev = {ev_class = 6, {ev_code = 1, ev_ptr = 0x1}, mx = 1029, my = 465, cx = 1029, cy = 465, button_state = 0, ev_key_mod = 0}
#9  0x00000000007c6549 in karte_t::interactive (this=0x44ba4c80, quit_month=2147483647) at simworld.cc:10417
        time = 98289
        hashes_ok = {data = 0x0, size = 0, count = 0}
        ms_difference = 0
#10 0x000000000075cb9f in simu_main (argc=2, argv=0x7fffffffdc38) at simmain.cc:1382
        pause_after_load = false
        welt = 0x44ba4c80
        view = 0x2f5adab0
        eventmanager = 0x2f5b4040
        resolutions = {{640, 480}, {800, 600}, {1024, 768}, {1280, 1024}, {704, 560}}
        disp_width = 704
        disp_height = 560
        fullscreen = 0
        quit_month = 2147483647
        path_sep = 0x88641e "/"
        pak_diagonal_multiplier = 724
        pak_tile_height = 8 '\b'
        pak_height_conversion_factor = 2 '\002'
        found_settings = true
        found_simuconf = true
        multiuser = true
        simuconf = {file = 0x0}
        path_to_simuconf = "config/simuconf.tab\000\000\000\000"
        version = 0x8864f0 "Simutrans version 120.2.1 Extended Nightly development build 14.5 from Sep  7 2019 #dc95dc0\n"
        cli_syslog_enabled = false
        cli_syslog_tag = 0x0
        file = {mode = 6, saving = false, buffered = false, curr_buff = 4160741648, buf_pos = {32767, 0}, buf_len = {0, 0}, ls_buf = {0x0, 0x0}, version = 120004, extended_version = 14, extended_revision = 12, ident = 32767,
          pak_extension = "settings only\000\000\000\020\341\377\367\377\177\000\000\334>@\000\000\000\000\000\377\377\377\377", '\000' <repeats 12 times>, "\060\222{\366\377\177\000\000\340$\374\367\377\177", '\000' <repeats 90 times>, "\020d\266\000\000\000\000\000\300q@\000\000\000\000\000\060\334\377\377\377\177", '\000' <repeats 18 times>, "\060\333\377\377\377\177\000\000"..., filename = {static npos = 18446744073709551615,
            _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0xd053d0 "settings-extended-debug.xml"}, _M_string_length = 27, {_M_local_buf = "\036", '\000' <repeats 14 times>,
              _M_allocated_capacity = 30}}, fd = 0xd04f10, static save_mode = loadsave_t::zipped, static autosave_mode = loadsave_t::zipped}
        xml_filename = "settings-extended-debug.xml\000\000\000\000"
--Type <RET> for more, q to quit, c to continue without paging--c
        xml_settings_found = true
        obj_conf = {static npos = 18446744073709551615, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x24e81e0 "/home/dome/simutrans/simuconf.tab"}, _M_string_length = 33, {_M_local_buf = "*\000\000\000\000\000\000\000\357\022\336\367\377\177\000", _M_allocated_capacity = 42}}
        themes_ok = true
        parameter = {0, 0}
        new_world = true
        loadgame = {static npos = 18446744073709551615, _M_dataplus = {<std::allocator<char>> = {<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No data fields>}, _M_p = 0x2c510b0 ""}, _M_string_length = 0, {_M_local_buf = "8\000\000\000\000\000\000\000\066\340\343\371\000\000\000", _M_allocated_capacity = 56}}
#11 0x000000000076fc60 in sysmain (argc=2, argv=0x7fffffffdc38) at simsys.cc:825
        buffer2 = 0x0
        buffer = "/home/dome/games/simutrans-ex-src/simutrans-extended/build/default/simutrans-extended\000\000\000\000\200 \000\000\000\000\000Lr \000\000\000\000\000\300{ \000\000\000\000\000\000`\000\000\000\000\000\000\003\000\000\000\000\000\000\000\000  \000\000\000\000\000\000@ \000\000\000\000\000\210\060 \000\000\000\000\000\020\061 \000\000\000\000\000\000 \000\000\000\000\000\000\003", '\000' <repeats 95 times>...
        length = 85
#12 0x000000000083bc20 in main (argc=2, argv=0x7fffffffdc38) at simsys_s2.cc:792
No locals.



Edit: It seems that the labels koord is invalid due to the terrain alignment. Labels on their own don't create their own
Additionally, when changing the value of such an invalid label, the game will also crash.

Changing the following lines in label_info.cc hotfixes this.
40:

Code Select Expand

if(gr && gr->get_text()) {
73:

Code Select Expand

if(gd && ((gd->get_text() == NULL && edit_name) || strcmp(gd->get_text(), edit_name)) )

This at least prevents crashes but still leaves an invalid label on the map. You can click on it, but it will always be empty, even if you set a new text to it (that's basically what the hotfix does) always showing undef on the map

To really fix this, we have to update the labels koord when the terrain underneath gets alligned or we have to prevent the allignment at all.
I prefer the first, as this would also allow us to allign terrain with labels on it, which would improve usability a bit.

[DrSuperGood]

Is this related to the "zeiger" (terrain pointer/marker) destruction crash that I fixed earlier this year in Standard and which fix has not yet been back ported to Extended?

When a ground is destroyed it automatically destroys all objects attached to it, including zeiger created by things such as the schedule window. The schedule window is then left with a dangling pointer resulting in it manipulating memory garbage and eventually trying to double free memory which is very likely to cause a crash.

The test for this involved creating a bridge. Creating a schedule that stops at some point on the bridge span. Closing the schedule window. Deleting the bridge. Opening the schedule window. Dragging a new bridge to replace the old one. Finally cancelling the bridge placement or reducing its span so as to no longer cover the scheduled stops would cause a crash or show graphic artefacts.

[Sirius]

Not quite sure but I don't think it is related.
I guess there is some confusion about the term "marker". What I meant is the "Marker" special construction tool, which is obj/label.cc  and gui/label_info.cc.

However, as you have mentioned standard, I have just tried to reproduce this with standard 120.4.1 and it is reproducible there, e.g. using the 2x1 station extension.
So it is is not specifically an extended bug.

[jamespetts]

Thank you very much for the report and fix: I have now applied the fix so as to prevent crashing. Anything more sophisticated such as trying to prevent invalid markers from existing will have to await me having a proper computer.

[Sirius]

Additionally note that a map reload (save/load) seems to fix these broken markers.
However, I don't know what exactly is happening at serialization/deserialization, so knowing that is not pretty useful to me, but maybe it is to someone else.

[jamespetts]

Additionally note that a map reload (save/load) seems to fix these broken markers.
However, I don't know what exactly is happening at serialization/deserialization, so knowing that is not pretty useful to me, but maybe it is to someone else.

If you want to look at the loading/saving code, look at the rdwr methods in each of the various classes. These essentially rely on reading/writing raw data in a hard-coded sequence which is determined by the file version number.