segfault when trying to print out memory information (-showmeminfo)

[ghost]

I noticed that D2X shows a memory block which left unfreed at the end of the game. So I wanted to trace this using -showmeminfo. Compiled the game with debug=1 and using the argument, the game segfaults upon exit.

bt:
Warning: MEM: 1 blocks were left allocated!

MEM_FREE_NOMALLOC: An attempt was made to free a ptr that wasn't
allocated with mem.h included.

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff63e1a2d in __memcpy_avx_unaligned () from /usr/lib/libc.so.6
(gdb) bt

0 0x00007ffff63e1a2d in __memcpy_avx_unaligned () from /usr/lib/libc.so.6

1 0x00007ffff7bb771d in PHYSFS_write () from /usr/lib/libphysfs.so.1

2 0x00000000004485f4 in PHYSFSX_check_write (C=, S=1, v=0x7fffffffdfa0 "21:05:39 ", file=0x1465a00) at common/include/physfsx.h:122

3 PHYSFSX_puts (len=, s=0x7fffffffdfa0 "21:05:39 ", file=0x1465a00) at common/include/physfsx.h:205

4 PHYSFSX_printf (file=0x1465a00, format=format@entry=0x506200 "%02i:%02i:%02i ") at common/include/physfsx.h:354

5 0x0000000000448649 in con_print_file (

buffer=buffer@entry=0x51a2f0 "\nMEM_FREE_NOMALLOC: An attempt was made to free a ptr that wasn't\nallocated with mem.h included.")
at similar/main/console.cpp:98

6 0x00000000004490c2 in con_puts (priority=priority@entry=-3,

buffer=buffer@entry=0x51a2f0 "\nMEM_FREE_NOMALLOC: An attempt was made to free a ptr that wasn't\nallocated with mem.h included.", len=len@entry=96)
at similar/main/console.cpp:125

7 0x00000000004e2b15 in con_puts_literal<97ul> (str=..., level=-3) at common/include/console.h:51

8 mem_free (buffer=0x1465760) at similar/mem/mem.cpp:246

9 0x00007ffff62f0e78 in __run_exit_handlers () from /usr/lib/libc.so.6

10 0x00007ffff62f0ec5 in exit () from /usr/lib/libc.so.6

11 0x00007ffff62db797 in __libc_start_main () from /usr/lib/libc.so.6

12 0x000000000040a0b9 in _start ()

[vLKp]

The memory leak is a false-positive caused by running the sweep before global destructors finish. The console prompt added by btb is freed, but only after the sweep flags it as a leak.

The crash is strange. It looks like it would be from gamelog_fp being closed, but that is an automatic type that should reset to nullptr when it is closed. I fixed up the -fno-inline build to pursue this, but the crash does not happen when using -fno-inline.

[ghost]

I have to investigate this deeper. I checked with Valgrind and I found some messages that hint towards the issue may be related to PhysFS itself. Which is strange since PhysFS didn't update for a long time. Which then again may be the root of the issue - the package hasn't been recompiled since 2003 so it may have some dependency issues. I'll continue investigating and see what I can find.

[vLKp]

Valgrind says that the call to PHYSFS_write accessed memory freed by a call to PHYSFS_close. This looks like gamelog_fp was closed early, except that its contained value at the time of the crash is not nullptr, which it should be if the destructor has run. The non-nullptr value might happen if the compiler optimized away the store of nullptr on the basis that the object is going out of scope and any further access to it is undefined behaviour.

[vLKp]

I think the flow is:

1. main ends.
2. atexit handler mem_display_blocks runs.
3. mem_display_blocks prints that the console Prompt leaked. This is not true. The prompt is still allocated, but is not leaked because its destructor will soon run and free it.
4. mem_display_blocks forcibly frees the console Prompt and marks it as unallocated.
5. mem_display_blocks returns.
6. Destructor for global gamelog_fp runs. This calls PHYSFS_close on the file and sets the contained pointer to nullptr. In optimized builds, the store may be removed because the lifetime of gamelog_fp has ended, so any further access to it is undefined behaviour.
7. Destructor for global console Prompt runs and tries to free the memory. This is undefined behaviour because the memory was already forced free in (4).
8. mem_free does not recognize the memory freed by Prompt because it was already freed in (4). mem_free tries to con_printf a warning, which uses the now free gamelog_fp. Reading gamelog_fp is undefined behaviour. It may return the stored nullptr and then return because the file is closed or it may return the previous file pointer if the clear to nullptr was optimized out.
9. If the nullptr store was optimized out, the call to PHYSFS_write accesses freed memory.

Removing the force-free in mem_display_blocks prevents the crash for me, and seems like a good idea since mem_display_blocks runs so early in the shutdown process. It looks like it will often, if not always, incorrectly flag as leaked any memory managed by a global scope object with a destructor. If we keep the force-free call, even if we prevented the use-after-free of gamelog_fp, we would still have the problem of double-free calls when global destructors clean up memory that was force-freed by mem_display_blocks.

[ghost]

Checked this again and the segfault is gone for me. The game still shows one un-freed block but I think this is not critical:

MEM_LEAKAGE: Memory block has not been freed.
Block 'CLI_DEFAULT_PROMPT' created in common/main/cli.cpp, line 68.
Warning: MEM: 1 blocks were left allocated!

[vLKp]

As I said above, I think this is a bogus report caused by running the sweep before global destructors finish. Memory allocated by the d_malloc family should not be stored at global scope.