Possible bug re: ISO_Fortran_binding.h

Possible bug re: ISO_Fortran_binding.h

[Harris Snyder]

Hi everyone,

I've discovered a possible bug in libgfortran (regarding
ISO_Fortran_binding.h), but I'd like confirmation from the community
that this is indeed a bug before I work on a patch.

In the f2018 standard, the description of the argument `elem_len` to
`CFI_establish` reads:

`element_len` is ignored unless `type` is equal to `CFI_type_struct`,
`CFI_type_other`, or a Fortran character type code, in which case
`element_len` must be greater than zero and equal to the storage size
in bytes of an element of the object.

However in libgfortran, (as I discovered the hard way),
`CFI_type_signed_char`  and `CFI_type_int8_t` have the same numeric
representation. As a result, if you try to create an array of int8_t
without specifying the element_len as 1, you get seg faults or
floating point exceptions because it thinks the element size is zero.

The standard doesn't appear to say that these should be equivalent,
and it feels to me like a bug. Possible solutions:
- change the overall logic of how the numeric constants are mapped to types.
- treat int8_t as a special case and explicitly check for it when
converting from the standard specified array descriptor to the
gfortran array descriptor
- ..?

This is my first post to any GCC list, so I apologize if I've
miscategorized this message - feel free to correct me regarding
protocol.

Harris Snyder

Re: Possible bug re: ISO_Fortran_binding.h

[Tobias Burnus]

Hi Harris,

On 10.01.21 23:17, Harris Snyder wrote:
> `element_len` is ignored unless `type` is equal to `CFI_type_struct`,
> `CFI_type_other`, or a Fortran character type code, in which case
> `element_len` must be greater than zero and equal to the storage size
> in bytes of an element of the object.
>
> However in libgfortran, (as I discovered the hard way),
> `CFI_type_signed_char`  and `CFI_type_int8_t` have the same numeric
> representation.

The problem is that in C, 'char' is an integer type – but also used for characters / for strings.

The Fortran 2018 standard states in
"18.2.2  Named constants and derived types in the module":

"The values of ... C_SIGNED_CHAR, ... shall each be a valid value
for an integer kind type parameter ..."

And:
"The value of C_CHAR shall be a valid value for a character kind type parameter ..."

Thus, this part looks fine: If you want to use a string, use 'C_CHAR',
if you want to have an integer, use 'C_SIGNED_CHAR'.

Admittedly, it is not quite clearly stated in
"18.5.4  Macros and typedefs in ISO_Fortran_binding.h"
how those map to CFI_type_… but the assumption that
C_CHAR maps to CFI_type_char and is for characters types
and C_SIGNED_CHAR maps to CFI_type_signed_char and is for
integer types seems to be sensible.

If you look at
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgfortran/ISO_Fortran_binding.h
that matches what gfortran has: CFI_type_char is a character type
while CFI_type_signed_character is an integer type.

Thus, the type handling itself seems to be fine → no bug.

> As a result, if you try to create an array of int8_t
> without specifying the element_len as 1, you get seg faults or
> floating point exceptions because it thinks the element size is zero.

This might be a bug. It is not quite clear to me whether you want to
use a string or an integer.

 From the wording, it sounds as if you have integers – and it fails
because (lib)gfortran treats it as string when it shouldn't and fails
because of elem_len.

This might be an actual bug. It would help tremendously if you could
send use an example.

Cheers,

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Re: Possible bug re: ISO_Fortran_binding.h

[Harris Snyder]

Hi Tobias,

Good points, thanks. Understood regarding signed char being an integer
type. I was trying to create an array of one byte integers, nothing
string-related.

Here's a minimal example to reproduce the potential bug:

fortranpart.f90

module m
   use iso_c_binding
contains
   subroutine fsub( x ) bind(C, name="fsub")
      integer(c_int8_t), intent(inout) :: x(:)
      x = x+1
   end subroutine
end module

cpart.c

#include <stdlib.h>
#include <stdio.h>
#include <inttypes.h>
#include "ISO_Fortran_binding.h"
extern void fsub(CFI_cdesc_t *);

int main(void)
{
   int N = 4;
   int8_t x[N];
   CFI_CDESC_T(1) dat;
   CFI_index_t ext[1];
   ext[0] = (CFI_index_t)N;

   int rc = CFI_establish((CFI_cdesc_t *)&dat, x, CFI_attribute_other,
                      CFI_type_int8_t, 0, (CFI_rank_t)1, ext);

   printf("CFI_establish call returned: %d\n", rc);
   fsub((CFI_cdesc_t *)&dat );
   for (int i=0; i<N-1; i++) printf("%"PRId8", ", x[i]);
printf("%"PRId8"\n", x[N-1]);
   return 0;
}


