ToME: the Tales of Maj'Eyal
http://forums.te4.org/

FOV memory issue
http://forums.te4.org/viewtopic.php?f=45&t=38631
Page 1 of 1

Author:  Harms [ Mon Aug 12, 2013 12:21 pm ]
Post subject:  FOV memory issue

Hey!
I am (as of not long ago) the package maintainer of tome4 in AUR.
I know the language C quite well, and I have noticed a bug in fov/fov.c.
It caused my compilation of tome4 to crash every time as the game was attempting to render the background of the main menu.

Putting a watch on global_shape showed that this variable was written to inside LARGE_ASS_FOV_DEFINE_OCTANT, and further investigations lead me to line 599 and GET_BUFFER:
Code:
578        #define GET_BUFFER(target, buffer_data, len)                                                      \
579            /* hurray, no branching, modulus, or malloc! */                                               \
580            buffer_data->index = (buffer_data->index + buffer_data->prev_len) & (FOV_BUFFER_SIZE - 1);    \                                                                                             \
581            buffer_data->prev_len = len;                                                                  \
582            target = buffer_data->buffer + buffer_data->index;

This bit of code is for allocating small buffers at high intervals (I guess), where buffer_data is the preallocated pool of memory.
Problem is that if buffer_data->index after line 580 ends up near the end of the buffer in such a way that the requested chunk of memory can't fit within the pool's boundaries it will usually cause a segmentation fault (or the infamous general protection fault from XP days).

We need a check here to make sure that the requested chunk is placed correctly within the pool (buffer_data); here is my suggestion:
Code:
578        #define GET_BUFFER(target, buffer_data, len)                                                      \
579            /* hurray, no branching, modulus, or malloc! */                                               \
580            buffer_data->index = (buffer_data->index + buffer_data->prev_len) & (FOV_BUFFER_SIZE - 1);    \
581            if (buffer_data->index+len >= FOV_BUFFER_SIZE) {                                              \
582                buffer_data->index = 0;                                                                   \
583            }                                                                                             \
584            buffer_data->prev_len = len;                                                                  \
585            target = buffer_data->buffer + buffer_data->index;

Unfortunately we must also remove the 'hurray', because I cannot see how we may avoid branching here :(
Luckily most of the time the if will fail, and modern CPU's exploit this and are able to preload assembly as if the if will always fail. See http://en.wikipedia.org/wiki/Branch_predictor.
The bottom line is that this branch should have absolutely no impact on performance :)

I have fixed this in the AUR package with a homebrew patch, so please tell me if you implement this fix and I must update the package :)

BR Thomas

Page 1 of 1 All times are UTC
Powered by phpBB® Forum Software © phpBB Group
https://www.phpbb.com/