This is the mail archive of the gcc@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

expand_builtin_memcpy bug exposed by TER and gfortran


I've been investigating a testsuite failure which shows up with the new
TER implementation, and it appears to be a bug in the code for
expand_builtin_memcpy.  

The problem is an array slice of a string constant, which is this case
originates in gfortran.

The source code looks like:

subroutine foo(i,j)
   integer i,j
   character (len=4) st4
   st4 (i, j) = "0123456789"(i:j)
   if (st4 (1, 4) .ne. "0123" ) call abort
 end subroutine


The old TER implementation didn't eliminate the temporary for st4, and
produced:

   D.1000 = (char[1:] *) &"0123456789"[1]{lb: 1 sz: 1};
   __builtin_memcpy (&st4.2, D.1000, 4);

and everything worked fine.

The new TER implementation eliminates the intermediate value and
produces:

  __builtin_memcpy (&st4.2, (char[1:] *) &"0123456789"[1]{lb: 1 sz: 1},
4);

which fails.

The problem happens when expand_builtin_memcpy() recognizes the SRC to
be a string constant and calls c_getstr () to extract the constant.  The
result is the string "1234" because the code doesn't appear to take into
account that the expression has a lower bound that is not 0.

c_getstr() calls 
  src = string_constant (src, &offset_node);
which returns an offset_node value of '1', which is simply calculated
as:

else if (TREE_CODE (TREE_OPERAND (arg, 0)) == ARRAY_REF)
        {
          array = TREE_OPERAND (TREE_OPERAND (arg, 0), 0);
          offset = TREE_OPERAND (TREE_OPERAND (arg, 0), 1);
          if (TREE_CODE (array) != STRING_CST
              && TREE_CODE (array) != VAR_DECL)
            return 0;
         }

this is where offset is calculated, and it makes no attempt to see if
the lower bound is something other than 0.

So it looks like there ought to be a check in here for
array_ref_low_bound (arg) not equal to zero, and some sort of
calculation of the proper offset. I don't think array_ref_low_bound()
has to be a constant so the fix isn't immediately obvious to me for that
case. 

My preference is to check in the TER code which exposes this bug, and
open a PR against the failure with this info.  That way we don't lose
track of the problem, and someone can fix it at their leisure. Until
then there will be a testsuite failure in gfortran for the testcase
which triggers this.

Does that seem reasonable? or would everyone prefer I get it fixed
before checking in the TER code?

The TER code bootstraps fine and only appears to affect this one
testcase (which causes 7 failures since its compiled with 7 different
options).

Andrew












Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]