This is the mail archive of the
fortran@gcc.gnu.org
mailing list for the GNU Fortran project.
Re: [PATCH, libgfortran] PR 60324 Unbounded stack allocations in libgfortran
- From: Cesar Philippidis <cesar_philippidis at mentor dot com>
- To: Janne Blomqvist <blomqvist dot janne at gmail dot com>, Tobias Burnus <burnus at net-b dot de>
- Cc: Fortran List <fortran at gcc dot gnu dot org>, GCC Patches <gcc-patches at gcc dot gnu dot org>
- Date: Fri, 14 Nov 2014 22:19:11 -0800
- Subject: Re: [PATCH, libgfortran] PR 60324 Unbounded stack allocations in libgfortran
- Authentication-results: sourceware.org; auth=none
- References: <CAO9iq9HTtPAM-ronwWCJWv7LnSbvg9sd=9oE-RqRQHEyevG7+w at mail dot gmail dot com> <54665600 dot 3080209 at mentor dot com> <54666DDE dot 9050601 at net-b dot de> <CAO9iq9E4=G-eQ_XHMd7pArP=qAQrqAts0fvEn+NE6N5fB2h_sw at mail dot gmail dot com>
On 11/14/2014 10:01 PM, Janne Blomqvist wrote:
> On Fri, Nov 14, 2014 at 11:02 PM, Tobias Burnus <burnus@net-b.de> wrote:
>> Cesar Philippidis wrote:
>>>
>>> On 11/13/2014 02:32 AM, Janne Blomqvist wrote:
>>> I hit an error when building intrinsics/random.c:
>>> error: expression in static assertion is not constant
>>> Joseph told me that static const variables cannot be used in constant
>>> expressions in C, so I've replaced the _Static_assert with a regular
>>> assert. Are you using g++ to build libgfortran?
>>
>>
>> I wonder why you are seeing this while others aren't.
>
> Yeah, I wonder the same?
It does seem strange. Maybe that function isn't necessary for all
targets? I've attached a reduced test case if anyone is interested in
it. That error should show up with "gcc random.c".
>>> I don't have a good baseline test this patch thoroughly, but at least I
>>> can bootstrap gcc without it failing in libgfortran. Is this OK for
>>> mainline and/or could someone see if it causes any regressions?
>>
>>
>> I think instead of doing a run-time check I'd prefer something like the
>> following, keeping the compile-time assert.
>>
>> --- a/libgfortran/intrinsics/random.c
>> +++ b/libgfortran/intrinsics/random.c
>> @@ -253 +253 @@ static GFC_UINTEGER_4 kiss_default_seed[] = {
>> -static const GFC_INTEGER_4 kiss_size =
>> sizeof(kiss_seed)/sizeof(kiss_seed[0]);
>> +#define KISS_SIZE ((GFC_INTEGER_4) (sizeof(kiss_seed)/sizeof(kiss_seed[0]))
>>
>> (plus s/kiss_size/KISS_SIZE/ changes in the code.)
>>
>> Janne, what do you think?
>
> I like it. With this, you can also get rid of the assert and the newly
> introduced KISS_MAX_SIZE macro, and just make the seed array the
> correct size, as was originally done (with a VLA). Consider such a
> patch pre-approved.
Thanks for fixing this!
Cesar
#include <stdint.h>
typedef int32_t GFC_INTEGER_4;
typedef uint32_t GFC_UINTEGER_4;
static const GFC_UINTEGER_4 kiss_seed[] = {
123456789, 362436069, 521288629, 316191069,
987654321, 458629013, 582859209, 438195021,
573658661, 185639104, 582619469, 296736107
};
static const GFC_INTEGER_4 kiss_size = sizeof(kiss_seed)/sizeof(kiss_seed[0]);
int
main ()
{
#define KISS_MAX_SIZE 12
unsigned char seed[4 * 12];
_Static_assert (kiss_size <= 12, "kiss_size must <= KISS_MAX_SIZE");
//static_assert (kiss_size <= 12, "kiss_size must <= KISS_MAX_SIZE");
return 0;
}