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/22/2018 04:47 PM, Martin Sebor wrote:
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.

The WG14 convener confirmed that this is indeed undefined.
The words that apply to this case (as well as all the others)
are in 6.7.9, p2:

  No initializer shall attempt to provide a value for an object
  not contained within the entity being initialized.

If GCC wants to make this well-defined and guarantee that
the excess elements are stripped (I'm not opposed to it)
we need to treat it should be treated as an enhancement
request, so the feature can be documented and properly
tested (I'm not opposed to it but I'm not going to champion
it either.)  Otherwise, I see no reason for a change.

Martin


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