crash with move_units_fake

MadMax · Post by **MadMax** » May 7th, 2015, 6:09 pm

OSX 10.10.3
Wesnoth 1.13.0

[move_units_fake] causes a hard crash. Reproducible with FtF scenario 2 (rescuing the hatchlings).

Spoiler:

Post by **Elvish_Hunter** » May 7th, 2015, 9:00 pm

I can confirm that this bug happens even on master, 1.13.0+dev (56329ac-Clean), Xubuntu 14.04, using the 1.12 version of FtF. I managed to create a savegame in which, to trigger the bug, you just need to move one of the Drakes near the Hatchlings:

FtF-Rebellion-Auto-Save8.gz: (32.63 KiB) Downloaded 168 times

I also ran Wesnoth with --log-debug all, and this is the relevant part from the log:

Code: Select all

20150507 22:47:37 info engine: Processing [move_units_fake]
20150507 22:47:37 info engine: Moving 2 units
20150507 22:47:37 debug config: trying to find Spearman in unit_type list (unit_type_data.unit_types)
20150507 22:47:37 debug unit: Building unit type Spearman, level 5
20150507 22:47:37 debug filesystem: Looking for 'portraits/humans/transparent/spearman.png'.
20150507 22:47:37 debug filesystem:   checking '/home/linux/.local/share/wesnoth/1.13/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/wesnoth-git-dev/code/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/.local/share/wesnoth/1.13/data/add-ons/Flight_Freedom/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/wesnoth-git-dev/code/data/add-ons/Flight_Freedom/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/.local/share/wesnoth/1.13/data/add-ons/Flight_Freedom_Resources/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/wesnoth-git-dev/code/data/add-ons/Flight_Freedom_Resources/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/.local/share/wesnoth/1.13/data/core/images/'
20150507 22:47:37 debug filesystem:   checking '/home/linux/wesnoth-git-dev/code/data/core/images/'
20150507 22:47:37 debug filesystem:   found at '/home/linux/wesnoth-git-dev/code/data/core/images/portraits/humans/transparent/spearman.png'
20150507 22:47:37 debug config: The config object has no child named »abilities«.
20150507 22:47:37 debug unit: inheriting from movement_type 'smallfoot'
20150507 22:47:37 debug unit: fake id: 4294967295
20150507 22:47:37 info unit: Generating a trait for unit type Spearman with musthaveonly 1
20150507 22:47:37 { BEGIN: apply mods
20150507 22:47:37 } END: apply mods (took 0ms)
20150507 22:47:37 info unit: Adding a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 0 ptr:0x3309000
20150507 22:47:37 info unit: Freshly constructed
20150507 22:47:37 info unit: Adding a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 1 ptr:0x3309000
20150507 22:47:37 info unit: Removing a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 2 ptr:0x3309000
20150507 22:47:37 info unit: Adding a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 1 ptr:0x3309000
20150507 22:47:37 info unit: Removing a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 2 ptr:0x3309000
20150507 22:47:37 info unit: Adding a reference to a unit: id = Spearman-4294967294, uid = 4294967294, refcount = 1 ptr:0x3309000
Errore di segmentazione (core dump creato)

where "Errore di segmentazione" means segmentation fault.

EDIT: apparently, it's this line that triggers the segfault:

Code: Select all

units[paths.size()] = u;

Replacing it with units.push_back(u) avoids the segfault, but then the fake units begin to move in a strange manner.

Post by **Iris** » May 8th, 2015, 12:41 am

Elvish_Hunter wrote:EDIT: apparently, it's this line that triggers the segfault:
Code: Select all
units[paths.size()] = u;
Replacing it with units.push_back(u) avoids the segfault, but then the fake units begin to move in a strange manner.

I don’t know the logic of this WML action to be able to tell why the fake units would “move in a strange manner” (elaborate?), but the original code is wrong and your fix is correct. In action_wml.cpp:768 there’s a call to std::vector<>::reserve(), which only preallocates storage for elements but does not actually initialize their memory or call their constructors (unlike resize() or push_back()). This wouldn’t necessarily be a problem (but it’d still be UB) if the vector’s element type were a POD type, but that is clearly not the case here.

Post by **Elvish_Hunter** » May 9th, 2015, 8:06 pm

shadowm wrote:I don’t know the logic of this WML action to be able to tell why the fake units would “move in a strange manner” (elaborate?)

At first, I didn't understand what was happening, but after several replays of the same turn I got it. What happens is that, after being moved by one step, the fake unit should stay visible on screen until its next step, or until the tag execution is over. This is the behaviour on 1.12.
On 1.13, after each step the unit disappears, to reappear only for the next step - so [move_units_fake] can effectively show only one unit at any given time. I suspect that the culprit may be this line:

Code: Select all

units[un]->anim_comp().set_standing();

but I'm not sure. Probably iceiceice knows more about this, given that he refactored the code to use the new fake_unit_ptr data type.

shadowm wrote:fix is correct. In action_wml.cpp:768 there’s a call to std::vector<>::reserve(), which only preallocates storage for elements but does not actually initialize their memory or call their constructors (unlike resize() or push_back()).

Fix pushed to master: http://git.io/vUsmb .

The Battle for Wesnoth Forums

crash with move_units_fake

crash with move_units_fake

Re: crash with move_units_fake

Re: crash with move_units_fake

Re: crash with move_units_fake