Crash in linux sdl port when disabling fulscreen in map view

AndreyB · July 9, 2017, 18:16

Fedora 25
Angband 4.1.0 (sdl frontend)
Angband crashes when switching out of full screen wile in map view.
To reproduce:

Start a new game
Set font to 16x16
Click "options" button in the title bar
Select any graphical tile set
Set tile tile width and tile height to 2
Enable fullsreen
Press ok
Enter map view
Disable full screen
Game crashes

Additional info

Without setting tile width and tile height to 2 the game doesn't crash
Sometimes the game hangs instead of crashing
Display resolution: 1920x1080
I'm able to reproduce the crash reliably
The game sometimes crashes when enabling fullscreen in map view too

AndreyB · July 19, 2017, 10:00

To aid debugging I ran valgrind for you. Here is the part of it's output that seams relevant:

Code:

==4194== Invalid read of size 4
==4194==    at 0x4E653CD: SDL_FreeSurface (in /usr/lib64/libSDL-1.2.so.0.11.4)
==4194==    by 0x4EFD7E: init_windows (main-sdl.c:3402)
==4194==    by 0x4EE136: sdl_HandleEvent (main-sdl.c:2524)
==4194==    by 0x4EE3D7: Term_xtra_sdl_event (main-sdl.c:2615)
==4194==    by 0x4EE63A: Term_xtra_sdl (main-sdl.c:2719)
==4194==    by 0x4D6781: Term_xtra (ui-term.c:433)
==4194==    by 0x4D9806: Term_inkey (ui-term.c:2035)
==4194==    by 0x4B20E1: inkey_aux (ui-input.c:78)
==4194==    by 0x4B237F: inkey_ex (ui-input.c:254)
==4194==    by 0x4B2424: anykey (ui-input.c:289)
==4194==    by 0x4BCA6F: do_cmd_view_map (ui-map.c:794)
==4194==    by 0x4EF50D: Term_view_map_sdl (main-sdl.c:3144)
==4194==  Address 0x7477e84 is 84 bytes inside a block of size 88 free'd
==4194==    at 0x4C2ED4A: free (vg_replace_malloc.c:530)
==4194==    by 0x4EFD4C: init_windows (main-sdl.c:3398)
==4194==    by 0x4EE136: sdl_HandleEvent (main-sdl.c:2524)
==4194==    by 0x4EE3D7: Term_xtra_sdl_event (main-sdl.c:2615)
==4194==    by 0x4EE63A: Term_xtra_sdl (main-sdl.c:2719)
==4194==    by 0x4D6781: Term_xtra (ui-term.c:433)
==4194==    by 0x4D9806: Term_inkey (ui-term.c:2035)
==4194==    by 0x4B20E1: inkey_aux (ui-input.c:78)
==4194==    by 0x4B237F: inkey_ex (ui-input.c:254)
==4194==    by 0x4B2424: anykey (ui-input.c:289)
==4194==    by 0x4BCA6F: do_cmd_view_map (ui-map.c:794)
==4194==    by 0x4EF50D: Term_view_map_sdl (main-sdl.c:3144)
==4194==  Block was alloc'd at
==4194==    at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
==4194==    by 0x4E6553B: SDL_CreateRGBSurface (in /usr/lib64/libSDL-1.2.so.0.11.4)
==4194==    by 0x4EEE48: sdl_BuildTileset (main-sdl.c:2995)
==4194==    by 0x4EEFB4: Term_pict_sdl (main-sdl.c:3045)
==4194==    by 0x4D7741: Term_fresh_row_both (ui-term.c:849)
==4194==    by 0x4D834A: Term_fresh (ui-term.c:1271)
==4194==    by 0x4D9003: Term_redraw (ui-term.c:1732)
==4194==    by 0x4EC3A7: ResizeWin (main-sdl.c:1814)
==4194==    by 0x4EFDAF: init_windows (main-sdl.c:3407)
==4194==    by 0x4EE136: sdl_HandleEvent (main-sdl.c:2524)
==4194==    by 0x4EE3D7: Term_xtra_sdl_event (main-sdl.c:2615)
==4194==    by 0x4EE63A: Term_xtra_sdl (main-sdl.c:2719)

There are a bunch of messages almost identical to this one. This is the first of them.
I ran valgrind 3 times and the output seams consistent. Attached all 3 complete outputs.
Comments:
The first few messages are printed during the initialization of the game and are probably false positives from the optimized sdl. The messages printed after the bug is triggered are below the (probably unrelated) error message about a missing sound.

