Debugging a 64-bit Segfault in an Old MUD Codebase

Published by albertw on

Once I got the MUD built and running, it didn’t take long to hit problems. In fact, the very first time I connected:

Connected to port 6715 on sionnach.
Segmentation fault (core dumped)

Good start.

1. Run it under GDB

First step: run the server under GDB so we can see where it crashes.

gdb --args ../bin/aberd -f

The –args lets us pass arguments (-f keeps the MUD in the foreground).

Then:

(gdb) run

Pretty quickly we hit:

Program received signal SIGSEGV, Segmentation fault.
check_send_msg (plx=111, a=-13872, t="The Thief has gone west.\n") at sendsys.c:172

I didn’t even need to connect this time, the NPC movement message alone triggered the crash!

2. Get the backtrace

First thing to do it get a backtrace:

(gdb) bt
#0  check_send_msg (plx=plx@entry=111, a=a@entry=-13872, t=t@entry=0x7fffffffd1f0 "The Thief has gone west.\n") at sendsys.c:172
#1  0x0000000000427318 in send_g_msg (destination=destination@entry=-613, func=func@entry=0x426e6b <check_send_msg>, arg=arg@entry=-13872, text=text@entry=0x7fffffffd1f0 "The Thief has gone west.\n")
    at sendsys.c:151
#2  0x0000000000427426 in send_msg (destination=-613, mode=mode@entry=0, min=<optimized out>, max=max@entry=100000, x1=x1@entry=111, x2=x2@entry=-32766, format=0x437c0b "%s has gone %s.\n") at sendsys.c:205
#3  0x000000000041a2ea in movemob (x=x@entry=111) at mobile.c:739
#4  0x000000000041a482 in consid_move (mon=mon@entry=111) at mobile.c:647
#5  0x000000000041a5af in move_mobiles () at mobile.c:527
#6  0x0000000000428462 in on_timer () at timing.c:538
#7  0x000000000041745a in main_loop (m_socket=m_socket@entry=0) at main.c:544
#8  0x0000000000418312 in xmain (fd=<optimized out>) at main.c:172
#9  main (argc=<optimized out>, argv=<optimized out>, ep=<optimized out>) at main.c:90
(gdb)

How we can see that the flow of the program. We have a main loop and within that we move mobiles (NPCs) around and when they move a message gets printed.

3. Look at the crashing function

gdb can display code inline (list) but I prefer to use a separate terminal. So lets look at check_send_msg

struct _send_msg_box {
  int mode;
  int min;
  int max;
  int x1;
  int x2;
};

char check_send_msg(int plx, int a, char t)
{
  struct _send_msg_box b = (struct _send_msg_box )a;
  
  if (test_rcv(plx,b->mode,b->min,b->max,b->x1,b->x2)) return t;
  return NULL;
}

We segfault just on that test_rcv line.

4. Inspect variables in GDB

What I typically do at this point is just print the arguments and see if something jumps out. We could look at the instruction pointer and see exactly where were crashing if we needed to, perhaps in another post.

(gdb) print plx
$3 = 111
(gdb) print b
$4 = (struct _send_msg_box *) 0xffffffffffffc9d0
(gdb) print b->mode
Cannot access memory at address 0xffffffffffffc9d0
(gdb) print b->min
Cannot access memory at address 0xffffffffffffc9d4
(gdb) print b->x2
Cannot access memory at address 0xffffffffffffc9e0
(gdb)

plx is probably fine, but those memory addresses look wrong, that’s an invalid memory address.

5. Where did b come from?

Back tot he code and we see :

struct _send_msg_box *b = (struct _send_msg_box *)a;

b is just a reinterpreted as a pointer. so if a is a valid address b should be valid.

(gdb) print a
$5 = -13872
(gdb)

oh. Looking at the function declaration we see :

char *check_send_msg(int plx, int a, char *t)

a is an integer. If it’s supposed to be a memory address it shouldn’t be an integer. You would get away with that on a 32 bit system but not modern 64 bit hardware. So it’s likely that somewhere we had a proper memory address but chopped it to an int somewhere.

6. Follow the value backwards

stepping back to where we call send_g_msg we see that we also have an address of -13872

(gdb) frame 1
#1  0x0000000000427318 in send_g_msg (destination=destination@entry=-613, func=func@entry=0x426e6b <check_send_msg>, arg=arg@entry=-13872, text=text@entry=0x7fffffffd1f0 "The Thief has gone west.\n")
    at sendsys.c:151
151		  (t = (func == NULL ? text : func(p, arg, text))) != NULL &&
(gdb) print arg
$8 = -13872
(gdb)

and the function declaration says int again:

void send_g_msg(int destination,                        /* Where to send to */
                char *func(int plx, int arg, char *t),  /* Test function */
                int arg,                             /* Argument to test */
                char *text)                          /* Text to send */
{

So lets step back again to send_msg:

(gdb) frame 2
#2  0x0000000000427426 in send_msg (destination=-613, mode=mode@entry=0, min=<optimized out>, max=max@entry=100000, x1=x1@entry=111, x2=x2@entry=-32766, format=0x437c0b "%s has gone %s.\n") at sendsys.c:205
205	  send_g_msg(destination,check_send_msg,(int)&b,bf);
(gdb) print b
$9 = {mode = 0, min = 0, max = 100000, x1 = 111, x2 = -32766}
(gdb) print (int)&b
$10 = -13872
(gdb) print &b
$11 = (struct _send_msg_box *) 0x7fffffffc9d0

7. Root cause

We took a valid pointer:

0x7fffffffc9d0

and did this:

(int)&b

On a 64-bit system:

  • pointers = 64 bits
  • int = 32 bits

So we truncate the pointer, keeping only the low 32 bits:

0x7fffffffc9d0 -> 0x0000c9d0 -> -13872

Later, it gets cast back to a pointer:

(struct _send_msg_box *)a

Which becomes:

0xffffffffffffc9d0

Then we dereference it:

b->mode

Segmentation fault

8. The Fix

The correct solution is to use a type that can safely hold a pointer. After a bit of reading I found that the correct way to do this is with intptr_t:

#include <stdint.h>
intptr_t arg;

Update the function signatures:

char *check_send_msg(int plx, intptr_t a, char *t)
{
  struct _send_msg_box *b = (struct _send_msg_box *)a;
  ...
}
void send_g_msg(..., intptr_t arg, ...)

And call it like:

send_g_msg(destination, check_send_msg, (intptr_t)&b, bf);

My changeset for this is at https://github.com/albertw/dirt/commit/94ec62c63e144920a04ca2e7e4290e04d0ee0319

9. Verify the fix

We can just run the mud and see that it doesn’t segfault, but a little better is to run it in gdb, set a breakpoint, and inspect that we have the right values now:

gdb) break check_send_msg<br>Breakpoint 1 at 0x426e6b: file sendsys.c, line 170.
(gdb) run
.
.
.
Breakpoint 1, check_send_msg (plx=plx@entry=111, a=a@entry=140737488341456, t=t@entry=0x7fffffffd1f0 "The Thief has gone west.\n") at sendsys.c:170<br>170 {<br>
(gdb) p a
$1 = 140737488341456
(gdb) p/x a
$2 = 0x7fffffffc9d0
(gdb) p *(struct _send_msg_box *)a<br>$3 = {mode = 0, min = 0, max = 100000, x1 = 114, x2 = -32766}

This is a classic example of why legacy C code that ‘worked fine’ on 32-bit systems can fail in subtle ways on modern architectures, and why types matter more than they seem.

Categories:computing

0 Comments