| Message ID | 87wqdzjccp.fsf@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, May 06, 2014 at 01:55:18PM +0100, Nick Clifton wrote: > 2014-05-06 Nick Clifton <nickc@redhat.com> > > * except.c (init_eh): Fix computation of builtin setjmp buffer > size. > > Index: gcc/except.c > =================================================================== > --- gcc/except.c (revision 210096) > +++ gcc/except.c (working copy) > @@ -287,7 +287,7 @@ > #endif > #else > /* builtin_setjmp takes a pointer to 5 words. */ > - tmp = size_int (5 * BITS_PER_WORD / POINTER_SIZE - 1); > + tmp = size_int (5 * POINTER_SIZE / BITS_PER_WORD - 1); > #endif > tmp = build_index_type (tmp); > tmp = build_array_type (ptr_type_node, tmp); That doesn't look correct to me. If the code wants to create 5 words long array, but with pointer elements (instead of word sized elements), then 5 * BITS_PER_WORD is the desired size in bits of the buffer, POINTER_SIZE is how many bits each void *array[...] element occupies and thus 5 * BITS_PER_WORD / POINTER_SIZE - 1 is the right last element, so I'd say the previous expression is the right one. Perhaps you want to round up rather than down, but certainly not swap the division operands. Now, if the code actually expects 5 pointers, rather than 5 words, or something else, then you'd at least need to also fix the comment. Jakub
> 2014-05-06 Nick Clifton <nickc@redhat.com> > > * except.c (init_eh): Fix computation of builtin setjmp buffer > size. That's the same patch as https://gcc.gnu.org/ml/gcc-patches/2011-11/msg00272.html and is still incorrect.
Hi Eric, >> 2014-05-06 Nick Clifton <nickc@redhat.com> >> >> * except.c (init_eh): Fix computation of builtin setjmp buffer >> size. > > That's the same patch as > https://gcc.gnu.org/ml/gcc-patches/2011-11/msg00272.html > and is still incorrect. Ah - you are worried about the case where STACK_SAVEAREA_MODE() is smaller than Pmode, yes ? OK then, how about this revised version of the patch where the size computation is now: tmp = size_int (MAX (GET_MODE_SIZE (STACK_SAVEAREA_MODE (SAVE_NONLOCAL)) / GET_MODE_SIZE (Pmode), 1) + 2 /* Stack pointer and frame pointer. */ + 1 /* Slop for mips. */ - 1); OK to apply ? Cheers Nick
> Ah - you are worried about the case where STACK_SAVEAREA_MODE() is > smaller than Pmode, yes ? No, simply that the modified formula is plain wrong. The code does: tmp = size_int (5 * BITS_PER_WORD / POINTER_SIZE - 1); tmp = build_index_type (tmp); tmp = build_array_type (ptr_type_node, tmp); so, in the end, the size of the buffer is: [(5 * BITS_PER_WORD / POINTER_SIZE - 1) + 1] * POINTER_SIZE which boilds down to: 5 * BITS_PER_WORD provided that POINTER_SIZE <= BITS_PER_WORD. So we have a problem if POINTER_SIZE > BITS_PER_WORD, in which case it's sufficient to use 5 * POINTER_SIZE instead. > OK then, how about this revised version of the patch where the size > computation is now: > > tmp = size_int (MAX (GET_MODE_SIZE (STACK_SAVEAREA_MODE > (SAVE_NONLOCAL)) > / GET_MODE_SIZE (Pmode), 1) > + 2 /* Stack pointer and frame pointer. */ > + 1 /* Slop for mips. */ > - 1); > > OK to apply ? No, that's too complicated and risky, just do the following: /* builtin_setjmp takes a pointer to 5 words or pointers. */ if (POINTER_SIZE > BITS_PER_WORD) tmp = size_int (4); else tmp = size_int (5 * BITS_PER_WORD / POINTER_SIZE - 1); which is simple and safe.
On May 15, 2014, at 12:55 AM, Eric Botcazou <ebotcazou@adacore.com> wrote: > No, that's too complicated and risky, just do the following: > > /* builtin_setjmp takes a pointer to 5 words or pointers. */ > if (POINTER_SIZE > BITS_PER_WORD) > tmp = size_int (4); > else > tmp = size_int (5 * BITS_PER_WORD / POINTER_SIZE - 1); > > which is simple and safe. But, fails whenever the size of the mode of the save area is bigger than a certain amount… On my port, the size taken up by the save area is large enough to cause this to fail. :-( The correct bug fix would have my port not fail.
> But, fails whenever the size of the mode of the save area is bigger than a > certain amount… On my port, the size taken up by the save area is large > enough to cause this to fail. :-( That's a bit unexpected, why do you need so big a save area exactly? The only architecture for which this doesn't work is the IA-64, which is a very special beast... In this case, the way out is to define DONT_USE_BUILTIN_SETJMP and JMP_BUF_SIZE to the needed size.
Index: gcc/except.c =================================================================== --- gcc/except.c (revision 210096) +++ gcc/except.c (working copy) @@ -287,7 +287,7 @@ #endif #else /* builtin_setjmp takes a pointer to 5 words. */ - tmp = size_int (5 * BITS_PER_WORD / POINTER_SIZE - 1); + tmp = size_int (5 * POINTER_SIZE / BITS_PER_WORD - 1); #endif tmp = build_index_type (tmp); tmp = build_array_type (ptr_type_node, tmp);