SlashTHEM icon indicating copy to clipboard operation
SlashTHEM copied to clipboard

Inconsistant results when dropping potion of acid into pool/puddle

Open Soviet5lo opened this issue 1 year ago • 1 comments

When dropping a potion of acid into a puddle, the potion will explode but the potion is still available on the space. If a few other items are dropped on the same space, the player steps away and returns to the same spot the items are heavily corrupted before crashing with the following backtrace:

#0  dog_goal (mtmp=0x555555a2daa0, edog=0x555555a2db40, after=0, udist=548, 
    whappr=0) at src/dogmove.c:489
#1  0x0000555555602b03 in dog_move (mtmp=0x555555a2daa0, after=0)
    at src/dogmove.c:736
#2  0x00005555556d0dc4 in m_move (mtmp=0x555555a2daa0, after=0)
    at src/monmove.c:857
#3  0x00005555556d0285 in dochug (mtmp=0x555555a2daa0) at src/monmove.c:686
#4  0x00005555556cd0eb in dochugw (mtmp=0x555555a2daa0) at src/monmove.c:111
#5  0x00005555556be5d3 in movemon () at src/mon.c:849
#6  0x0000555555590962 in moveloop () at src/allmain.c:113
#7  0x000055555581d0fa in main (argc=1, argv=0x7fffffffdeb8)
    at sys/unix/unixmain.c:309

Dropping multiple (+8?) items on the spot, navigating away and returning to the spot procudes a different crash when attempting to pick up an object:

#0  0x0000555555718d86 in query_objlist (qstr=0x55555586d26a "Pick up what?", 
    olist=0x555555b59420, qflags=43, pick_list=0x7fffffffd3f8, how=2, 
    allow=0x555555717537 <all_but_uchain>) at src/pickup.c:917
#1  0x000055555571811d in pickup (what=0) at src/pickup.c:601
#2  0x000055555565074a in dopickup () at src/hack.c:2706
#3  0x00005555555c86ab in rhack (cmd=0x5555558f51a0 <in_line> ",")
    at src/cmd.c:3814
#4  0x0000555555591c05 in moveloop () at src/allmain.c:654
#5  0x000055555581d0fa in main (argc=1, argv=0x7fffffffdeb8)
    at sys/unix/unixmain.c:309

Dropping potions of acid into a pool however tends to result in an immediate crash with the following backtrace

#0  m_move (mtmp=0x555555a30c10, after=1436733744) at src/monmove.c:1030
#1  0x00005555556d0285 in dochug (mtmp=0x555555a30c10) at src/monmove.c:686
#2  0x00005555556cd0eb in dochugw (mtmp=0x555555a30c10) at src/monmove.c:111
#3  0x00005555556be5d3 in movemon () at src/mon.c:849
#4  0x0000555555590962 in moveloop () at src/allmain.c:113
#5  0x000055555581d0fa in main (argc=1, argv=0x7fffffffdeb8)
    at sys/unix/unixmain.c:309

Will attempt to pull further details (address sanitizer) later

Soviet5lo avatar Jul 07 '22 04:07 Soviet5lo

It's kind of a PITA to deal with this in the right way but it's worth doing

I think the problem is that water_damage generally targets a chain of items and doesn't report when any object has been destroyed, so any references afterwards could operate on an object that isn't there (use after frees and other crashes for example)

vanilla splits up the water_damage function to one that specifically targets a single item (reporting using a return value what happened to that object, e.g. ER_DESTROYED) and a water_damage_chain that focuses on an item chain

That's probably what we should do here too, might try that at the weekend

If we split that out then in e.g. dropping the potion of acid we can check whether the potion was destroyed then avoid continuing on with the item at that point

gebulmer avatar Jul 22 '22 20:07 gebulmer