Hello,

Le 01/08/2024 à 10:53, Jakub Jelinek a écrit :
Hi!

A static analyzer found a pasto in gfc_get_array_descr_info.
The code does
       t = base_decl;
       if (!integer_zerop (dtype_off))
         t = fold_build_pointer_plus (t, dtype_off);
       dtype = TYPE_MAIN_VARIANT (get_dtype_type_node ());
       field = gfc_advance_chain (TYPE_FIELDS (dtype), GFC_DTYPE_RANK);
       rank_off = byte_position (field);
       if (!integer_zerop (dtype_off))
         t = fold_build_pointer_plus (t, rank_off);
i.e. uses the same !integer_zerop check between both, while it should
be checking rank_off in the latter case.
This actually doesn't change anything on the generated code, because
both the dtype_off and rank_off aren't zero,
typedef struct dtype_type
{
   size_t elem_len;
   int version;
   signed char rank;
   signed char type;
   signed short attribute;
}
dtype_type;
struct {
   type *base_addr;
   size_t offset;
   dtype_type dtype;
   index_type span;
   descriptor_dimension dim[];
};
dtype_off is 16 on 64-bit arches and 8 on 32-bit ones and rank_off is
12 on 64-bit arches and 8 on 32-bit arches, so this patch is just to
pacify those static analyzers or be prepared if the ABI changes in the
future.
Though, as we know that the offsets currently aren't zero, checking for
!integer_zerop isn't actually an optimization but slow things down,
fold_build_pointer_plus will handle += 0 as well, so maybe we should just
drop it for the cases where we know it isn't 0.

Yes, I've always wondered how much of a win these integer_zerop checks were, probably not that much. In the cases we know they are useless, let's remove them (patch pre-approved for gfc_get_array_descr_info).

Anyway, the following patch just does the minimal change,
bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?

Looks good, but as said removing the check seems preferable.

2024-08-01  Jakub Jelinek  <ja...@redhat.com>

        * trans-types.cc (gfc_get_array_descr_info): Add rank_off to t
        if rank_off isn't zero rather than dtype_off.

--- gcc/fortran/trans-types.cc.jj       2024-07-19 17:22:59.405097054 +0200
+++ gcc/fortran/trans-types.cc  2024-07-31 20:48:20.546569993 +0200
@@ -3605,7 +3605,7 @@ gfc_get_array_descr_info (const_tree typ
        dtype = TYPE_MAIN_VARIANT (get_dtype_type_node ());
        field = gfc_advance_chain (TYPE_FIELDS (dtype), GFC_DTYPE_RANK);
        rank_off = byte_position (field);
-      if (!integer_zerop (dtype_off))
+      if (!integer_zerop (rank_off))
        t = fold_build_pointer_plus (t, rank_off);
t = build1 (NOP_EXPR, build_pointer_type (TREE_TYPE (field)), t);

        Jakub


Reply via email to