On Ubuntu 20.10 with gcc 10.2.0, this prints:
CFI_establish call returned: 0
Floating point exception (core dumped)

As for the site of the issue,
libgfortran/runtime/ISO_Fortran_binding.c, circa line 348:

  if (type == CFI_type_char || type == CFI_type_ucs4_char ||
      type == CFI_type_signed_char || type == CFI_type_struct ||
      type == CFI_type_other)
    dv->elem_len = elem_len;
  else
...

So we're clearly using elem_len for signed char types and therefore
int8_t. In light of your comments above about how signed char is
supposed to be an integer type, what about simply removing
CFI_type_signed_char from the alternates in the if statement?

Harris

On Mon, Jan 11, 2021 at 6:24 AM Tobias Burnus <tobias@codesourcery.com> wrote:
>
> Hi Harris,
>
> On 10.01.21 23:17, Harris Snyder wrote:
> > `element_len` is ignored unless `type` is equal to `CFI_type_struct`,
> > `CFI_type_other`, or a Fortran character type code, in which case
> > `element_len` must be greater than zero and equal to the storage size
> > in bytes of an element of the object.
> >
> > However in libgfortran, (as I discovered the hard way),
> > `CFI_type_signed_char`  and `CFI_type_int8_t` have the same numeric
> > representation.
>
> The problem is that in C, 'char' is an integer type – but also used for characters / for strings.
>
> The Fortran 2018 standard states in
> "18.2.2  Named constants and derived types in the module":
>
> "The values of ... C_SIGNED_CHAR, ... shall each be a valid value
> for an integer kind type parameter ..."
>
> And:
> "The value of C_CHAR shall be a valid value for a character kind type parameter ..."
>
> Thus, this part looks fine: If you want to use a string, use 'C_CHAR',
> if you want to have an integer, use 'C_SIGNED_CHAR'.
>
> Admittedly, it is not quite clearly stated in
> "18.5.4  Macros and typedefs in ISO_Fortran_binding.h"
> how those map to CFI_type_… but the assumption that
> C_CHAR maps to CFI_type_char and is for characters types
> and C_SIGNED_CHAR maps to CFI_type_signed_char and is for
> integer types seems to be sensible.
>
> If you look at
> https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libgfortran/ISO_Fortran_binding.h
> that matches what gfortran has: CFI_type_char is a character type
> while CFI_type_signed_character is an integer type.
>
> Thus, the type handling itself seems to be fine → no bug.
>
> > As a result, if you try to create an array of int8_t
> > without specifying the element_len as 1, you get seg faults or
> > floating point exceptions because it thinks the element size is zero.
>
> This might be a bug. It is not quite clear to me whether you want to
> use a string or an integer.
>
>  From the wording, it sounds as if you have integers – and it fails
> because (lib)gfortran treats it as string when it shouldn't and fails
> because of elem_len.
>
> This might be an actual bug. It would help tremendously if you could
> send use an example.
>
> Cheers,
>
> Tobias
>
> -----------------
> Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
> Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Re: Possible bug re: ISO_Fortran_binding.h

[Tobias Burnus]

Hi Harris,

On 11.01.21 16:42, Harris Snyder wrote:
> As for the site of the issue,
> libgfortran/runtime/ISO_Fortran_binding.c, circa line 348:
>
>    if (type == CFI_type_char || type == CFI_type_ucs4_char ||
>        type == CFI_type_signed_char || type == CFI_type_struct ||
>        type == CFI_type_other)
>      dv->elem_len = elem_len;
> [...]
> So we're clearly using elem_len for signed char types and therefore
> int8_t. In light of your comments above about how signed char is
> supposed to be an integer type, what about simply removing
> CFI_type_signed_char from the alternates in the if statement?
Seems as if you found the fix for issue yourself.

I added a link to this thread to https://gcc.gnu.org/PR93524

Thanks for the testcase and digging.

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

Re: Possible bug re: ISO_Fortran_binding.h

[Harris Snyder]

Hi Tobias,

No problem! Would it be appropriate for me to submit a patch for this?
It's only a small part of the bug you linked (I think), but worth
fixing in it's own right IMO.

Harris

Re: Possible bug re: ISO_Fortran_binding.h

[Tobias Burnus]

