search

dxx-rebirth / dxx-rebirth

Descent-Rebirth and Descent II-Rebirth
Other
501 stars 117 forks source link

Possible memory error with D1 PIG loading in D2 #340

Open Blzut3 opened 7 years ago

Blzut3 commented 7 years ago

Was trying to force Descent 2 to have classic rendered mine exit sequences like the original Descent and was largely successful by copying a few bits from D2DEMO.HOG and adding DESCENT.PIG to my d2x rebirth directory. However levels 17-20 would crash entering the exit tunnel and I traced it to the following two lines:

https://github.com/dxx-rebirth/dxx-rebirth/blob/b0cb681ae79e776fdfc83ea95cf299acbd7db459/similar/main/piggy.cpp#L1834 https://github.com/dxx-rebirth/dxx-rebirth/blob/b0cb681ae79e776fdfc83ea95cf299acbd7db459/similar/main/piggy.cpp#L1844

After forcing DXX Rebirth to leak memory (removing the call to free a little ways down the call stack in the first case, and changing realloc to malloc) the tunnels worked, but the game would crash later which probably means it's stomping on some arbitrary memory (which would explain why only those 4 levels are affected despite the code path being travelled).

Later I found that using EXIT.HAM from the Mac version of the D2 demo and providing the bbms outside of DESCENT.PIG produces a stable game so there definitely seems to be something wrong with the way memory is being handled in this code.

vLKp commented 7 years ago

At nothing else, that second line is buggy because it will leak if d_realloc fails. Bitmap memory handling has always been ugly. The original code had a lot of tricky logic to minimize allocations, which makes the ownership rules a mess.

Could you post either the stacktrace of the crash (preferably from a -ggdb -fsanitize=address build) or instructions on how I can recreate your modified exit sequence? I have all the files for Descent and Descent 2, but I do not know what you copied from the demo or where it needs to go.

Blzut3 commented 7 years ago

I copied d2leva-*.txb, lev01ter.bbm, moon01.bbm, moonsky1.bbm, and moonsky2.bbm from D2DEMO.HOG into my DESCENT2.HOG (just after all the other txbs). Grabbed palette.256 from DESCENT.HOG into DESCENT2.HOG before the other palettes. Optionally grab endlevel.hm* from D2DEMO.HOG (added these later so I know the crash happens without them).

I deleted the movie files to prevent them from being played (they take precedence) and added DESCENT.PIG to the directory.

Now the exit sequences should work, but level 17-20 will crash when you exit the mines. I've observed this with Kubuntu 17.04 64-bit and Windows (my own build in both cases, Windows build was with TDM-GCC 5.1).

vLKp commented 7 years ago

First, thank you for the initial analysis. I should have said that in my opening response. Inducing a memory leak is not a viable solution, but knowing that the leak prevents the crash may turn out to be a useful hint.

Second, I do not generally modify the data files, so I don't have the tools set up to do so. I will need to set aside some time to prepare the relevant tools and apply your instructions. I will likely not be able to get to this for a few days.

Blzut3 commented 7 years ago

This report is more of a brain dump any way. As stated in the report I did find another way to do what I want that avoids the D1 PIG code and doesn't have the problem, but of course I'm sure you'd rather know about the issue rather than me just working around it and continuing on my way. Especially since I'm sure this code path isn't very often travelled. :)