AndreyB · July 19, 2017, 11:06

Sometimes two code snippets can tell more than a thousand words.

Code:

[user@computer src]$ cat -n main-sdl.c | sed -n '3380,3419p'
  3380	/**
  3381	 * Create the windows
  3382	 * Called sometime after load_prefs()
  3383	 */
  3384	static void init_windows(void)
  3385	{
  3386		int i;
  3387	
  3388		for (i = 0; i < ANGBAND_TERM_MAX; i++) {
  3389			term_window *win = &windows[i];
  3390			
  3391			/* Only bother with visible windows */
  3392			if (win->visible) {
  3393				/* Don't crowd out the status bar... */
  3394				if (win->top < StatusHeight) win->top = StatusHeight;
  3395	
  3396				/* Invalidate the gfx surface */
  3397				if (win->tiles) {
  3398					SDL_FreeSurface(win->tiles);
  3399					win->tiles = NULL;
  3400				}
  3401				if (win->onebyone) {
  3402					SDL_FreeSurface(win->onebyone);
  3403					win->onebyone = NULL;
  3404				}
  3405	
  3406				/* This will set up the window correctly */
  3407				ResizeWin(win, win->width, win->height);
  3408			} else {
  3409				/* Doesn't exist */
  3410				angband_term[i] = NULL;
  3411			}
  3412			
  3413			/* Term 0 is at the top */
  3414			Zorder[i] = ANGBAND_TERM_MAX - i - 1;
  3415		}
  3416	
  3417		/* Good to go... */
  3418		Term_activate(term_screen);
  3419	}

Code:

[user@computer src]$ grep -B17 -A14 -rne 'win->tiles = win->onebyone'
main-sdl.c-3123-static void Term_view_map_sdl(term *t)
main-sdl.c-3124-{
main-sdl.c-3125-	SDL_Surface *fulltiles;
main-sdl.c-3126-	/* Get the right window */
main-sdl.c-3127-	term_window *win = (term_window*)(t->data);
main-sdl.c-3128-
main-sdl.c-3129-	/* First time a pict is requested we load the tileset in */
main-sdl.c-3130-	if (!win->tiles)
main-sdl.c-3131-		sdl_BuildTileset(win);
main-sdl.c-3132-
main-sdl.c-3133-	/* Override large tiles with small ones for the map view */
main-sdl.c-3134-	if (win->onebyone) {
main-sdl.c-3135-		/* Save screen so we can load it again after the tile image
main-sdl.c-3136-		 * is restored */
main-sdl.c-3137-		screen_save();
main-sdl.c-3138-
main-sdl.c-3139-		fulltiles = win->tiles;
main-sdl.c:3140:		win->tiles = win->onebyone;
main-sdl.c-3141-	}
main-sdl.c-3142-
main-sdl.c-3143-	t->view_map_hook = NULL;
main-sdl.c-3144-	do_cmd_view_map();
main-sdl.c-3145-	t->view_map_hook = Term_view_map_sdl;
main-sdl.c-3146-
main-sdl.c-3147-	if (win->onebyone) {
main-sdl.c-3148-		win->tiles = fulltiles;
main-sdl.c-3149-
main-sdl.c-3150-		/* Load screen with the correct tiles - the screen load in the
main-sdl.c-3151-		 * view map command was still using the image with small tiles */
main-sdl.c-3152-		screen_load();
main-sdl.c-3153-	}
main-sdl.c-3154-}
[user@computer src]$

I wonder how many times in my career I will make the same bug.
Edit: valgrind is awesome. It's the biggest timesaver people invented since the invention of the wheel.

AnonymousHero · July 19, 2017, 19:44

Quote:

Originally Posted by AndreyB

Sometimes two code snippets can tell more than a thousand words.
I wonder how many times in my career I will make the same bug.
Edit: valgrind is awesome. It's the biggest timesaver people invented since the invention of the wheel.

UBSAN/ASAN/etc. are even better and much faster -- so fast that you can just run with it full-time without getting extremely annoyed. For example, they'll actually print the problematic source code (assuming debug info) with a little context. Do

Code:

SANITIZE_FLAGS="-fsanitize=undefined -fsanitize=address" ./configure [...]

when configuring. (I *think* I added the support in a place where you have to set it while configuring, it might also be sufficient to just add the SANITIZE_FLAGS bit before "make" on the command line.)

AndreyB · July 20, 2017, 04:49

Quote:

Originally Posted by AnonymousHero

