|
From: Nicholas N. <nj...@cs...> - 2005-05-07 03:35:57
|
Hi,
I just saw this around line 478 of vg_signals.c:
VG_TRACK( new_mem_stack_signal,
ss->ss_sp + ss->ss_size - VGA_STACK_REDZONE_SIZE,
VGA_STACK_REDZONE_SIZE );
This doesn't look right -- it's mixing pointer arithmetic with integer
arithmetic. Tom, I think you wrote this, can you please check it?
Thanks.
N
|
|
From: Tom H. <to...@co...> - 2005-05-07 06:44:34
|
In message <Pin...@ch...>
Nicholas Nethercote <nj...@cs...> wrote:
> I just saw this around line 478 of vg_signals.c:
>
> VG_TRACK( new_mem_stack_signal,
> ss->ss_sp + ss->ss_size - VGA_STACK_REDZONE_SIZE,
> VGA_STACK_REDZONE_SIZE );
>
> This doesn't look right -- it's mixing pointer arithmetic with integer
> arithmetic. Tom, I think you wrote this, can you please check it?
I don't understand the problem... I assume you're talking about
the second argument right?
The idea is to mark the top 128 bytes of the signal stack as valid
on amd64 so we add the size to the base to find the top of the stack
and then subtract the redzone size (128 bytes on amd64) to get the
address of the memory we want to mark as valid.
It's an entirely valid calculation - adding an integer to a pointer
gives a pointer from which we then subtract an integer to get another
pointer.
Tom
--
Tom Hughes (to...@co...)
http://www.compton.nu/
|
|
From: Nicholas N. <nj...@cs...> - 2005-05-07 16:48:57
|
On Sat, 7 May 2005, Tom Hughes wrote: >> VG_TRACK( new_mem_stack_signal, >> ss->ss_sp + ss->ss_size - VGA_STACK_REDZONE_SIZE, >> VGA_STACK_REDZONE_SIZE ); >> >> This doesn't look right -- it's mixing pointer arithmetic with integer >> arithmetic. Tom, I think you wrote this, can you please check it? > > I don't understand the problem... I assume you're talking about > the second argument right? Yes. > The idea is to mark the top 128 bytes of the signal stack as valid > on amd64 so we add the size to the base to find the top of the stack > and then subtract the redzone size (128 bytes on amd64) to get the > address of the memory we want to mark as valid. > > It's an entirely valid calculation - adding an integer to a pointer > gives a pointer from which we then subtract an integer to get another > pointer. Sure, but what are the units? ss->ss_sp has type "void*". So if you add an integer i to it, it will really add "sizeof(void*) * i" to it, right? If it had type "Addr" instead, adding i to it would really add i to it. But VGA_STACK_REDZONE_SIZE is in bytes, not words (it should be called VGA_STACK_REDZONE_SZB to avoid this confusion). I don't know what the units of ss->ss_size are, bytes or words. So basically I'm saying the units (bytes and words) are all mixed up. Is the problem clear now? Or maybe I've totally misunderstood how C works. N |
|
From: Tom H. <to...@co...> - 2005-05-07 16:54:37
|
In message <Pin...@ch...>
Nicholas Nethercote <nj...@cs...> wrote:
> ss->ss_sp has type "void*". So if you add an integer i to it, it will
> really add "sizeof(void*) * i" to it, right? If it had type "Addr"
> instead, adding i to it would really add i to it.
Ah right. I understand now... I didn't realise it was a void pointer.
I'll fix it up, but we really should have the gcc warning for doing
arithmetic on void pointers turned on.
Tom
--
Tom Hughes (to...@co...)
http://www.compton.nu/
|