Invalid enum load in monster::store

[hexagonrecursion]

Describe the bug

src/savegame_json.cpp:1973:9: runtime error: load of value 1172321806, which is not a valid value for type 'monster_horde_attraction'
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior src/savegame_json.cpp:1973:9 in 

(lldb) bt
* thread #1, name = 'cataclysm-tiles', stop reason = Invalid enum load
  * frame #0: 0x000000000216db50 cataclysm-tiles`__ubsan_on_report
    frame #1: 0x000000000216812d cataclysm-tiles`__ubsan::Diag::~Diag() + 669
    frame #2: 0x0000000002169f9c cataclysm-tiles`handleLoadInvalidValue(__ubsan::InvalidValueData*, unsigned long, __ubsan::ReportOptions) + 492
    frame #3: 0x000000000216d5f5 cataclysm-tiles`__ubsan_handle_load_invalid_value + 37
    frame #4: 0x00000000059416e1 cataclysm-tiles`monster::store(this=0x000061d00058ca90, json=0x00007fffffff8e20) const at savegame_json.cpp:1973:9
    frame #5: 0x000000000593e4fd cataclysm-tiles`monster::serialize(this=0x000061d00058ca90, json=0x00007fffffff8e20) const at savegame_json.cpp:1929:5
    frame #6: 0x00000000058ca70b cataclysm-tiles`Creature_tracker::serialize(this=<unavailable>, jsout=0x00007fffffff8e20) const at savegame.cpp:1733:15
    frame #7: 0x0000000005890670 cataclysm-tiles`game::serialize(this=0x00006190000c1c80, fout=<unavailable>) at savegame.cpp:96:10
    frame #8: 0x00000000029a5b6f cataclysm-tiles`write_to_file(path="./save/Test2/#R3JlZ2cgQXJhZ29u.sav", writer=0x00007fffffff9ae0)> const&) at cata_utility.cpp:358:5
    frame #9: 0x00000000029a5d63 cataclysm-tiles`write_to_file(path="./save/Test2/#R3JlZ2cgQXJhZ29u.sav", writer=<unavailable>, fail_message=<unavailable>)> const&, char const*) at cata_utility.cpp:366:9
    frame #10: 0x00000000033da089 cataclysm-tiles`game::save_player_data(this=<unavailable>) at game.cpp:2876:29
    frame #11: 0x00000000033dae40 cataclysm-tiles`game::save(this=0x00006190000c1c80) at game.cpp:2925:14
    frame #12: 0x000000000367d30b cataclysm-tiles`game::handle_action(this=0x00006190000c1c80) at handle_action.cpp:2155:25
    frame #13: 0x000000000339db39 cataclysm-tiles`game::do_turn(this=0x00006190000c1c80) at game.cpp:1450:21
    frame #14: 0x000000000418f650 cataclysm-tiles`main(argc=<unavailable>, argv=0x00007fffffffe0c8) at main.cpp:688:20
    frame #15: 0x00007ffff78d5f43 libc.so.6`__libc_start_main(main=(cataclysm-tiles`main at main.cpp:135), argc=1, argv=0x00007fffffffe0c8, init=<unavailable>, fini=<unavailable>, rtld_fini=<unavailable>, stack_end=0x00007fffffffe0b8) at libc-start.c:308:16
    frame #16: 0x0000000002054a6e cataclysm-tiles`_start + 46

Steps To Reproduce

Steps to reproduce the behavior:

1. compile the game with clang UndefinedBehaviorSanitizer
2. start a new game via "play now (fixed scenario)"
3. save and quit

Note: this is not 100% reliable, but triggers most of the time for me.

Expected behavior

UBSan does not report the error.

Versions and configuration

• OS: Linux
  • OS Version: Fedora 30
• Game Version: 0.D-11025-g9fe2e72d43 [64-bit]
• Graphics Version: Tiles
• Mods loaded: [
  Dark Days Ahead [dda],
  Disable NPC Needs [no_npc_food]
  ]

Additional context

I was unable to generate a save file that would reproduce this. loading the save and saving it again does not trigger the issue.

[hexagonrecursion]

I figured out why bad enum value is not saved

Cataclysm-DDA/src/savegame_json.cpp

Lines 1973 to 1975 in 397d11e

	if( horde_attraction > MHA_NULL && horde_attraction < NUM_MONSTER_HORDE_ATTRACTION ) {
	json.member( "horde_attraction", horde_attraction );
	}