UBSAN/ASAN/etc. are even better and much faster -- so fast that you can just run with it full-time without getting extremely annoyed. For example, they'll actually print the problematic source code (assuming debug info) with a little context. Do

Code:

SANITIZE_FLAGS="-fsanitize=undefined -fsanitize=address" ./configure [...]

when configuring. (I *think* I added the support in a place where you have to set it while configuring, it might also be sufficient to just add the SANITIZE_FLAGS bit before "make" on the command line.)

At first this didn't seam to work but then I ran

Code:

make clean

and (after installing two libraries) it compiled. The output from triggering the bug is below.

Code:

z-virt.c:55:2: runtime error: null pointer passed as argument 1, which is declared to never be null
=================================================================
==1837==ERROR: AddressSanitizer: attempting double-free on 0x7f5f1935f800 in thread T0:
    #0 0x7f5f47f4db00 in free (/lib64/libasan.so.3+0xc6b00)
    #1 0x7f5f47c1548c in SDL_FreeSurface (/lib64/libSDL-1.2.so.0+0x2b48c)
    #2 0x78611c in init_windows /home/user/angband/src/main-sdl.c:3402
    #3 0x77d82f in sdl_HandleEvent /home/user/angband/src/main-sdl.c:2524
    #4 0x77e6b5 in Term_xtra_sdl_event /home/user/angband/src/main-sdl.c:2615
    #5 0x77f3db in Term_xtra_sdl /home/user/angband/src/main-sdl.c:2719
    #6 0x722166 in Term_xtra /home/user/angband/src/ui-term.c:433
    #7 0x730c03 in Term_inkey /home/user/angband/src/ui-term.c:2035
    #8 0x69c0b1 in inkey_aux /home/user/angband/src/ui-input.c:78
    #9 0x69c8d1 in inkey_ex /home/user/angband/src/ui-input.c:254
    #10 0x69cae1 in anykey /home/user/angband/src/ui-input.c:289
    #11 0x6c426c in do_cmd_view_map /home/user/angband/src/ui-map.c:794
    #12 0x784346 in Term_view_map_sdl /home/user/angband/src/main-sdl.c:3144
    #13 0x6c401a in do_cmd_view_map /home/user/angband/src/ui-map.c:763
    #14 0x697c89 in textui_process_command /home/user/angband/src/ui-game.c:313
    #15 0x697e02 in textui_get_cmd /home/user/angband/src/ui-game.c:325
    #16 0x698ccf in play_game /home/user/angband/src/ui-game.c:434
    #17 0x7679c4 in main /home/user/angband/src/main.c:524
    #18 0x7f5f45f91400 in __libc_start_main (/lib64/libc.so.6+0x20400)
    #19 0x403769 in _start (/home/user/angband/src/angband+0x403769)

0x7f5f1935f800 is located 0 bytes inside of 4194304-byte region [0x7f5f1935f800,0x7f5f1975f800)
freed by thread T0 here:
    #0 0x7f5f47f4db00 in free (/lib64/libasan.so.3+0xc6b00)
    #1 0x7f5f47c1548c in SDL_FreeSurface (/lib64/libSDL-1.2.so.0+0x2b48c)

previously allocated by thread T0 here:
    #0 0x7f5f47f4e020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7f5f47c15684 in SDL_CreateRGBSurface (/lib64/libSDL-1.2.so.0+0x2b684)

SUMMARY: AddressSanitizer: double-free (/lib64/libasan.so.3+0xc6b00) in free
==1837==ABORTING

AndreyB · July 20, 2017, 04:59

Anyway. I showed what (I think) was causing the crash. I don't know the codebase well enough to suggest a correct way to fix this. The only fix I came up with will probably cause weird graphical glitches, will definitely leak memory and might even cause more crashes. I did all I could for this bug.

AndreyB · July 20, 2017, 05:08

Quote:

Originally Posted by AnonymousHero

UBSAN/ASAN/etc. are even better and much faster -- so fast that you can just run with it full-time without getting extremely annoyed. For example, they'll actually print the problematic source code (assuming debug info) with a little context. Do

Code:

SANITIZE_FLAGS="-fsanitize=undefined -fsanitize=address" ./configure [...]

when configuring. (I *think* I added the support in a place where you have to set it while configuring, it might also be sufficient to just add the SANITIZE_FLAGS bit before "make" on the command line.)

Also on normal exit your thing reports a bunch of leaks. Is this normal?

Code:

z-virt.c:55:2: runtime error: null pointer passed as argument 1, which is declared to never be null

