Segmentation fault

[raz0rknaif]

Thread 1 "7kaa" received signal SIGSEGV, Segmentation fault.
Spy::capture_firm (this=this@entry=0x5555586f696c) at OSPY.cpp:923
923 int nationReputation = (int) nation_array[true_nation_recno]->reputation;

(gdb) bt
#0 Spy::capture_firm (this=this@entry=0x5555586f696c) at OSPY.cpp:923
#1 0x0000555555626fd6 in Spy::think_bribe (this=)
at OSPY2.cpp:228
Backtrace stopped: Cannot access memory at address 0x7fffffffdd68

[the3dfxdude]

Stack is corrupted. Can you explain how to reproduce?

[raz0rknaif]

I cannot explain it, but I have the saved game. It segfaults almost immediately.

…

On Sun, Jun 23, 2019 at 10:55 PM the3dfxdude ***@***.***> wrote: Stack is corrupted. Can you explain how to reproduce? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#154?email_source=notifications&email_token=AG7OEIZH4B5TVPVX4G4ZUP3P4BAU5A5CNFSM4H22OLPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYLV2SA#issuecomment-504847688>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG7OEIZD4PYKCDORAL4NUPLP4BAU5ANCNFSM4H22OLPA> .

-- Timothy Rink: 6462CD04 <https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x0C7780B26462CD04> @ https://keyserver.ubuntu.com/ for PGP key.

[raz0rknaif]

(gdb) bt
#0 Spy::capture_firm (this=this@entry=0x5555586d400c) at OSPY.cpp:923
#1 0x0000555555626fd6 in Spy::think_bribe (this=this@entry=0x555558ccaf90)
at OSPY2.cpp:228
#2 0x0000555555627758 in Spy::think_firm_spy (this=this@entry=0x555558ccaf90)
at OSPY2.cpp:125
#3 0x0000555555627926 in Spy::process_ai (this=this@entry=0x555558ccaf90)
at OSPY2.cpp:46
#4 0x0000555555627ec8 in SpyArray::next_day (this=0x5555558f84a0 <spy_array>)
at OSPYA.cpp:190
#5 0x000055555562eae4 in Sys::process (this=this@entry=0x5555558fcf20 )
at OSYS2.cpp:222
#6 0x000055555562d386 in Sys::main_loop (
this=this@entry=0x5555558fcf20 , isLoadedGame=isLoadedGame@entry=1)
at OSYS.cpp:825
#7 0x000055555562d645 in Sys::run (this=0x5555558fcf20 ,
isLoadedGame=isLoadedGame@entry=1) at OSYS.cpp:572
#8 0x000055555557b925 in Battle::run_loaded (this=)
at OBATTLE.cpp:652
#9 0x00005555555c008d in Game::single_player_menu (this=0x5555558f71e0 )
at OGAMMAIN.cpp:625
#10 0x00005555555c07ed in Game::run_main_menu_option (
this=this@entry=0x5555558f71e0 , optionId=optionId@entry=1)
at OGAMMAIN.cpp:306
---Type to continue, or q to quit---
#11 0x00005555555c0c72 in Game::main_menu (this=0x5555558f71e0 )
at OGAMMAIN.cpp:256
#12 0x0000555555565c1c in main (argc=1, argv=0x7fffffffe048) at AM.cpp:387
(gdb)

[the3dfxdude]

Send me the save file.

[the3dfxdude]

What OS ? Looks 64-bit so I guess linux.

[raz0rknaif]

Yes, Linux. Debian 9 Stable to be exact.

…

On Mon, Jun 24, 2019 at 11:20 AM the3dfxdude ***@***.***> wrote: What OS ? Looks 64-bit so I guess linux. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#154?email_source=notifications&email_token=AG7OEI4BUZTVARY766LNVGLP4DX3VA5CNFSM4H22OLPKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYNOTKI#issuecomment-505080233>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AG7OEI4UE6OEOASXUHYTUCDP4DX3VANCNFSM4H22OLPA> .

-- Timothy Rink: 6462CD04 <https://keyserver.ubuntu.com/pks/lookup?op=get&search=0x0C7780B26462CD04> @ https://keyserver.ubuntu.com/ for PGP key.

[the3dfxdude]

I've tracked it down, and this needs to be fixed. It shows machine dependent execution through a certain situation which will be crashes / desync, etc. Actually a whole class of potential problems really, so a project in itself to track down the bugs caused from the implementation.

[the3dfxdude]

This is fixed, but I should explain this some. The problem is the Spy class eventually calls spy_array.resize (DynArray managed), while having Spy pointers stored, including on the call stack. This is bad, and likely never worked. A Spy class method cannot cause spy_array to resize.

There are three ways I considered fixing this:

1. Split out a processing class from the Spy class, improving encapsulation, and the high level processing is forced through recno, not a pointer, so an invalidated pointer can never be stored. Locking can ensure Spy throws an error if it ever accidentally triggers the issue again.
2. Use an action queue for things like spy_array.add_spy to be processed later (splits in the middle of the method that triggers the crash). Complicates things, and could break existing compatibility, since a queue means another thing to be tracked.
3. Rewrite all the derived DynArray classes so it's properly encapsulated. This could ensure existing compatibility, but a massive amount of work.

With these series of commits, you can see option #1 looks the easiest and cleanest without triggering a massive rewrite.
8611e24
1a54078
3cacff6
05b1e86

The issue found here does apply in other places. I was able to trigger a crash in Unit after building an improvised static call tree looking for problems. So there probably will be more changes later.