Hi Harris,

On 11.01.21 19:18, Harris Snyder wrote:
> No problem! Would it be appropriate for me to submit a patch for this?
> It's only a small part of the bug you linked (I think), but worth
> fixing in it's own right IMO.

Yes, that's definitely something which should be fixed soon.

Thus, you could either wait a while – or as suggested,
you could also submit a patch for it.

If so:

* Sent it both to fortran@ and gcc-patches@.
* 8 spaces are replaced by a 'tab'; more coding style info:
   https://gcc.gnu.org/codingconventions.html
* Include "PR fortran/93524" in the commit log
  (see "git log" for the format;
   "git diff | ./contrib/mklog.py" generates a template
   coding convention also mention it; the ChangeLog file
   is autogenerated from the commit log)
* Add a testcase – https://gcc.gnu.org/wiki/TestCaseWriting
   gives some guide line (or look at the existing ones at
   gcc/testcases/gfortran.dg/)
* Run the testsuite ("check-fortran" is sufficient in this case,
   tip: first use RUNTESTFLAGS to check that your testcase works
   and read the item about "dg-additional-sources".)

* In principle, a copyright assignment to the FSF is needed, cf.
     https://gcc.gnu.org/contribute.html
   but one single-line patch counts as obvious.
   But in case you want to contribute in the future, you may
   consider it ...

* If you have not yet build GCC:
   GIT: https://gcc.gnu.org/git.html
   https://gcc.gnu.org/install/
   - build into an empty directory
   - if you have an old system or no root access,
     ./contrib/download_prerequisites
     downloads some aux libraries, alternatively,
     install their -dev/-devel in your distro

If you have more questions, feel free to ask.

Thanks already for the report and finding the issue,

Tobias

-----------------
Mentor Graphics (Deutschland) GmbH, Arnulfstraße 201, 80634 München / Germany
Registergericht München HRB 106955, Geschäftsführer: Thomas Heurung, Alexander Walter

[PATCH, Fortran] Bug fix in ISO_Fortran_binding - unsigned char arrays

[Harris Snyder]

Hi everyone,

In a previous email thread (subject: Possible bug re:
ISO_Fortran_binding.h) it was determined that there was a bug in
ISO_Fortran_binding.c. When attempting to use the ISO Fortran binding
from C to create an array of type uint8_t or signed char, a crash
would result unless the element size was manually specified as 1 when
calling CFI_establish. The F2018 standard indicates that signed char /
uint8_t should be an integer type and therefore explicitly specifying
the element size should not be required. The attached patch fixes the
issue (test case included).

OK for master? I don't have write access so I will need someone else
to commit this for me if possible.

Thanks,
Harris Snyder


Fixes a bug in ISO_Fortran_binding.c whereby signed char or uint8_t
arrays would cause crashes unless an element size is specified.

libgfortran/ChangeLog:

    * runtime/ISO_Fortran_binding.c (CFI_establish): fixed signed char arrays.

gcc/testsuite/ChangeLog:

    * gfortran.dg/iso_fortran_binding_uint8_array.f90: New test.
    * gfortran.dg/iso_fortran_binding_uint8_array_driver.c: New test.


diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
new file mode 100644
index 00000000000..bf85bf52949
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
@@ -0,0 +1,11 @@
+! { dg-do run }
+! { dg-additional-sources iso_fortran_binding_uint8_array_driver.c }
+
+module m
+   use iso_c_binding
+contains
+   subroutine fsub( x ) bind(C, name="fsub")
+      integer(c_int8_t), intent(inout) :: x(:)
+      x = x+1
+   end subroutine
+end module
diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
new file mode 100644
index 00000000000..84ed127d525
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <inttypes.h>
+#include "ISO_Fortran_binding.h"
+
+extern void fsub(CFI_cdesc_t *);
+
+int main(void)
+{
+   int8_t x[] = {1,2,3,4};
+   int N = sizeof(x)/sizeof(x[0]);
+
+   CFI_CDESC_T(1) dat;
+   CFI_index_t ext[1];
+   ext[0] = (CFI_index_t)N;
+   int rc = CFI_establish((CFI_cdesc_t *)&dat, &x, CFI_attribute_other,
+                      CFI_type_int8_t, 0, (CFI_rank_t)1, ext);
+   printf("CFI_establish call returned: %d\n", rc);
+
+   fsub((CFI_cdesc_t *)&dat );
+
+   for (int i=0; i<N; i++)
+       printf("%"PRId8"\n", x[i]);
+   return 0;
+}
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
b/libgfortran/runtime/ISO_Fortran_binding.c
index 86ff25afb23..3746ec1c681 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -345,8 +345,7 @@ int CFI_establish (CFI_cdesc_t *dv, void
*base_addr, CFI_attribute_t attribute,
   dv->base_addr = base_addr;

   if (type == CFI_type_char || type == CFI_type_ucs4_char ||
-      type == CFI_type_signed_char || type == CFI_type_struct ||
-      type == CFI_type_other)
+      type == CFI_type_struct || type == CFI_type_other)
     dv->elem_len = elem_len;
   else
     {

Re: Bug fix in ISO_Fortran_binding - unsigned char arrays

[Harris Snyder]

Hi everyone,

Sorry, my previous email erroneously referred to unsigned chars / uint8_t, when I in fact meant signed chars / int8_t. The actual patch works, but the test case files have ‘unit’ in the file names. I’ll provide a corrected patch tomorrow to fix the file names. 

Harris

[PATCH, Fortran] PR fortran/93524 - ISO_Fortran_binding signed char arrays

[Harris Snyder]

On Wed, Jan 13, 2021 at 1:34 AM Harris Snyder <hsnyder@structura.bio> wrote:
>
> Hi everyone,
>
> Sorry, my previous email erroneously referred to unsigned chars / uint8_t,
> when I in fact meant signed chars / int8_t. The actual patch works, but the
> test case files have ‘uint’ in the file names. I’ll provide a corrected patch
> tomorrow to fix the file names.
>
> Harris

Corrected patch below.

OK for master? I don't have write access so I will need someone else
to commit this for me if possible.

Thanks,
Harris Snyder


Fixes a bug in ISO_Fortran_binding.c whereby signed char or int8_t
arrays would cause crashes unless an element size is specified.
Related to PR fortran/93524.

libgfortran/ChangeLog:

    * runtime/ISO_Fortran_binding.c (CFI_establish): fixed signed char arrays.

gcc/testsuite/ChangeLog:

    * gfortran.dg/iso_fortran_binding_int8_array.f90: New test.
    * gfortran.dg/iso_fortran_binding_int8_array_driver.c: New test.

diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
new file mode 100644
index 00000000000..31fdf863e0a
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
@@ -0,0 +1,11 @@
+! { dg-do run }
+! { dg-additional-sources iso_fortran_binding_int8_array_driver.c }
+
+module m
+   use iso_c_binding
+contains
+   subroutine fsub( x ) bind(C, name="fsub")
+      integer(c_int8_t), intent(inout) :: x(:)
+      x = x+1
+   end subroutine
+end module
diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
new file mode 100644
index 00000000000..84ed127d525
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
@@ -0,0 +1,25 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <inttypes.h>
+#include "ISO_Fortran_binding.h"
+
+extern void fsub(CFI_cdesc_t *);
+
+int main(void)
+{
+   int8_t x[] = {1,2,3,4};
+   int N = sizeof(x)/sizeof(x[0]);
+
+   CFI_CDESC_T(1) dat;
+   CFI_index_t ext[1];
+   ext[0] = (CFI_index_t)N;
+   int rc = CFI_establish((CFI_cdesc_t *)&dat, &x, CFI_attribute_other,
+                      CFI_type_int8_t, 0, (CFI_rank_t)1, ext);
+   printf("CFI_establish call returned: %d\n", rc);
+
+   fsub((CFI_cdesc_t *)&dat );
+
+   for (int i=0; i<N; i++)
+       printf("%"PRId8"\n", x[i]);
+   return 0;
+}
diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
deleted file mode 100644
index e69de29bb2d..00000000000
diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
deleted file mode 100644
index e69de29bb2d..00000000000
diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
b/libgfortran/runtime/ISO_Fortran_binding.c
index 86ff25afb23..3746ec1c681 100644
--- a/libgfortran/runtime/ISO_Fortran_binding.c
+++ b/libgfortran/runtime/ISO_Fortran_binding.c
@@ -345,8 +345,7 @@ int CFI_establish (CFI_cdesc_t *dv, void
*base_addr, CFI_attribute_t attribute,
   dv->base_addr = base_addr;

   if (type == CFI_type_char || type == CFI_type_ucs4_char ||
-      type == CFI_type_signed_char || type == CFI_type_struct ||
-      type == CFI_type_other)
+      type == CFI_type_struct || type == CFI_type_other)
     dv->elem_len = elem_len;
   else
     {

Re: [PATCH, Fortran] PR fortran/93524 - ISO_Fortran_binding signed char arrays

[Jerry DeLisle]

Looks good and I have tested. I will commit after one more double check 
for regressions with the test case in the testsuite

I have been following Harris on this one for several days and tested the 
patch before submitted here.

Thanks for patch Harris, much appreciated.

Regards,

Jerry

On 1/13/21 8:02 AM, Harris Snyder wrote:
> On Wed, Jan 13, 2021 at 1:34 AM Harris Snyder <hsnyder@structura.bio> wrote:
>> Hi everyone,
>>
>> Sorry, my previous email erroneously referred to unsigned chars / uint8_t,
>> when I in fact meant signed chars / int8_t. The actual patch works, but the
>> test case files have ‘uint’ in the file names. I’ll provide a corrected patch
>> tomorrow to fix the file names.
>>
>> Harris
> Corrected patch below.
>
> OK for master? I don't have write access so I will need someone else
> to commit this for me if possible.
>
> Thanks,
> Harris Snyder
>
>
> Fixes a bug in ISO_Fortran_binding.c whereby signed char or int8_t
> arrays would cause crashes unless an element size is specified.
> Related to PR fortran/93524.
>
> libgfortran/ChangeLog:
>
>      * runtime/ISO_Fortran_binding.c (CFI_establish): fixed signed char arrays.
>
> gcc/testsuite/ChangeLog:
>
>      * gfortran.dg/iso_fortran_binding_int8_array.f90: New test.
>      * gfortran.dg/iso_fortran_binding_int8_array_driver.c: New test.
>
> diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
> b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
> new file mode 100644
> index 00000000000..31fdf863e0a
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array.f90
> @@ -0,0 +1,11 @@
> +! { dg-do run }
> +! { dg-additional-sources iso_fortran_binding_int8_array_driver.c }
> +
> +module m
> +   use iso_c_binding
> +contains
> +   subroutine fsub( x ) bind(C, name="fsub")
> +      integer(c_int8_t), intent(inout) :: x(:)
> +      x = x+1
> +   end subroutine
> +end module
> diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
> b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
> new file mode 100644
> index 00000000000..84ed127d525
> --- /dev/null
> +++ b/gcc/testsuite/gfortran.dg/iso_fortran_binding_int8_array_driver.c
> @@ -0,0 +1,25 @@
> +#include <stdlib.h>
> +#include <stdio.h>
> +#include <inttypes.h>
> +#include "ISO_Fortran_binding.h"
> +
> +extern void fsub(CFI_cdesc_t *);
> +
> +int main(void)
> +{
> +   int8_t x[] = {1,2,3,4};
> +   int N = sizeof(x)/sizeof(x[0]);
> +
> +   CFI_CDESC_T(1) dat;
> +   CFI_index_t ext[1];
> +   ext[0] = (CFI_index_t)N;
> +   int rc = CFI_establish((CFI_cdesc_t *)&dat, &x, CFI_attribute_other,
> +                      CFI_type_int8_t, 0, (CFI_rank_t)1, ext);
> +   printf("CFI_establish call returned: %d\n", rc);
> +
> +   fsub((CFI_cdesc_t *)&dat );
> +
> +   for (int i=0; i<N; i++)
> +       printf("%"PRId8"\n", x[i]);
> +   return 0;
> +}
> diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
> b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array.f90
> deleted file mode 100644
> index e69de29bb2d..00000000000
> diff --git a/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
> b/gcc/testsuite/gfortran.dg/iso_fortran_binding_uint8_array_driver.c
> deleted file mode 100644
> index e69de29bb2d..00000000000
> diff --git a/libgfortran/runtime/ISO_Fortran_binding.c
> b/libgfortran/runtime/ISO_Fortran_binding.c
> index 86ff25afb23..3746ec1c681 100644
> --- a/libgfortran/runtime/ISO_Fortran_binding.c
> +++ b/libgfortran/runtime/ISO_Fortran_binding.c
> @@ -345,8 +345,7 @@ int CFI_establish (CFI_cdesc_t *dv, void
> *base_addr, CFI_attribute_t attribute,
>     dv->base_addr = base_addr;
>
>     if (type == CFI_type_char || type == CFI_type_ucs4_char ||
> -      type == CFI_type_signed_char || type == CFI_type_struct ||
> -      type == CFI_type_other)
> +      type == CFI_type_struct || type == CFI_type_other)
>       dv->elem_len = elem_len;
>     else
>       {