=================================================================
==2065==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 929760 byte(s) in 30 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a93158cc in TTF_OpenFontIndexRW (/lib64/libSDL_ttf-2.0.so.0+0x28cc)

Direct leak of 450 byte(s) in 36 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7656e1 in mem_alloc /home/user/angband/src/z-virt.c:41
    #2 0x765991 in string_make /home/user/angband/src/z-virt.c:98
    #3 0x4bfe8a in parse_trap_visibility /home/user/angband/src/init.c:810
    #4 0x5e0a6a in parser_parse /home/user/angband/src/parser.c:329
    #5 0x434624 in parse_file /home/user/angband/src/datafile.c:105
    #6 0x434437 in parse_file_quit_not_found /home/user/angband/src/datafile.c:70
    #7 0x4c19e4 in run_parse_trap /home/user/angband/src/init.c:1171
    #8 0x434379 in run_parser /home/user/angband/src/datafile.c:54
    #9 0x4ccda4 in init_arrays /home/user/angband/src/init.c:2816
    #10 0x4cd151 in init_angband /home/user/angband/src/init.c:2892
    #11 0x76792e in main /home/user/angband/src/main.c:517
    #12 0x7fd6a7cfd400 in __libc_start_main (/lib64/libc.so.6+0x20400)

Direct leak of 144 byte(s) in 8 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7656e1 in mem_alloc /home/user/angband/src/z-virt.c:41
    #2 0x765991 in string_make /home/user/angband/src/z-virt.c:98
    #3 0x776eeb in load_prefs /home/user/angband/src/main-sdl.c:1852
    #4 0x787930 in init_sdl /home/user/angband/src/main-sdl.c:3596
    #5 0x7677e8 in main /home/user/angband/src/main.c:479
    #6 0x7fd6a7cfd400 in __libc_start_main (/lib64/libc.so.6+0x20400)

Direct leak of 88 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7fd6a998153b in SDL_CreateRGBSurface (/lib64/libSDL-1.2.so.0+0x2b53b)

Direct leak of 18 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7656e1 in mem_alloc /home/user/angband/src/z-virt.c:41
    #2 0x765991 in string_make /home/user/angband/src/z-virt.c:98
    #3 0x56d808 in parse_artifact_curse /home/user/angband/src/obj-init.c:2656
    #4 0x5e0a6a in parser_parse /home/user/angband/src/parser.c:329
    #5 0x434624 in parse_file /home/user/angband/src/datafile.c:105
    #6 0x434437 in parse_file_quit_not_found /home/user/angband/src/datafile.c:70
    #7 0x56ddd7 in run_parse_artifact /home/user/angband/src/obj-init.c:2692
    #8 0x434379 in run_parser /home/user/angband/src/datafile.c:54
    #9 0x4ccda4 in init_arrays /home/user/angband/src/init.c:2816
    #10 0x4cd151 in init_angband /home/user/angband/src/init.c:2892
    #11 0x76792e in main /home/user/angband/src/main.c:517
    #12 0x7fd6a7cfd400 in __libc_start_main (/lib64/libc.so.6+0x20400)

Direct leak of 16 byte(s) in 2 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7fd6a7d69cc9 in __GI___strdup (/lib64/libc.so.6+0x8ccc9)
    #2 0x2f  (<unknown module>)

Indirect leak of 67108864 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a9981684 in SDL_CreateRGBSurface (/lib64/libSDL-1.2.so.0+0x2b684)

Indirect leak of 104736 byte(s) in 654 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7fd6a9315295  (/lib64/libSDL_ttf-2.0.so.0+0x2295)

Indirect leak of 2400 byte(s) in 30 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a93158f5 in TTF_OpenFontIndexRW (/lib64/libSDL_ttf-2.0.so.0+0x28f5)

Indirect leak of 1920 byte(s) in 30 object(s) allocated from:
    #0 0x7fd6a9cb9e60 in malloc (/lib64/libasan.so.3+0xc6e60)
    #1 0x7fd6a9965d4d in SDL_AllocRW (/lib64/libSDL-1.2.so.0+0xfd4d)

Indirect leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a997fdaf  (/lib64/libSDL-1.2.so.0+0x29daf)

Indirect leak of 48 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a997ef1c  (/lib64/libSDL-1.2.so.0+0x28f1c)

Indirect leak of 16 byte(s) in 1 object(s) allocated from:
    #0 0x7fd6a9cba020 in calloc (/lib64/libasan.so.3+0xc7020)
    #1 0x7fd6a997fdc6  (/lib64/libSDL-1.2.so.0+0x29dc6)

