ToME: the Tales of Maj'Eyal

Everything about ToME
It is currently Wed Apr 26, 2017 2:03 am

All times are UTC




Post new topic Reply to topic  [ 1 post ] 
Author Message
 Post subject: FOV memory issue
PostPosted: Mon Aug 12, 2013 12:21 pm 
Offline

Joined: Mon Aug 12, 2013 10:47 am
Posts: 1
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


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 1 post ] 

All times are UTC


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group