[master 2ae37e1b7] Possible crash on startup related to accommodation classes

[ceeac]

Steps to reproduce:

1. Compile and link with -fsantitize=address,undefined with GCC or Clang
2. Run the game

Result:

Code Select Expand

=================================================================
==90057==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200043fef8 at pc 0x55673209e431 bp 0x7fffd051e260 sp 0x7fffd051e250
READ of size 2 at 0x60200043fef8 thread T0
    #0 0x55673209e430 in vehicle_desc_t::get_max_accommodation_class() const /home/ceeac/Projects/code/simutrans-ex/descriptor/vehicle_desc.cc:476
    #1 0x556731c64d72 in compare_freight /home/ceeac/Projects/code/simutrans-ex/bauer/vehikelbauer.cc:209
    #2 0x556731c67d1b in vehicle_builder_t::compare_vehicles(vehicle_desc_t const*, vehicle_desc_t const*, vehicle_builder_t::sort_mode_t) /home/ceeac/Projects/code/simutrans-ex/bauer/vehikelbauer.cc:309
    #3 0x556731c68056 in compare /home/ceeac/Projects/code/simutrans-ex/bauer/vehikelbauer.cc:330
    #4 0x556731c7d536 in bool __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>::operator()<vehicle_desc_t**, vehicle_desc_t**>(vehicle_desc_t**, vehicle_desc_t**) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50fd536)
    #5 0x556731c7d049 in void std::__move_median_to_first<vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)> >(vehicle_desc_t**, vehicle_desc_t**, vehicle_desc_t**, vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50fd049)
    #6 0x556731c7c756 in vehicle_desc_t** std::__unguarded_partition_pivot<vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)> >(vehicle_desc_t**, vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50fc756)
    #7 0x556731c7c1ee in void std::__introsort_loop<vehicle_desc_t**, long, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)> >(vehicle_desc_t**, vehicle_desc_t**, long, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50fc1ee)
    #8 0x556731c7b41f in void std::__sort<vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)> >(vehicle_desc_t**, vehicle_desc_t**, __gnu_cxx::__ops::_Iter_comp_iter<bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50fb41f)
    #9 0x556731c785fe in void std::sort<vehicle_desc_t**, bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)>(vehicle_desc_t**, vehicle_desc_t**, bool (*)(vehicle_desc_t const*, vehicle_desc_t const*)) (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x50f85fe)
    #10 0x556731c6838c in vehicle_builder_t::successfully_loaded() /home/ceeac/Projects/code/simutrans-ex/bauer/vehikelbauer.cc:351
    #11 0x55673205c5b0 in vehicle_reader_t::successfully_loaded() const /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/vehicle_reader.cc:34
    #12 0x55673201de49 in obj_reader_t::finish_rd() /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/obj_reader.cc:64
    #13 0x556733cc1ac7 in simu_main(int, char**) /home/ceeac/Projects/code/simutrans-ex/simmain.cc:1269
    #14 0x556733d48981 in sysmain(int, char**) /home/ceeac/Projects/code/simutrans-ex/sys/simsys.cc:1102
    #15 0x55673450163b in main /home/ceeac/Projects/code/simutrans-ex/sys/simsys_s2.cc:1017
    #16 0x7f47e4de0d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #17 0x7f47e4de0e3f in __libc_start_main_impl ../csu/libc-start.c:392
    #18 0x556731b374c4 in _start (/media/ceeac/Projects/code/simutrans-ex/build/simutrans/simutrans-extended+0x4fb74c4)

0x60200043fef8 is located 2 bytes to the right of 6-byte region [0x60200043fef0,0x60200043fef6)
allocated by thread T0 here:
    #0 0x7f47e5eb5337 in operator new[](unsigned long) ../../../../src/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x55673206dd2c in vehicle_reader_t::read_node(_IO_FILE*, obj_node_info_t&) /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/vehicle_reader.cc:430
    #2 0x556732020834 in obj_reader_t::read_nodes(_IO_FILE*, obj_desc_t*&, int, unsigned int) /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/obj_reader.cc:238
    #3 0x556732020b1c in obj_reader_t::read_nodes(_IO_FILE*, obj_desc_t*&, int, unsigned int) /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/obj_reader.cc:242
    #4 0x55673201fe5b in obj_reader_t::read_file(char const*) /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/obj_reader.cc:199
    #5 0x55673201f507 in obj_reader_t::load(char const*, char const*) /home/ceeac/Projects/code/simutrans-ex/descriptor/reader/obj_reader.cc:152
    #6 0x556733cc0f21 in simu_main(int, char**) /home/ceeac/Projects/code/simutrans-ex/simmain.cc:1246
    #7 0x556733d48981 in sysmain(int, char**) /home/ceeac/Projects/code/simutrans-ex/sys/simsys.cc:1102
    #8 0x55673450163b in main /home/ceeac/Projects/code/simutrans-ex/sys/simsys_s2.cc:1017
    #9 0x7f47e4de0d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

SUMMARY: AddressSanitizer: heap-buffer-overflow /home/ceeac/Projects/code/simutrans-ex/descriptor/vehicle_desc.cc:476 in vehicle_desc_t::get_max_accommodation_class() const
Shadow bytes around the buggy address:
  0x0c048007ff80: fa fa 00 fa fa fa 00 00 fa fa 00 00 fa fa 00 fa
  0x0c048007ff90: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 00
  0x0c048007ffa0: fa fa 00 fa fa fa 00 00 fa fa 00 fa fa fa 00 00
  0x0c048007ffb0: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 fa
  0x0c048007ffc0: fa fa 00 00 fa fa 00 00 fa fa 00 fa fa fa 00 00
=>0x0c048007ffd0: fa fa 00 fa fa fa 00 00 fa fa 03 fa fa fa 06[fa]
  0x0c048007ffe0: fa fa 00 01 fa fa 00 00 fa fa 00 01 fa fa 00 00
  0x0c048007fff0: fa fa 00 00 fa fa 00 00 fa fa 00 00 fa fa 00 fa
  0x0c0480080000: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 00
  0x0c0480080010: fa fa 00 fa fa fa 00 00 fa fa 00 fa fa fa 00 00
  0x0c0480080020: fa fa 00 00 fa fa 00 fa fa fa 00 00 fa fa 00 fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==90057==ABORTING


[jamespetts]

Thank you for that report - I think that this is from GUI code?

[ceeac]

No - The issue occurs because the number of classes is not updated before being used by the compare_freight function in vehikelbauer.cc during pakset loading. Calling vehicle_desc_t::fix_number_of_classes before the sort in vehicle_builder_t::successfully_loaded should fix the issue, I think. Check PR#567.

[jamespetts]

No - The issue occurs because the number of classes is not updated before being used by the compare_freight function in vehikelbauer.cc during pakset loading. Calling vehicle_desc_t::fix_number_of_classes before the sort in vehicle_builder_t::successfully_loaded should fix the issue, I think. Check PR#567.

Excellent, thank you; now incorporated.