PR 78534 Change character length from int to size_t

Hi Janne,

I found that you are favoring build_int_cst (size_type_node, 0) over
size_zero_node. Is there a reason to this?

Furthermore did I have to patch this:


to bootstrap on x86_64-linux/f23.

And I have this regression:

FAIL: gfortran.dg/allocate_deferred_char_scalar_1.f03   -O1  (test for excess
errors)

allocate_deferred_char_scalar_1.f03:184:0:

     p = '12345679'
 
Warning: '__builtin_memcpy' writing 8 bytes into a region of size 5 overflows
     the destination [-Wstringop-overflow=]
     allocate_deferred_char_scalar_1.f03:242:0:

     p = 4_'12345679'
 
Warning: '__builtin_memcpy' writing 32 bytes into a region of size 20 overflows
the destination [-Wstringop-overflow=]

I also tried with a sanitized gfortran and am seeing some issues there. I have
to sort through these, but thought to let you know about the above already.

Btw, the patch for changing the ABI of the coarray-libs is already nearly done.
I just need to figure that what the state of regressions is with and without my
change.

- Andre

On Mon, Dec 12, 2016 at 7:39 PM, Andre Vehreschild <vehre@gmx.de> wrote:
> Hi Janne,
>
> I found that you are favoring build_int_cst (size_type_node, 0) over
> size_zero_node. Is there a reason to this?

Yes. AFAIU size_zero_node is a zero constant for sizetype which is not
the same as size_type_node.

AFAIK the difference is that size_type_node is the C size_t type,
whereas sizetype is a GCC internal type used for address expressions.
On a "normal" target I understand that they are the same size, but
there are some slight differences in semantics, e.g. size_type_node
like C unsigned integer types is defined to wrap around on overflow
whereas sizetype is undefined on overflow.

I don't know if GCC supports some strange targets with some kind of
segmented memory where the size of sizetype would be different from
size_type_node, but I guess it's possible in theory at least.

That being said, now that you mention in I should be using
build_zero_cst (some_type_node) instead of
build_int_cst(some_type_node, 0). There's also build_one_cst that I
should use.

> Furthermore did I have to patch this:
>
> diff --git a/gcc/fortran/dump-parse-tree.c b/gcc/fortran/dump-parse-tree.c
> index 585f25d..f374558 100644
> --- a/gcc/fortran/dump-parse-tree.c
> +++ b/gcc/fortran/dump-parse-tree.c
> @@ -465,7 +465,7 @@ show_expr (gfc_expr *p)
>           break;
>
>         case BT_HOLLERITH:
> -         fprintf (dumpfile, "%dH", p->representation.length);
> +         fprintf (dumpfile, "%zdH", p->representation.length);
>           c = p->representation.string;
>           for (i = 0; i < p->representation.length; i++, c++)
>             {
>
> to bootstrap on x86_64-linux/f23.

Ah, thanks for the catch. I'll fix it by using HOST_WIDE_INT_PRINT_DEC
since I'll have to change gfc_charlen_t to be a typedef form
HOST_WIDE_INT (see my answer to FX).

> And I have this regression:
>
> FAIL: gfortran.dg/allocate_deferred_char_scalar_1.f03   -O1  (test for excess
> errors)
>
> allocate_deferred_char_scalar_1.f03:184:0:
>
>      p = '12345679'
>
> Warning: '__builtin_memcpy' writing 8 bytes into a region of size 5 overflows
>      the destination [-Wstringop-overflow=]
>      allocate_deferred_char_scalar_1.f03:242:0:
>
>      p = 4_'12345679'
>
> Warning: '__builtin_memcpy' writing 32 bytes into a region of size 20 overflows
> the destination [-Wstringop-overflow=]

I'm seeing that too, but I assumed they would be fixed by Paul's
recent patch which I don't yet have in my tree yet due to the git
mirror being stuck..

> Btw, the patch for changing the ABI of the coarray-libs is already nearly done.
> I just need to figure that what the state of regressions is with and without my
> change.

Thanks.

I'll produce an updated patch with the changes discussed so far.

Hi Janne,

How about adding charlen_zero_node and one_node like the others have it to prevent repeating ourselves?

- Andre

Am 12. Dezember 2016 20:39:38 MEZ, schrieb Janne Blomqvist <blomqvist.janne@gmail.com>:
>On Mon, Dec 12, 2016 at 7:39 PM, Andre Vehreschild <vehre@gmx.de>
>wrote:
>> Hi Janne,
>>
>> I found that you are favoring build_int_cst (size_type_node, 0) over
>> size_zero_node. Is there a reason to this?
>
>Yes. AFAIU size_zero_node is a zero constant for sizetype which is not
>the same as size_type_node.
>
>AFAIK the difference is that size_type_node is the C size_t type,
>whereas sizetype is a GCC internal type used for address expressions.
>On a "normal" target I understand that they are the same size, but
>there are some slight differences in semantics, e.g. size_type_node
>like C unsigned integer types is defined to wrap around on overflow
>whereas sizetype is undefined on overflow.
>
>I don't know if GCC supports some strange targets with some kind of
>segmented memory where the size of sizetype would be different from
>size_type_node, but I guess it's possible in theory at least.
>
>That being said, now that you mention in I should be using
>build_zero_cst (some_type_node) instead of
>build_int_cst(some_type_node, 0). There's also build_one_cst that I
>should use.
>
>> Furthermore did I have to patch this:
>>
>> diff --git a/gcc/fortran/dump-parse-tree.c
>b/gcc/fortran/dump-parse-tree.c
>> index 585f25d..f374558 100644
>> --- a/gcc/fortran/dump-parse-tree.c
>> +++ b/gcc/fortran/dump-parse-tree.c
>> @@ -465,7 +465,7 @@ show_expr (gfc_expr *p)
>>           break;
>>
>>         case BT_HOLLERITH:
>> -         fprintf (dumpfile, "%dH", p->representation.length);
>> +         fprintf (dumpfile, "%zdH", p->representation.length);
>>           c = p->representation.string;
>>           for (i = 0; i < p->representation.length; i++, c++)
>>             {
>>
>> to bootstrap on x86_64-linux/f23.
>
>Ah, thanks for the catch. I'll fix it by using HOST_WIDE_INT_PRINT_DEC
>since I'll have to change gfc_charlen_t to be a typedef form
>HOST_WIDE_INT (see my answer to FX).
>
>> And I have this regression:
>>
>> FAIL: gfortran.dg/allocate_deferred_char_scalar_1.f03   -O1  (test
>for excess
>> errors)
>>
>> allocate_deferred_char_scalar_1.f03:184:0:
>>
>>      p = '12345679'
>>
>> Warning: '__builtin_memcpy' writing 8 bytes into a region of size 5
>overflows
>>      the destination [-Wstringop-overflow=]
>>      allocate_deferred_char_scalar_1.f03:242:0:
>>
>>      p = 4_'12345679'
>>
>> Warning: '__builtin_memcpy' writing 32 bytes into a region of size 20
>overflows
>> the destination [-Wstringop-overflow=]
>
>I'm seeing that too, but I assumed they would be fixed by Paul's
>recent patch which I don't yet have in my tree yet due to the git
>mirror being stuck..
>
>> Btw, the patch for changing the ABI of the coarray-libs is already
>nearly done.
>> I just need to figure that what the state of regressions is with and
>without my
>> change.
>
>Thanks.
>
>I'll produce an updated patch with the changes discussed so far.