[PATCH] Check the STRING_CSTs in varasm.c

[Richard Biener]

On Sat, 4 Aug 2018, Bernd Edlinger wrote:

> On 08/03/18 23:36, Jeff Law wrote:
> > On 08/01/2018 05:35 AM, Bernd Edlinger wrote:
> >> Hi,
> >>
> >> this completes the previous patches, and adds a check in varasm.c
> >> that ensures that all string constants are NUL terminated,
> >> And that varasm does not strip anything but _exactly_ one NUL
> >> character.
> >>
> >> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> >> Is it OK for trunk?
> >>
> >>
> >> Thanks
> >> Bernd.
> >>
> >>
> >> patch-varasm.diff
> >>
> >>
> >> 2018-08-01  Bernd Edlinger  <bernd.edlinger@hotmail.de>
> >>
> >> 	* varasm.c (check_string_literal): New checking function.
> >> 	(output_constant): Use it.
> > My biggest concern here is that we've explicitly defined STRING_CST
> > nodes as not needing NUL termination.  See generic.texi.  That explicit
> > definition is almost certainly derived from existing practice that long
> > pre-dates the introduction of gimple/generic.
> > 
> > Even so I'm generally OK with the concept of tightening the rules here.
> > If we need to fault in more fixes, that's fine with me.  I'm assuming
> > that you and Ian have sorted out what to do WRT Go.
> > 
> > If we decide to go forward, you'll definitely want to update the
> > STRING_CST documentation in generic.texi.
> > 
> 
> This is the same patch with STRING_CST documentation updated.

+embedded @code{NUL} characters.  However, the
+@code{TREE_STRING_LENGTH} always includes a trailing @code{NUL} that
+is not part of the language string literal but appended by the front end.
+If the string shall not be @code{NUL}-terminated the @code{TREE_TYPE}
+is one character shorter than @code{TREE_STRING_LENGTH}.
+Excess caracters other than one trailing @code{NUL} character are not
+permitted.

I find this very confusing and oppose to that change.  Can we get
back to the drawing board please?  If we want an easy way to
see whether a string is "properly" terminated then maybe we can
simply use a flag that gets set by build_string?

Richard.