Complete.Org: Mailing Lists: Archives: freeciv-dev: December 2002:
[Freeciv-Dev] (PR#2665) invalid read in civserver
Home

[Freeciv-Dev] (PR#2665) invalid read in civserver

[Top] [All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index] [Thread Index]
To: undisclosed-recipients:;
Subject: [Freeciv-Dev] (PR#2665) invalid read in civserver
From: "Jason Short via RT" <rt@xxxxxxxxxxxxxx>
Date: Sun, 29 Dec 2002 00:22:27 -0800
Reply-to: rt@xxxxxxxxxxxxxx

I ran an autogame under valgrind, and quickly ran across this error:

> ==15769== Invalid read of size 4
==15769==    at 0x80C3543: ai_choose_diplomat_offensive
(aidiplomat.c:156)
==15769==    by 0x80B4E1D: military_advisor_choose_build
(advmilitary.c:1476)
==15769==    by 0x80B8688: ai_manage_cities (aicity.c:509)
==15769==    by 0x80BAAB4: ai_do_last_activities (aihand.c:66)
==15769==    Address 0x40A5EE88 is 12 bytes after a block of size 2144
alloc'd
==15769==    at 0x4003BA4E: malloc (vg_clientfuncs.c:100)
==15769==    by 0x804C1AD: fc_real_malloc (mem.c:40)
==15769==    by 0x8061E74: create_city (citytools.c:987)
==15769==    by 0x805EEB3: unit_enter_hut (unittools.c:2470)

It is easy to determine that in citytools.c sizeof(struct city) is 2144,
while in aidiplomat.c it is 2160.  This appears to be because config.h
is not #included in aidiplomat.c.  I believe the error is a direct
result of stdbool.h being used (with HAVE_STDBOOL_H defined in
config.h), since modern versions of gcc (I use 3.2) use an internal
representation of "bool" (probably 1-byte, as opposed to freeciv's
default 4-byte fc_bool type).  Of course there could be other problems
as well.

This bug could cause completely unpredictable behavior - reads and
writes in the completely wrong location anywhere in aidiplomat.c and any
other files that don't include config.h.  The fix is to #include
<config.h> from EVERY .c file.  And this requirement should be added to
the documentation.

An alternative is to #include <config.h> only in the "appropriate"
locations (in this case, shared.h).  But this is much riskier and is not
likely to give any substantial benefit.

jason




[Prev in Thread] Current Thread [Next in Thread]