crash with move_units_fake

Having trouble with the game? Report issues and get help here. Read this first!

Moderator: Forum Moderators

Forum rules
Before reporting issues in this section, you must read the following topic:
Post Reply
MadMax
Posts: 1792
Joined: June 6th, 2004, 3:29 pm
Location: Weldyn, Wesnoth

crash with move_units_fake

Post by MadMax »

OSX 10.10.3
Wesnoth 1.13.0

[move_units_fake] causes a hard crash. Reproducible with FtF scenario 2 (rescuing the hatchlings).
Spoiler:
"ILLEGITIMIS NON CARBORUNDUM"

Father of Flight to Freedom
http://www.wesnoth.org/wiki/FlightToFreedom
User avatar
Elvish_Hunter
Posts: 1600
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: crash with move_units_fake

Post by Elvish_Hunter »

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 253 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.
Current maintainer of these add-ons, all on 1.16:
The Sojournings of Grog, Children of Dragons, A Rough Life, Wesnoth Lua Pack, The White Troll (co-author)
User avatar
Iris
Site Administrator
Posts: 6800
Joined: November 14th, 2006, 5:54 pm
Location: Chile
Contact:

Re: crash with move_units_fake

Post by Iris »

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.
Author of the unofficial UtBS sequels Invasion from the Unknown and After the Storm.
User avatar
Elvish_Hunter
Posts: 1600
Joined: September 4th, 2009, 2:39 pm
Location: Lintanir Forest...

Re: crash with move_units_fake

Post by Elvish_Hunter »

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 .
Current maintainer of these add-ons, all on 1.16:
The Sojournings of Grog, Children of Dragons, A Rough Life, Wesnoth Lua Pack, The White Troll (co-author)
Post Reply