On Fri, 3 Aug 2018, 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.edlin...@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.
Note that I'm a little bit confused here given build_string
already appends a '\0' after TREE_STRING_LENGTH. So it is safe
to call strlen() on all STRING_CSTs. I think I raised this in
the review of one of Bernds patches but to be honest all the
various threads have become a bit convoluted (esp. if joining
in after two weeks of vacation).
So -- why is what we do currently not enough?
Quoting our single STRING_CST creator:
tree
build_string (int len, const char *str)
{
tree s;
size_t length;
/* Do not waste bytes provided by padding of struct tree_string. */
length = len + offsetof (struct tree_string, str) + 1;
record_node_allocation_statistics (STRING_CST, length);
s = (tree) ggc_internal_alloc (length);
memset (s, 0, sizeof (struct tree_typed));
TREE_SET_CODE (s, STRING_CST);
TREE_CONSTANT (s) = 1;
TREE_STRING_LENGTH (s) = len;
memcpy (s->string.str, str, len);
s->string.str[len] = '\0';
return s;
}
Richard.
