Hi Bernd, > On 31 Jul 2018, at 14:07, Bernd Edlinger <bernd.edlin...@hotmail.de> wrote: > > Hi! > > > This fixes a couple STRING_CST that are not explicitly NUL terminated. > These were caught in a new check in varasm.c I am currently working on. > > Having a NUL terminated string does not change the binary output, but it > makes things easier for he middle-end. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk?
--- gcc/ada/gcc-interface/utils2.c 2017-12-21 07:57:41.000000000 +0100
+++ gcc/ada/gcc-interface/utils2.c 2018-07-31 11:44:01.517117923 +0200
@@ -1844,7 +1844,7 @@ expand_sloc (Node_Id gnat_node, tree *fi
}
const int len = strlen (str);
- *filename = build_string (len, str);
+ *filename = build_string (len + 1, str);
TREE_TYPE (*filename) = build_array_type (char_type_node,
build_index_type (size_int (len)));
*line = build_int_cst (NULL_TREE, line_number);
This one looks good to me. I'm not officially listed as a maintainer
so I'll let Eric have the final word. I'm answering because ...
--- gcc/ada/gcc-interface/trans.c 2018-07-17 10:10:04.000000000 +0200
+++ gcc/ada/gcc-interface/trans.c 2018-07-31 11:16:27.350728886 +0200
@@ -6079,7 +6079,7 @@ gnat_to_gnu (Node_Id gnat_node)
where GCC will want to treat it as a C string. */
string[i] = 0;
- gnu_result = build_string (length, string);
+ gnu_result = build_string (length + 1, string);
/* Strings in GCC don't normally have types, but we want
this to not be converted to the array type. */
We have a local variant of this one, on which I worked after we realized
that it was not enough to achieve the intended effect.
The difference at this spot is simply that we prevent the +1 if the
original string happens to be explicitly nul terminated already. Like:
build_string
((length > 0 && string[length-1] == 0) ? length : length + 1,
string);
This is however not enough because the extra zero is only conveyed
through the string value, not the corresponding type, and
varasm.c: mergeable_string_section
currently uses the type bounds to search for a zero termination.
We can't really change the type, so came up with an adjustment to
varasm. The motivation for using the type bounds was
https://gcc.gnu.org/ml/gcc-patches/2006-06/msg01004.html
which, IIUC, was tightly linked to string constants used as
initializers for objects which have a size of their own.
(Jakub cc'ed)
For string constants not used as initializers, it seems that
we might be able to use the string bounds instead, possibly
wider. The attached patch does that and passed testing a few weeks
ago.
So, while we're at it, does that look right, in light of all the
string length related issues that have been discussed lately ?
I'm not sure I grasped all the ramifications.
Thanks in advance!
* varasm.c (mergeable_string_section): Accept an extra SCAT
argument conveying the section category from which the request
originates. Only restrict the search to the string type bounds
if we're queried for an initializer. Consider the string bounds
otherwise.
(default_elf_select_section, STR and STR_INIT): Pass the section
category to mergeable_string_section.
ada-string.diff
Description: Binary data
