Cataclysm-dda: Segfault while fixing clothes inside a vehicle

[YukariMindGap]

Describe the bug
I got a segmentation fault error while fixing my clothes inside a vehicle.

To Reproduce
Steps to reproduce the behavior:

1. Try to fix clothes inside the vehicle

Expected behavior
The clothes fixing will continue as usual

Screenshots
If applicable, add screenshots to help explain your problem.

Versions and configuration(please complete the following information):

• OS: Windows 7
• Game Version: 0.D-2318-g1323624
• Graphics version: Tiles
• Mods loaded:
  "dda", "aftershock","Tolerate_This", "FIC_Weapons", "realguns", "growable-pots", "ew_pack",
  "makeshift", "More_Survival_Tools", "nw_pack", "modular_turrets", "Salvaged_Robots", "alt_map_key", "hydroponics", "MMA", "more_locations", "FujiStruct", "Urban_Development", "boats", "deoxymod", "blazemod", "Tanks", "DP_XOTTO_INDICATORS", "more_classes_scenarios", "no_npc_food"

Additional context
Crash and Debug logs
crash.log
debug.log
Save
Main World_copy.zip

[neitsa]

I have been able to repro the crash on the latest experimental version (as of today 0.D-2478-ge68efed92d).

TL;DR

While trying to fix a (worn?) piece of clothes with repeat fully until it is repaired, the item might be destroyed (in the game and c++ sense), but the code still try to reuse the destroyed object.

Repro Steps

• Load the world :p
• Don't move and stay in the car (I don't think the bug related to vehicle though)
• Pick up the sewing kit
  
  
  
  • Open the advanced inventory: /
  
  
  • Right pane should be on Area focus.
  
  
  • Filter the area to find the sewing kit.
  
  
  • On the left pane, select your Inventory
  
  
  • Pick up the sewing kit
  
  
  • Close advanced invoentory (ESC)
  
  
• Open your Inventory
• Activate the sewing kit
• On the repair what? screen select the briefs (player is currently wearing this item)
• On the repairing briefs screen
  
  
  
  • choose 3: Repeat until fully repaired, but don't reinforce
  
  
  • note: I haven't tested with the other options
  
  
• Crash

Note: there's a roaming German shepherd near the car and it might interrupt the fixing job, just choose to ignore it and continue. The interruption doesn't seem to be related to the bug.

Stack trace

>   Cataclysm.exe!item::damage() Line 441   C++
    Cataclysm.exe!activity_handlers::repair_item_finish(player_activity * act, player * p) Line 2361    C++
    [External Code] 
    Cataclysm.exe!activity_type::call_finish(player_activity * act, player * p) Line 99 C++
    Cataclysm.exe!player_activity::do_turn(player & p) Line 123 C++
    Cataclysm.exe!game::process_activity() Line 1592    C++
    Cataclysm.exe!game::do_turn() Line 1403 C++
    Cataclysm.exe!SDL_main(int argc, char * * argv) Line 689    C++
    Cataclysm.exe!main_getcmdline(...) Line 177 C
    [External Code] 

Analysis

Everything happens in void activity_handlers::repair_item_finish( player_activity *act, player *p ) in \src\activity_handlers.cpp.

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2290-L2291

The current fix location is obtained as follow:

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2321-L2323

The fix object is a shared_ptr (std::shared_ptr<item_location::impl>):

>? &fix.ptr
0x000001e18e0117f0 shared_ptr {who={stomach={vitamins={ size=0x0000000000000005 } vitamins_absorbed={ size=0x0000000000000005 } calories=...} ...} } [0x00000001 strong ref] [default]
    [ptr]: 0x000001e1a039a930 {who={stomach={vitamins={ size=0x0000000000000005 } vitamins_absorbed={ size=0x0000000000000005 } ...} ...} }
    [control block]: default
    [Raw View]: 0x000001e18e0117f0 {...}

At some point the item is destroyed:

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2344-L2352

We have, after the above block of code:

• destroyed is true
• attempt set to AS_DESTROYED
• act->targets contains now only one element (it must contain 2 elements to enter the top if block) which clearly shows the item was removed.

>? destroyed
true
>? attempt
AS_DESTROYED (0x00000003)
>? act->targets
{ size=0x0000000000000001 }
    [capacity]: 0x0000000000000002
    [allocator]: allocator
    [0x00000000]: {ptr=shared_ptr {who={stomach={vitamins={ size=0x0000000000000005 } vitamins_absorbed={ size=0x0000000000000005 } calories=...} ...} } [0x00000001 strong ref] [default] }
    [Raw View]: {...}

This basically means the act->targets[1] object has been freed during the act->targets.pop_back(); line above. This means that the fix object is no longer valid.

Then just a few lines below:

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2359-L2362

Note: repeat is set to REPEAT_FULL

>? repeat
REPEAT_FULL (0x00000003)

fix->damage() is called but fix is now a deleted object, leading to the crash.

This is the fix object just before calling fix->damage(), clearly showing a deleted object:

>? fix
{ptr=shared_ptr {what=0xdddddddddddddddd {type=??? contents={ size=??? } components={ size=??? } ...} idx=0xdddddddd ...} [0xdddddddd strong refs, 0xdddddddc weak refs] [{_Uses=0xdddddddd _Weaks=0xdddddddd }] }
    ptr: shared_ptr {what=0xdddddddddddddddd {type=??? contents={ size=??? } components={ size=??? } ...} idx=0xdddddddd ...} [0xdddddddd strong refs, 0xdddddddc weak refs] [{_Uses=0xdddddddd _Weaks=0xdddddddd }]
>

Bugfix

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2344-L2352

Notice that as soon as act->targets.pop_back() is called and we have repeat == REPEAT_FULL the bug will be triggered in fix->damage():

https://github.com/CleverRaven/Cataclysm-DDA/blob/e68efed92d7a46a5784c2b6e0a898e1457626956/src/activity_handlers.cpp#L2359-L2362

Thus to fix the bug we need to check if any of the conditions - leading to act->targets.pop_back() - was true and if we have REPEAT_FULL. In this case we need to prevent the call to fix->damage().