SUMMARY: AddressSanitizer: 68148524 byte(s) leaked in 796 allocation(s).

AndreyB · July 20, 2017, 12:12

AnonymousHero, thanks BTW.

AnonymousHero · July 20, 2017, 17:18

(Without looking into your messages in detail.)

Yes, the leaks are probably "normal" -- Angband allocates a large amount of structures once-per-run and doesn't always deallocate before exiting. As long as the structures are "static" and don't accumulate "per turn" (or whatever), it's fine since the OS will free everything when the program exits.

EDIT: Hey, nice shout out in your signature

. I take it that it was sufficient to just set the flag before running "make"? (Obviously, you may need "clean" just because "make" doesn't track compile flags.) Somebody (hint, hint, it'd be a good first GitHub PR!) should really add a little snippet to the "compiling.txt" file

.

AndreyB · July 20, 2017, 19:34

Sorry, my first PR is already submitted https://github.com/angband/angband/pull/483. I was too lazy and ran both configure and make with the flag just in case (a la universal install script so I don't know if applying the flag only to make would be enough.
Edit: just checked and yes doing make with flags is enough.

Thread Tools
Show Printable Version Email this Page
Display Modes
Linear Mode Switch to Hybrid Mode Switch to Threaded Mode

Similar Threads
Thread	Thread Starter	Forum	Replies	Last Post
changing config.txt for sangband SDl port under ubuntu linux	Malak Darkhunter	Variants	1	December 2, 2014 14:28
Building Sangband 101129 SDL on Linux	Mikko Lehtinen	Variants	6	September 30, 2013 23:08
Comparison View	OOD Town drunk	Vanilla	2	May 19, 2013 17:48
Disabling Graphics in Vulture's Claw	kingvictory2003	Idle chatter	2	November 15, 2010 09:13
Unangband: disabling fog of war	d_m	Variants	6	December 31, 2009 06:27

July 20, 2017, 04:59	#6
AndreyB Scout Join Date: May 2017 Location: Russia Posts: 26	Anyway. I showed what (I think) was causing the crash. I don't know the codebase well enough to suggest a correct way to fix this. The only fix I came up with will probably cause weird graphical glitches, will definitely leak memory and might even cause more crashes. I did all I could for this bug. __________________ I like valgrind. it's the biggest timesaver people invented since the invention of the wheel. UBSan&ASan are even better (no noticeable initialization delay on my machine). I'm now playing with it on. WARNING: with UBSan&ASan panic saves don't seam to work.

July 20, 2017, 12:12	#8
AndreyB Scout Join Date: May 2017 Location: Russia Posts: 26	AnonymousHero, thanks BTW. __________________ I like valgrind. it's the biggest timesaver people invented since the invention of the wheel. UBSan&ASan are even better (no noticeable initialization delay on my machine). I'm now playing with it on. WARNING: with UBSan&ASan panic saves don't seam to work.

July 20, 2017, 17:18	#9
AnonymousHero Veteran Join Date: Jun 2007 Posts: 1,391	(Without looking into your messages in detail.) Yes, the leaks are probably "normal" -- Angband allocates a large amount of structures once-per-run and doesn't always deallocate before exiting. As long as the structures are "static" and don't accumulate "per turn" (or whatever), it's fine since the OS will free everything when the program exits. EDIT: Hey, nice shout out in your signature . I take it that it was sufficient to just set the flag before running "make"? (Obviously, you may need "clean" just because "make" doesn't track compile flags.) Somebody (hint, hint, it'd be a good first GitHub PR!) should really add a little snippet to the "compiling.txt" file . Last edited by AnonymousHero; July 20, 2017 at 17:27.

July 20, 2017, 19:34	#10
AndreyB Scout Join Date: May 2017 Location: Russia Posts: 26	Sorry, my first PR is already submitted https://github.com/angband/angband/pull/483. I was too lazy and ran both configure and make with the flag just in case (a la universal install script so I don't know if applying the flag only to make would be enough. Edit: just checked and yes doing make with flags is enough. __________________ I like valgrind. it's the biggest timesaver people invented since the invention of the wheel. UBSan&ASan are even better (no noticeable initialization delay on my machine). I'm now playing with it on. WARNING: with UBSan&ASan panic saves don't seam to work. Last edited by AndreyB; July 20, 2017 at 19:44.

Currently Active Users Viewing This Thread: 1 (0 members and 1 guests)