On 07/22/2018 02:00 AM, Bernd Edlinger wrote:
On 07/21/18 00:15, Martin Sebor wrote:
On 07/20/2018 08:51 AM, Bernd Edlinger wrote:
Martin,
when I look at tree-ssa-forwprop.c:
str1 = string_constant (src1, &off1);
if (str1 == NULL_TREE)
break;
if (!tree_fits_uhwi_p (off1)
|| compare_tree_int (off1, TREE_STRING_LENGTH (str1) - 1) > 0
|| compare_tree_int (len1, TREE_STRING_LENGTH (str1)
- tree_to_uhwi (off1)) > 0
|| TREE_CODE (TREE_TYPE (str1)) != ARRAY_TYPE
|| TYPE_MODE (TREE_TYPE (TREE_TYPE (str1)))
!= TYPE_MODE (char_type_node))
break;
I don't think it is a good idea to let string_constant return
strings which have not necessarily valid content after the first
NUL byte. It looks like the content has to be valid up to
TREE_STRING_LENGTH.
I'm not sure I understand when this could happen. From the patch
below it sounds like you are concerned about cases like:
const char a[4] = "abc\000\000";
where the STRING_CST is bigger than the array. But the string
constant has valid content here up to TREE_STRING_LENGTH. Am
I missing something? (A test case would be helpful.)
No, I mean something like:
$ cat y.c
const char a[2][3] = { "1234", "xyz" };
char b[6];
int main ()
{
__builtin_memcpy(b, a, 4);
__builtin_memset(b + 4, 'a', 2);
__builtin_printf("%.6s\n", b);
}
$ gcc y.c
y.c:1:24: warning: initializer-string for array of chars is too long
const char a[2][3] = { "1234", "xyz" };
^~~~~~
y.c:1:24: note: (near initialization for 'a[0]')
$ ./a.out
1234aa
but expected would be "123xaa".
Hmm. I assumed this was undefined in C but after double
checking I'm not sure. If it's in fact valid and the excess
elements are required to be ignored I'll of course fix it in
a subsequent patch. Let me find out.
In any case, unless the concern is specifically related to
this bug fix let's discuss it separately so I can fix this
bug. I'm not opposed to making further changes here (in
fact, I have one in the queue and two others that I raised
in Bugzilla in response to our discussion so far), I just
want to avoid mission creep.
I think when you touch a function you should fix it completely
or restrict it to the subset that is known to be safe, and not
leave lots of already known bugs behind.
I find it preferable to open a new bug for separate problems
and tackle each on its own. That makes it possible to track
separate bugs independently, make sure each fix is adequately
tested, and backport patches to release branches as necessary.
Wholesale changes tend to make this more difficult.
If the code in your test case is in fact well-defined then
it's especially important that a test case be added for it
because none exists yet, and opening a separate bug makes
sure this happens.
Martin
