Bugs and complaints on current master

[AnonymousHero]

OK, I have this working now - the trick is to do
(snip)

Ah, excellent that you've got it working -- happy bug squishing! I was unaware of the "gcc-5" thing -- it must be a Debian/Ubuntu/Fedora thing? I'm on Arch Linux, FWIW.

[Nick]

OK, I have this working now - the trick is to do

CC=clang ./configure <configure switches>
SANITIZE_FLAGS="-fsanitize=address,undefined" make

and everything works.

As a side note, my default gcc is 4.8.5;

CC=gcc-5 ./configure <configure switches>
SANITIZE_FLAGS="-fsanitize=address,undefined" make

works pretty much exactly the same.

[AnonymousHero]

Just because I still have the terminal window open, here's one of the heap-buffer overflows:

=================================================================
==5429==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x619000058b90 at pc 0x7efef826f6f9 bp 0x7ffe89c4c490 sp 0x7ffe89c4bc38
READ of size 1036 at 0x619000058b90 thread T0
    #0 0x7efef826f6f8 in __interceptor_wcslen /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:590
    #1 0x748edb in vstrnfmt GAME_DIR/src/z-form.c:460
    #2 0x746a9d in file_vputf GAME_DIR/src/z-file.c:642
    #3 0x74697f in file_putf GAME_DIR/src/z-file.c:624
    #4 0x74f8ee in textblock_to_file GAME_DIR/src/z-textblock.c:339
    #5 0x553518 in object_info_spoil GAME_DIR/src/obj-info.c:1882
    #6 0x738345 in spoil_artifact GAME_DIR/src/wiz-spoil.c:411
    #7 0x73a69c in spoiler_menu_act GAME_DIR/src/wiz-spoil.c:676
    #8 0x6b48f9 in menu_action_handle GAME_DIR/src/ui-menu.c:102
    #9 0x6bb86b in menu_handle_action GAME_DIR/src/ui-menu.c:661
    #10 0x6bcf39 in menu_select GAME_DIR/src/ui-menu.c:797
    #11 0x73a7d8 in do_cmd_spoilers GAME_DIR/src/wiz-spoil.c:709
    #12 0x674208 in death_spoilers GAME_DIR/src/ui-death.c:331
    #13 0x6b48f9 in menu_action_handle GAME_DIR/src/ui-menu.c:102
    #14 0x6bb86b in menu_handle_action GAME_DIR/src/ui-menu.c:661
    #15 0x6bcf39 in menu_select GAME_DIR/src/ui-menu.c:797
    #16 0x674514 in death_screen GAME_DIR/src/ui-death.c:396
    #17 0x688fb0 in close_game GAME_DIR/src/ui-game.c:564
    #18 0x688937 in play_game GAME_DIR/src/ui-game.c:437
    #19 0x7563f5 in main GAME_DIR/src/main.c:524
    #20 0x7efef64bf290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)
    #21 0x404239 in _start (GAME_DIR/src/angband+0x404239)

0x619000058b90 is located 0 bytes to the right of 1040-byte region [0x619000058780,0x619000058b90)
allocated by thread T0 here:
    #0 0x7efef82f0210 in __interceptor_realloc /build/gcc/src/gcc/libsanitizer/asan/asan_malloc_linux.cc:75
    #1 0x7542de in mem_realloc GAME_DIR/src/z-virt.c:75
    #2 0x74d99a in textblock_resize_if_needed GAME_DIR/src/z-textblock.c:81
    #3 0x74dc9f in textblock_vappend_c GAME_DIR/src/z-textblock.c:116
    #4 0x74e672 in textblock_append GAME_DIR/src/z-textblock.c:173
    #5 0x542973 in info_out_list GAME_DIR/src/obj-info.c:155
    #6 0x543f2f in describe_ignores GAME_DIR/src/obj-info.c:359
    #7 0x552c40 in object_info_out GAME_DIR/src/obj-info.c:1785
    #8 0x5534fa in object_info_spoil GAME_DIR/src/obj-info.c:1881
    #9 0x738345 in spoil_artifact GAME_DIR/src/wiz-spoil.c:411
    #10 0x73a69c in spoiler_menu_act GAME_DIR/src/wiz-spoil.c:676
    #11 0x6b48f9 in menu_action_handle GAME_DIR/src/ui-menu.c:102
    #12 0x6bb86b in menu_handle_action GAME_DIR/src/ui-menu.c:661
    #13 0x6bcf39 in menu_select GAME_DIR/src/ui-menu.c:797
    #14 0x73a7d8 in do_cmd_spoilers GAME_DIR/src/wiz-spoil.c:709
    #15 0x674208 in death_spoilers GAME_DIR/src/ui-death.c:331
    #16 0x6b48f9 in menu_action_handle GAME_DIR/src/ui-menu.c:102
    #17 0x6bb86b in menu_handle_action GAME_DIR/src/ui-menu.c:661
    #18 0x6bcf39 in menu_select GAME_DIR/src/ui-menu.c:797
    #19 0x674514 in death_screen GAME_DIR/src/ui-death.c:396
    #20 0x688fb0 in close_game GAME_DIR/src/ui-game.c:564
    #21 0x688937 in play_game GAME_DIR/src/ui-game.c:437
    #22 0x7563f5 in main GAME_DIR/src/main.c:524
    #23 0x7efef64bf290 in __libc_start_main (/usr/lib/libc.so.6+0x20290)

