This is the mail archive of the gcc-patches@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]

Re: [PATCH] fix a couple of bugs in const string folding (PR 86532)


On 07/18/2018 02:31 AM, Richard Biener wrote:
On Tue, 17 Jul 2018, Martin Sebor wrote:

The attached update takes care of a couple of problems pointed
out by Bernd Edlinger in his comments on the bug.  The ICE he
mentioned in comment #20 was due mixing sizetype, ssizetype,
and size_type_node in c_strlen().  AFAICS, some of it predates
the patch but my changes made it worse and also managed trigger
it.

+        has no internal zero bytes.  If the offset falls within the
bounds
+        of the string subtract the offset from the length of the string,
+        and return that.  Otherwise the length is zero.  Take care to
+        use SAVE_EXPR in case the OFFSET has side-effects.  */
+      tree offsave = TREE_SIDE_EFFECTS (byteoff) ? save_expr (byteoff) :
byteoff;
+      offsave = fold_convert (ssizetype, offsave);
+      tree condexp = fold_build2_loc (loc, LE_EXPR, boolean_type_node,
offsave,
+                                     build_int_cst (ssizetype, len *
eltsize));
+      tree lenexp = size_diffop_loc (loc, ssize_int (strelts * eltsize),
offsave);
+      return fold_build3_loc (loc, COND_EXPR, ssizetype, condexp, lenexp,
+                             build_zero_cst (ssizetype));

in what case are you expecting to return an actual COND_EXRP and
why is that useful?

It's necessary to correctly handle strings with multiple trailing
nuls, like in:

  const char a[8] = "123";
  int f (int i)
  {
    return strlen (a + i);
  }

If (i <= 3) then the length is i.  If it's greater than 3 then
the length is zero.  I'd expect such strings to be quite common,
even pervasive, in the case of multidimensional arrays or arrays
of structs with array members.  (Probably less so in plain one-
dimensional arrays like the one above.)

You return a signed value but bother to
guard it so it is never less than zero.  Why?  Why not simply
return the difference as you did before but with the side-effects
properly handled?

Hopefully the above answers this question (if there's a way
to do it in a more straightforward way please let me know).

FWIW, as I said in bug 86434, I think this folding is premature
and prevents other optimizations that I suspect would be more
profitable.  I'm only preserving it here for now but at some
point I hope we can agree to defer it until later when more
information about the offset is known and when it will
benefit other optimizations.  I read your comments on the bug
and I'll see if it's possible to have it both ways.

Martin


On 07/17/2018 09:19 AM, Martin Sebor wrote:
My enhancement to extract constant strings out of complex
aggregates committed last week introduced a couple of bugs in
dealing with non-constant indices and offsets.  One of the bugs
was fixed earlier today (PR 86528) but another one remains.  It
causes strlen (among other functions) to incorrectly fold
expressions involving a non-constant index into an array of
strings by treating the index the same as a non-consatnt
offset into it.

The non-constant index should either prevent the folding, or it
needs to handle it differently from an offset.

The attached patch takes the conservative approach of avoiding
the folding in this case.  The remaining changes deal with
the fallout from the fix.

Tested on x86_64-linux.

Martin






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