[hexagonrecursion]

This error might be caused by an uninitialized memory read.
horde_attraction is not initialized in monster.h

Cataclysm-DDA/src/monster.h

Line 538 in 397d11e

monster_horde_attraction horde_attraction;

or in monster::monster()

Cataclysm-DDA/src/monster.cpp

Lines 185 to 245 in 397d11e

	monster::monster()
	{
	position.x = 20;
	position.y = 10;
	position.z = -500; // Some arbitrary number that will cause debugmsgs
	unset_dest();
	wandf = 0;
	hp = 60;
	moves = 0;
	friendly = 0;
	anger = 0;
	morale = 2;
	faction = mfaction_id( 0 );
	mission_id = -1;
	no_extra_death_drops = false;
	dead = false;
	death_drops = true;
	made_footstep = false;
	hallucination = false;
	ignoring = 0;
	upgrades = false;
	upgrade_time = -1;
	last_updated = 0;
	biosig_timer = -1;
	}
	monster::monster( const mtype_id &id ) : monster()
	{
	type = &id.obj();
	moves = type->speed;
	Creature::set_speed_base( type->speed );
	hp = type->hp;
	for( auto &sa : type->special_attacks ) {
	auto &entry = special_attacks[sa.first];
	entry.cooldown = rng( 0, sa.second->cooldown );
	}
	anger = type->agro;
	morale = type->morale;
	faction = type->default_faction;
	ammo = type->starting_ammo;
	upgrades = type->upgrades && ( type->half_life || type->age_grow );
	reproduces = type->reproduces && type->baby_timer && !monster::has_flag( MF_NO_BREED );
	biosignatures = type->biosignatures;
	if( monster::has_flag( MF_AQUATIC ) ) {
	fish_population = dice( 1, 20 );
	}
	if( monster::has_flag( MF_RIDEABLE_MECH ) ) {
	itype_id mech_bat = itype_id( type->mech_battery );
	const itype &type = *item::find_type( mech_bat );
	int max_charge = type.magazine->capacity;
	item mech_bat_item = item( mech_bat, 0 );
	mech_bat_item.ammo_consume( rng( 0, max_charge ), tripoint_zero );
	battery_item = mech_bat_item;
	}
	}
	monster::monster( const mtype_id &id, const tripoint &p ) : monster( id )
	{
	position = p;
	unset_dest();
	}

According to https://github.com/CleverRaven/Cataclysm-DDA/search?q=horde_attraction&unscoped_q=horde_attraction horde_attraction is only assigned to
in monster::load()

Cataclysm-DDA/src/savegame_json.cpp

Line 1901 in 6674292

horde_attraction = static_cast<monster_horde_attraction>( data.get_int( "horde_attraction", 0 ) );

in monster::get_horde_attraction(), but not before it is read, which means it should be initialized before calling get_horde_attraction()

Cataclysm-DDA/src/monster.cpp

Lines 2839 to 2845 in 605409a

	monster_horde_attraction monster::get_horde_attraction()
	{
	if( horde_attraction == MHA_NULL ) {
	horde_attraction = static_cast<monster_horde_attraction>( rng( 1, 5 ) );
	}
	return horde_attraction;
	}

and in monster::set_horde_attraction()

Cataclysm-DDA/src/monster.cpp

Lines 2847 to 2850 in 605409a

	void monster::set_horde_attraction( monster_horde_attraction mha )
	{
	horde_attraction = mha;
	}

According to https://github.com/CleverRaven/Cataclysm-DDA/search?q=set_horde_attraction&unscoped_q=set_horde_attraction set_horde_attraction is only invoked in one place: when a monster stops being part of a horde.

Cataclysm-DDA/src/map.cpp

Lines 7175 to 7178 in 397d11e

	// If a monster came from a horde population, configure them to always be willing to rejoin a horde.
	if( group.horde ) {
	tmp.set_horde_attraction( MHA_ALWAYS );
	}

Apparently when loading a saved monster the default is 0 (MHA_NULL), which get_horde_attraction later replaces with a random value (see above)

Cataclysm-DDA/src/savegame_json.cpp

Line 1901 in 6674292

horde_attraction = static_cast<monster_horde_attraction>( data.get_int( "horde_attraction", 0 ) );