ToME: the Tales of Maj'Eyal

Everything about ToME
It is currently Thu Feb 23, 2017 11:34 am

All times are UTC




Post new topic Reply to topic  [ 321 posts ]  Go to page Previous  1 ... 17, 18, 19, 20, 21, 22  Next
Author Message
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 6:19 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Yeah, I figured that out, sorry. :oops: Looks like something you deprecated.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 6:22 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 476
Lord Estraven wrote:
Yeah, I figured that out, sorry. :oops: Looks like something you deprecated.


No worries. Btw, in my experience sometimes "deliberately crash the program" is actually the best thing you can do... but it's not something I "reach for" often :).

EDIT: It's basically always better to "abort" rather than run into the dreaded Undefined Behavior.


Last edited by AnonymousHero on Sat Oct 24, 2015 6:24 pm, edited 1 time in total.

Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 6:24 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Good point. :oops:

Meanwhile, Linux SSP is actually getting invoked no matter what one enters on the Automizer prompt. Hmm.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 6:54 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
The buffer overrun here, wherever it is, seems to have been present at least since the introduction of the new jansson-based Automizer. Maybe earlier.

Edit: right, git bisect. D'oh!

Edit: yeah, 2.3.9-ah works. Let's start bisecting.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 24, 2015 7:28 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Alright, the first bad commit is 6add91e17080e06cae938a31c53c94e59c7f0bfb.

