11:20 p.m.
Hi all,
I managed to compile the latest CVS natively under Win32 (and using my
own setup ;-) but while trying the debug version, I got a stack
corruption. The following patch cures it.
--- src\doc.c~ Mon May 8 15:23:16 2006
+++ src\doc.c Fri May 12 16:00:19 2006
＠＠ -49,7 +49,7 ＠＠
{
Ibyte buf[DOC_MAX_FILENAME_LENGTH+1];
Ibyte *buffer = buf;
- int buffer_size = sizeof (buf), space_left;
+ int buffer_size = sizeof (buf) - 1, space_left;
Ibyte *from, *to;
REGISTER Ibyte *p = buffer;
Lisp_Object return_me;
＠＠ -168,7 +168,7 ＠＠
{
Ibyte buf[512 * 32 + 1];
Ibyte *buffer = buf;
- int buffer_size = sizeof (buf);
+ int buffer_size = sizeof (buf) - 1;
Ibyte *from, *to;
REGISTER Ibyte *p = buffer;
Lisp_Object return_me;
＠＠ -215,13 +215,14 ＠＠
if (space_left == 0)
{
Ibyte *old_buffer = buffer;
+ buffer_size *= 2;
if (buffer == buf)
{
- buffer = xnew_ibytes (buffer_size *= 2);
+ buffer = xnew_ibytes (buffer_size + 1);
memcpy (buffer, old_buffer, p - old_buffer);
}
else
- XREALLOC_ARRAY (buffer, Ibyte, buffer_size *= 2);
+ XREALLOC_ARRAY (buffer, Ibyte, buffer_size + 1);
p += buffer - old_buffer;
space_left = buffer_size - (p - buffer);
}
Best regards,
--
Fabrice Popineau
------------------------
e-mail: Fabrice.Popineau(a)supelec.fr | The difference between theory
voice-mail: +33 (0) 387764715 | and practice, is that
surface-mail: Supelec, 2 rue E. Belin, | theoretically,
F-57070 Metz | there is no difference !
7:32 p.m.
Ar an triú lá déag de mí Bealtaine, scríobh Fabrice Popineau:
I managed to compile the latest CVS natively under Win32 (and using
my
own setup ;-) but while trying the debug version, I got a stack
corruption. The following patch cures it.
Cool, thank you for tracking it down. But are you sure the corruption
appears after Stephen’s patch of
http://mid.gmane.org/87slnt7nrq.fsf_-_＠tleepslib.sk.tsukuba.ac.jp was
committed? Because the first part of it looks like it would have roughly
the same effect on the stack as Stephen’s fix, and the second part is an
change to old code that’s been around for ten years without provoking a
bug.
--
Aidan Kehoe, http://www.parhasard.net/
8:49 p.m.
Aidan Kehoe wrote:
Ar an triú lá déag de mí Bealtaine, scríobh Fabrice Popineau:
> I managed to compile the latest CVS natively under Win32 (and using my
> own setup ;-) but while trying the debug version, I got a stack
> corruption. The following patch cures it.
Cool, thank you for tracking it down. But are you sure the corruption
appears after Stephen’s patch of
http://mid.gmane.org/87slnt7nrq.fsf_-_＠tleepslib.sk.tsukuba.ac.jp was
committed? Because the first part of it looks like it would have roughly
the same effect on the stack as Stephen’s fix, and the second part is an
change to old code that’s been around for ten years without provoking a
bug.
It looks like he has already applied Stephen's patch.
7:38 a.m.
* Aidan Kehoe <kehoea(a)parhasard.net> writes:
Ar an triú lá déag de mí Bealtaine, scríobh Fabrice Popineau:
> I managed to compile the latest CVS natively under Win32 (and
> using my own setup ;-) but while trying the debug version, I got
> a stack corruption. The following patch cures it.
Cool, thank you for tracking it down. But are you sure the
corruption appears after Stephen~s patch of
http://mid.gmane.org/87slnt7nrq.fsf_-_＠tleepslib.sk.tsukuba.ac.jp
was committed? Because the first part of it looks like it would
have roughly the same effect on the stack as Stephen~s fix, and
the second part is an change to old code that~s been around for
ten years without provoking a bug.
Not at all. Stephen's patch was abour enlarging the buffer. But that
doesn't cure the fact that :
Ibyte buf[DOC_MAX_FILENAME_LENGTH+1];
Ibyte *buffer = buf;
int buffer_size = sizeof (buf), space_left;
^^^^^^^^^^^^
whatever the size of the buf[] array, buffer_size is set to this size.
REGISTER Ibyte *p = buffer;
p and buffer point to the same location
space_left = buffer_size - (p - buffer);
space_left is buffer_size
while (space_left > 0)
{
int nread;
nread = Lstream_read (XLSTREAM (instream), p, space_left);
You read buffer_size characters into buf[0 .. buffer_size - 1] that is the buf[] array is
full
p[nread] = 0;
^^^^^^^^^^^^^^^^^^^
and then you are accessing p[buffer_size] which is prohibited whatever
the real size of the buffer is, because your buf index goes from 0 to
buffer_size - 1.
I fixed it in both places but I can acknowledge only for the first one
causing a crash.
Fabrice
8:20 a.m.
Ar an séú lá déag de mí Bealtaine, scríobh Fabrice Popineau:
I fixed it in both places but I can acknowledge only for the first
one
causing a crash.
You’re right, forgive my hasty response. And in the second place the code,
while incorrect, didn’t ever trigger a crash, because the bit of code with
the comment /* Don't read too much in one go */ meant the buffer wasn’t ever
filled up to its end.
--
Aidan Kehoe, http://www.parhasard.net/
8:26 a.m.
>>>> "Fabrice" == Fabrice Popineau
<Fabrice.Popineau(a)supelec.fr> writes:
Fabrice> Not at all. Stephen's patch was abour enlarging the
Fabrice> buffer. But that doesn't cure the fact that :
Wow! I wonder why my patch worked at all; I guess my compiler (GCC)
pads to a multiple of wordsize or something like that, while yours
doesn't. Thanks for catching this!
Fabrice> Ibyte buf[DOC_MAX_FILENAME_LENGTH+1];
Fabrice> Ibyte *buffer = buf;
Fabrice> int buffer_size = sizeof (buf), space_left;
Since the '\0' is actually an artificial sentinel, I suggest
(1) change the init code to
Ibyte buf[DOC_MAX_FILENAME_LENGTH+1];
Ibyte *buffer = buf;
int buffer_size = sizeof (buf) - 1, /* omit sentinel position */
space_left;
(2) change all other uses of DOC_MAX_FILENAME_LENGTH to buffer_size.
--
School of Systems and Information Engineering http://turnbull.sk.tsukuba.ac.jp
University of Tsukuba Tennodai 1-1-1 Tsukuba 305-8573 JAPAN
Just Say No to porting sucky GNU code!
6765
days inactive
6769
days old
5 comments
4 participants
participants (4)
-
Aidan Kehoe -
Fabrice Popineau -
Rodney Sparapani -
Stephen J. Turnbull