SUMMARY: AddressSanitizer: heap-buffer-overflow /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cc:590 in __interceptor_wcslen
Shadow bytes around the buggy address:
  0x0c3280003120: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280003130: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280003140: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280003150: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x0c3280003160: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x0c3280003170: 00 00[fa]fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3280003180: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c3280003190: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800031a0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800031b0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x0c32800031c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd fd
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Heap right redzone:      fb
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack partial redzone:   f4
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==5429==ABORTING

(a minor detail changed for privacy reasons; GAME_DIR.)

The "0 bytes right of" makes me suspect that it's an off-by-one.

[Nick]

OK thanks, will play with it some more later.

[AnonymousHero]

Hm - now is recognizing the environment variable (I can tell, because it is fine with address, but doesn't like undefined), but still nothing on running.

Hm, those warnings may be because of "undefined" (which you don't have). Here's what I'm seeing on starting a game and pressing Ctrl+S:

z-virt.c:55:2: runtime error: null pointer passed as argument 1, which is declared to never be null
z-rand.c:521:9: runtime error: signed integer overflow: 97159930294914 * 1103515245 cannot be represented in type 'long int'

The first may be a real problem, the latter is unlikely (I'm guessing) to be a problem because there's probably no way than an optimizer is going to "see through" the z-rand code and start optimizing based on UB. (Though if there's any non-2C machines for which Angband is supposed to work it might still be bad code.)

Anyway, if you're using GCC, I think support for "undefined" is pretty recent, so support may effectively be dictated by the version carried by your distro. I'd recommend trying to compile with "clang" instead -- it generally had sanitizer features before gcc, so if your distro is a bit behind the bleeding edge you'll probably have better luck with clang.

EDIT to add: I also observed at least one heap overflow during the "dump artifacts" bit of the death screen. (It took a couple of tries with different randart sets.). I think another -- perhaps more reliable -- one was to "dump artifacts", focus away from the window and then click in the window again.

[Nick]

Hm - now is recognizing the environment variable (I can tell, because it is fine with address, but doesn't like undefined), but still nothing on running.

[AnonymousHero]

No, I'm seeing no difference, no reports, nothing. Started a game, descended two levels, got killed, generated artifact spoilers, nothing.

D'oh. I'm an idiot... the problem is the "SANITIZE_FLAGS =" line in buildsys.mk.in. I noticed the problem when testing, but must have flubbed the PR. Sorry about that.

Just remove that line from buildsys.mk and it should work.

EDIT: Btw, you should see at least one line the instant that the game starts up (with the X11 frontend at least). There's some sort of "NULL parameter given to memset" problem.

[Nick]

Just run normally. Do you see errors when running normally?

No, I'm seeing no difference, no reports, nothing. Started a game, descended two levels, got killed, generated artifact spoilers, nothing.

[AnonymousHero]

How do I actually run after doing the sanitize build?

Just run normally. Do you see errors when running normally? If you are, then you might try without "address" -- there was a really obscure ordering bug around program initialization in some version combos of gcc+glibc+sanitizer. (I *think* it would work with clang... or was it gcc?)

Also I find discovery of memory errors the opposite of depressing

I suppose one could also call them "interesting"

Possibly. Some menus have the behaviour where left arrow is like escape - it takes you back a step (birth menu, ignore menus, etc) - so it might be a bit confusing for option menus to have left and right arrows behave the same.

True. I think the birth menus should perhaps actually be changed to maybe actually ignore left-arrow (and right-arrow). There's already precent for left-arrow not doing anything "drastic" in the Point-allocation menu. (Maybe it's just my luddite self who experiences this and it may be compounded by my maintaining T2... which means that I playtest it quite a bit too.)

[Nick]

How do I actually run after doing the sanitize build?

Also I find discovery of memory errors the opposite of depressing

@Nick: Another minor little thing: Could we allow left-arrow to also function as "toggle" on the option screens? (Right-arrow works as toggle, so it seems logical to me that left-arrow should also work.)

Possibly. Some menus have the behaviour where left arrow is like escape - it takes you back a step (birth menu, ignore menus, etc) - so it might be a bit confusing for option menus to have left and right arrows behave the same.

[AnonymousHero]

@Nick: Another minor little thing: Could we allow left-arrow to also function as "toggle" on the option screens? (Right-arrow works as toggle, so it seems logical to me that left-arrow should also work.)

[AnonymousHero]

@Nick: I'm not sure if you've been using the sanitizers at all, but it turns out it's a bit easier to get them working these days. I've created a PR for basic (i.e. no "configure"-type support) sanitizer support: https://github.com/angband/angband/pull/474

EDIT: WARNING: It's a bit depressing running with the sanitizers... there seems to be quite a few heap-buffer overflows, especially around dumping artifact spoilers and the exit screen itself. However, these are actual bugs and should be fixed. (I wouldn't worry too much about the memory-leak reports.)

[Azuria]

Had a bug occur where smeagol was unable to pick up normal arrows, same kind of text you'd expect if it were a weapon of slay evil or similar.

[spara]

Hah. My HT warrior found a Ring of the Dog. Too bad it's cursed with Curse of Cowardice.

[spara]

@ got blinded and dashed the corridor towards the nearest stairs. On his way to safety, Wormtongue creates a set of traps around @. @ instantly feels and recognizes the traps around him even though he is on the run and blind as a bat.

This feels slightly too streamlined, so here's a suggestion. Maybe, when blinded, @ should feel undiscovered traps only when he tries to move to the trap square.

So when trying to move to an undiscovered trap square, a blind @ would not move or use energy, but the trap would be revealed with a message saying "Found a trap."