Fastdeath uploaded file crashdump.txt (18.9 KiB)

Fastdeath commented

Forgot details:
WZ Version: 2.3.2
OS: Ubuntu Linux 10.04

Fastdeath commented

Happened again with 3 Players on the same map.

cybersphinx commented

The crash is caused by player being 4294967285 (and using that to index an array of size 8), a value that it can't get if I didn't misread the code...

Zarel commented

Fastdeath says cybersphinx thinks it might be compiler bug.

I'm not sure, but I think it might be a new-in-2.3.2 bug. Both people were playing on Linux, so that might have something to do with it, too.

My theory is that it's caused by an array overflow overwriting the contents of the player variable. Review all our 2.3.2 commits that could have caused an overflow?

cybersphinx commented

It might happen in 2.3.2 not because we changed some array to overflow or something, but because the code layout changed, and what it overwrote in 2.3.1 was irrelevant (or lead to different problems bugs).

The problem with those kind of bugs is that we really need a way to reproduce them to fix them (or you need to run in gdb all the time, with a watch on player that triggers when it goes above 7, and then get a full backtrace).

Fastdeath commented

This is maybe a compiler bug 2.3.2 works "currently" fine with gcc-4.4.1

Zarel commented

Replying to Warzone2100/old-trac-import#2029 (comment:7):

This is maybe a compiler bug 2.3.2 works "currently" fine with gcc-4.4.1

I seriously doubt it's a compiler bug. Like cybersphinx says, far more likely that a compiler change caused one of our other bugs to modify player instead of something else.

Fortunately, this makes it a lot easier to fix, too. Set a watchpoint on player, and wait for the crash. Can you do that for us? Pleasepleaseplease?

cybersphinx commented

Run "gdb src/warzone2100", then enter

set height 0
watch pickADroid::player
commands
 if (player < 0 || player > 7)
  bt full
 end
 if (player >= 0 && player <= 8)
  continue
 end
end
run

That will trigger a full backtrace if player goes outside its valid range, and continue otherwise.

The indentation is just for clarity, and height 0 prevents "press enter to continue" messages after one screen page of output.

cybersphinx commented

Oh, you probably need to use "pickADroid::player" instead of just "player" in the conditions, since when this bug triggers, it won't be in scope.

cybersphinx commented

More crashdumps in #2031, #2032, #2034.

cybersphinx commented

Zarel: If you have more info about this problem, would you mind adding it here?

Zarel commented

I have no info except a question about why, for a bug already reported like four times, we still haven't gotten a trace of a watchpoint hit of pickADroid::player > 7 yet.

cybersphinx commented

Maybe because we need one of the binaries that actually crash? I've asked Edek22 to attach his here, and I think I've asked Fastdeath as well. If not, he shall now consider himself asked.

Zarel commented

Well, yeah, I'm not blaming you or the staff, I'm just wondering why none of the people experiencing this bug have actually done the trace.

Fastdeath commented

My crashing binary:
http://www.pc-dummy.net/downloads/warzone/warzone2100.bz2

Fastdeath uploaded file pickadroid-gdb.txt (984.7 KiB)

Watchpoint on pickadroid::player with crash

Fastdeath uploaded file pickadroid-gdb.txt.bz2 (3.3 KiB)

Safety0ff changed priority from major to blocker

cybersphinx uploaded file 2029.patch (2.6 KiB)

Proposed patch.

cybersphinx uploaded file 2029-nowhitespace.patch (1.9 KiB)

Same without whitespace changes, since the real changes are not much.

the_cybersphinx changed status from new to closed

the_cybersphinx set resolution to fixed

the_cybersphinx commented

(In [11322]) Prevent out of bounds access of player stat arrays.

In the 2.3 netcode, we use player 9 for features, while we only have stats for
players 0-7. Fixes #2029 and probably a truckoad of other random problems.

the_cybersphinx commented

(In [11323]) Prevent out of bounds access of player stat arrays.

In the 2.3 netcode, we use player 9 for features, while we only have stats for
players 0-7. Fixes #2029 and probably a truckoad of other random problems.

the_cybersphinx commented

(In [11324]) Prevent out of bounds access of player stat arrays.

In the 2.3 netcode, we use player 9 for features, while we only have stats for
players 0-7. Fixes #2029 and probably a truckoad of other random problems.

Git SVN Gateway <gateway@...> commented

(In Warzone2100/warzone2100@324722f) Prevent out of bounds access of player stat arrays.

In the 2.3 netcode, we use player 9 for features, while we only have stats for
players 0-7. Fixes #2029 and probably a truckoad of other random problems.

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@11323 4a71c877-e1ca-e34f-864e-861f7616d084

Git SVN Gateway <gateway@...> commented

In Warzone2100/warzone2100@324722f:

#CommitTicketReference repository="" revision="324722fc93e6cc069a04e8a64a5841d75a17a3bd"
Prevent out of bounds access of player stat arrays.

In the 2.3 netcode, we use player 9 for features, while we only have stats for
players 0-7. Fixes #2029 and probably a truckoad of other random problems.

git-svn-id: https://warzone2100.svn.sourceforge.net/svnroot/warzone2100/trunk@11323 4a71c877-e1ca-e34f-864e-861f7616d084