Opened 9 years ago

Closed 9 years ago

#1254 closed bug (fixed (in master))

Game crashes when examining an artifact

Reported by: ycombinator Owned by: d_m
Milestone: 3.2.0 Keywords: recall
Cc:

Description

git commit d1a64ee5

Game crashes when I try to examine "The Long Sword of Niond" (see attached savefile).

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fc6dae76750 (LWP 12264)]
0x00007fc6d9dd1c70 in strlen () from /lib/libc.so.6
(gdb) bt
#0  0x00007fc6d9dd1c70 in strlen () from /lib/libc.so.6
#1  0x00000000004c20e6 in my_strcpy (buf=0x7fffefcb5b80 ", ", 
    src=0x4 <Address 0x4 out of bounds>, bufsize=1024) at z-util.c:134
#2  0x00000000004bc9c7 in vstrnfmt (buf=0x7fc6cc0d1183 "turn", max=133, 
    fmt=0x4dd0bf "turn%s%s", vp=0x7fffefcb6490) at z-form.c:557
#3  0x00000000004c27c8 in textblock_vappend_c (tb=0x7fc6cc0c9ea8, 
    attr=1 '\001', fmt=0x4dd0bf "turn%s%s", vp=0x7fffefcb6490)
    at z-textblock.c:68
#4  0x00000000004c295e in textblock_append (tb=0x7fc6cc0c9ea8, 
    fmt=0x4dd0bf "turn%s%s") at z-textblock.c:92
#5  0x000000000045f7fe in describe_digger (tb=0x7fc6cc0c9ea8, 
    o_ptr=0x7fc6cc0a37d0, mode=OINFO_SUBJ) at object/obj-info.c:913
#6  0x00000000004609a0 in object_info_out (o_ptr=0x7fc6cc0a37d0, 
    mode=OINFO_SUBJ) at object/obj-info.c:1304
#7  0x00000000004609ff in object_info (o_ptr=0x7fc6cc0a37d0, mode=OINFO_SUBJ)
    at object/obj-info.c:1325
#8  0x000000000041728c in textui_obj_examine (o_ptr=0x7fc6cc0a37d0, item=19)
    at cmd-obj.c:243
#9  0x000000000040f293 in textui_process_key (c=73 'I') at cmd0.c:704
#10 0x000000000040f34f in textui_process_command (no_request=false)
    at cmd0.c:743
#11 0x00000000004b9762 in textui_get_cmd (context=CMD_GAME, wait=true)
    at xtra3.c:1804
#12 0x00000000004c308c in default_get_cmd (context=CMD_GAME, wait=true)
    at main.c:227
#13 0x00000000004263b7 in cmd_get (c=CMD_GAME, cmd=0x7fffefcb75b0, wait=true)
    at game-cmd.c:235
#14 0x0000000000426a15 in process_command (ctx=CMD_GAME, no_request=false)
    at game-cmd.c:389
#15 0x000000000041af05 in process_player () at dungeon.c:1088
#16 0x000000000041b892 in dungeon () at dungeon.c:1518
#17 0x000000000041bec4 in play_game () at dungeon.c:1845
#18 0x00000000004c34d9 in main (argc=1, argv=0x7fffefcb77a8) at main.c:464

Attachments (1)

1000.Rooslan (57.9 KB) - added by ycombinator 9 years ago.
save file

Download all attachments as: .zip

Change History (11)

Changed 9 years ago by ycombinator

save file

comment:1 Changed 9 years ago by magnate

I cannot reproduce this: the savefile works fine for me - I can inspect the longsword (and other items) with no problems.

comment:2 Changed 9 years ago by magnate

  • Keywords recall added
  • Milestone changed from Triage to 3.2.0
  • Resolution set to worksforme
  • Status changed from new to closed

Fizzix can't reproduce this either - closing until someone else reports it.

comment:3 Changed 9 years ago by ycombinator

It works fine on 32-bit systems, but crashes on 64-bit Linux. I've tried Ubuntu 9.04, Ubuntu 10.10 and Debian Squeeze, it's reproducible on all of them. Looks like a portability bug to me. I'm looking into it now, but my gdb-fu may be too weak to deal with apparent va_list corruption. It crashes on the third iteration of loop in obj-info.c:881-915.

comment:4 Changed 9 years ago by d_m

  • Resolution worksforme deleted
  • Status changed from closed to reopened

I will take a look at this... I think there is some unhealthy int/pointer conflation going on which might be responsible.

comment:5 Changed 9 years ago by d_m

  • Owner set to d_m
  • Status changed from reopened to assigned

comment:6 Changed 9 years ago by ycombinator

Got it. Unlike i386, va_list is a reference type on amd64 architecture and cannot be safely reused. The loop in textblock_vappend_c caused the problem. One should use va_copy each time before passing an argument list to vstrnfmt. Here is the patch for this particular problem, but someone should carefully review all uses of varargs throughout the code.

--- a/src/z-textblock.c
+++ b/src/z-textblock.c
@@ -60,12 +60,16 @@ static void textblock_vappend_c(textblock *tb, byte attr, const char *fmt,
 {
        while (1)
        {
+               va_list args;
+
                size_t len;
 
                size_t remaining = tb->size - tb->strlen;
                char *fmt_start = tb->text + tb->strlen;
 
-               len = vstrnfmt(fmt_start, remaining, fmt, vp);
+               va_copy(args, vp);
+               len = vstrnfmt(fmt_start, remaining, fmt, args);
+               va_end(args);
                if (len < remaining - 1)
                {
                        byte *attr_start = tb->attrs + (tb->strlen * sizeof *tb->attrs);
Last edited 9 years ago by ycombinator (previous) (diff)

comment:7 Changed 9 years ago by d_m

Great! noz (pnd10) and I are auditing the uses of va_list. Thanks so much for diagnosing this for us.

comment:8 Changed 9 years ago by ycombinator

BTW, va_copy is a C99 thing and thus can pose another portability problem. What compiler is used to build Angband for Windows? va_copy seems to be missing from MSVC at least.

Last edited 9 years ago by ycombinator (previous) (diff)

comment:9 Changed 9 years ago by noz

Fixed in [9cb8549], but not tested compiling for Windows...

comment:10 Changed 9 years ago by d_m

  • Resolution set to fixed
  • Status changed from assigned to closed

We're going to use a VA_COPY macro which will be a noop in MSVC. This might be bad for 64-bit builds on MSVC, but since we're using mingw to do the official build I'm less worried.

The macro is in staging and should be in master soon. The basic fix is already in master.

Note: See TracTickets for help on using tickets.