This commit basically contains the entirety of the jansson-based automizer code, almost as it appears today. :(

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sun Oct 25, 2015 7:56 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
So, working on the negation rule crash, this is interesting:

Code:
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/libthread_db.so.1".
tome: /home/proteus/Projects/tome2/src/squeltch.c:2083: display_condition: Assertion `condition != ((void *)0)' failed.

Program received signal SIGABRT, Aborted.
0xb70c59dc in raise () from /lib/libc.so.6
(gdb) bt
#0  0xb70c59dc in raise () from /lib/libc.so.6
#1  0xb70c71f3 in abort () from /lib/libc.so.6
#2  0xb70beac6 in __assert_fail_base () from /lib/libc.so.6
#3  0xb70beb77 in __assert_fail () from /lib/libc.so.6
#4  0x080d44cb in display_condition ()
#5  0x080d4a9f in display_condition ()
#6  0x080d52f9 in display_rule ()
#7  0x080d5cbc in do_cmd_automatizer ()
#8  0x0816ee06 in do_cmd_options ()
#9  0x0809c9f2 in process_command ()
#10 0x0809d25e in process_player ()
#11 0x0809ddd5 in dungeon ()
#12 0x0809e7f2 in play_game ()
#13 0x081de7a0 in main ()
(gdb)


The assert that's breaking looks like this

Code:
static void display_condition(condition_type *condition)
{
        byte bcol = TERM_L_GREEN;
        byte ecol = TERM_GREEN;
        int i;
               
        assert(condition != NULL);


and this is the caller:

Code:
static void display_rule(arule_type *rule)
{
        cptr action_s;
        int hgt, wid;

        action_s = action_to_string(rule->action);


... but here is how it calls display_condition():

Code:
        /* Write out the condition */
        if (rule->condition != NULL)
        {
                display_condition(rule->condition);
        }


... It's checking that rule->condition is not NULL. And only displaying it if it's not NULL. But it's NULL anyway.

:?

Maybe something to do with what the NULL macro actually represents on different platforms?

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sun Oct 25, 2015 8:09 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
I added another assert before the 'if' statement, and it's obviously a null pointer before the 'if' is reached. Hmm.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sun Oct 25, 2015 8:18 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Wait wait wait! display_condition is recursive, so it must be nullifying the pointer somewhere. Just a minute.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sun Oct 25, 2015 8:25 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Okay, I think I got this. The M_NOT case in display_condition() is invoking the function recursively on condition->subcondition, which is explicitly NULL. (Line 2219 in squeltch.c.) Let's see what happens when we check for that being NULL too.

Edit: That works. I'll look over it for further invocations without NULL checking, then request a merge. (With apologies for the last merge request.)

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Mon Oct 26, 2015 1:08 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Okay, so for the SSP crash, the problem is this

Code:
static int create_new_rule()
{
        action_type action;
        char name[20] = { '\0' };


The name[] buffer is too small for some reason. Not sure exactly what's happening here though - "No name" is only 8 characters (including the terminating NUL).

Setting the buffer size to 100 works (obviously?), but that doesn't tell me why things are breaking, and might break again in the future.

I also tried using snprintf. Oddly, that resulted in exactly the same crash.

Weird.

Edit: looks like the buffer size must be > 80 characters to not cause an overflow. Hmm.

Edit 2: ah got it, it's an off-by-one error.

Code:
        if (!input_box("Name?", hgt / 2, wid / 2, name, sizeof(name)+1))


That extra '+1' breaks things.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Mon Oct 26, 2015 1:38 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Okay - all three of those are in my automizer-fix branch, and I've filed a merge request. :)

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 31, 2015 1:24 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
Hey @AnonymousHero, did you change something in the cpp branch before merging it? Because now the compile fails on cmd1.cc.

Code:
[  3%] Building CXX object src/CMakeFiles/game.dir/cmd1.cc.o
/home/proteus/Games/tome2/src/cmd1.cc: In function ‘bool_ pattern_seq(int, int, int, int)’:
/home/proteus/Games/tome2/src/cmd1.cc:2848:30: error: cannot pass objects of non-trivially-copyable type ‘struct cave_type’ through ‘...’
                 cave[c_y][c_x]);
                              ^
make[2]: *** [src/CMakeFiles/game.dir/cmd1.cc.o] Error 1
make[1]: *** [src/CMakeFiles/game.dir/all] Error 2
make: *** [all] Error 2


I will see if I can fix this. But looking at the error, I suspect it will require a function prototype to be changed.

_________________
"These aren't the hobbits you're looking for."


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 31, 2015 1:26 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 476
Lord Estraven wrote:
Hey @AnonymousHero, did you change something in the cpp branch before merging it? Because now the compile fails on cmd1.cc.

Code:
[  3%] Building CXX object src/CMakeFiles/game.dir/cmd1.cc.o
/home/proteus/Games/tome2/src/cmd1.cc: In function ‘bool_ pattern_seq(int, int, int, int)’:
/home/proteus/Games/tome2/src/cmd1.cc:2848:30: error: cannot pass objects of non-trivially-copyable type ‘struct cave_type’ through ‘...’
                 cave[c_y][c_x]);
                              ^
make[2]: *** [src/CMakeFiles/game.dir/cmd1.cc.o] Error 1
make[1]: *** [src/CMakeFiles/game.dir/all] Error 2
make: *** [all] Error 2


I will see if I can fix this. But looking at the error, I suspect it will require a function prototype to be changed.


I have a fix already, but I may have forgotten to push it -- will investigate ASAP. (You're compiling with Clang, right?)

I believe it should work to compile with gcc.


Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 31, 2015 1:28 pm 
Offline
Uruivellas

Joined: Tue Dec 13, 2005 12:35 am
Posts: 702
This was with GCC (4.8.2).

BTW this is stuff related to the Straight Road (i.e. Zangband Pattern). Straight Road vaults don't even exist any more. It can probably be removed.

Edit: whoops, and that line is for Wizard Mode anyway.

Edit 2: I just modified the printf invocation to not provide any coordinates (i.e. no %d).

Though, I'm gonna take a look at the vaults info file. I want to see if I can reintroduce Straight Road vaults.

_________________
"These aren't the hobbits you're looking for."


Last edited by Lord Estraven on Sat Oct 31, 2015 1:33 pm, edited 1 time in total.

Top
 Profile  
 
 Post subject: Re: ToME 2 maintenance
PostPosted: Sat Oct 31, 2015 1:30 pm 
Offline
Spiderkin

Joined: Sat Mar 18, 2006 12:48 pm
Posts: 476
Lord Estraven wrote:
This was with GCC (4.8.2).

BTW this is stuff related to the Straight Road (i.e. Zangband Pattern). Straight Road vaults don't even exist any more. It can probably be removed.


Oh, perhaps I got clang/gcc mixed up. Anyway, the fix is pushed now.


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 321 posts ]  Go to page Previous  1 ... 17, 18, 19, 20, 21, 22  Next

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