On Sun, 2018-08-05 at 16:59 +0000, Bernd Edlinger wrote: > Hi! > > > My other patch with adds assertions to varasm.c regarding correct > nul termination of sting literals did make these incorrect string > constants in JIT frontend fail. > > The string constants are not nul terminated if their length exceeds > 200 characters. The test cases do not use strings of that size where > that would make a difference. But using a fixed index type is > clearly > wrong. > > This patch removes the fixed char[200] array type from > playback::context, > and uses build_string_literal instead of using build_string directly. > > > Bootstrapped and reg-tested on x86_64-pc-linux-gnu. > Is it OK for trunk?
Sorry for the belated response. Was this tested with --enable-host-shared and --enable-languages=jit ? Note that "jit" is not included in --enable-languages=all. The patch seems reasonable, but I'm a little confused over the meaning of "len" in build_string_literal and build_string: does it refer to the length or the size of the string? > @@ -617,16 +616,9 @@ playback::rvalue * > playback::context:: > new_string_literal (const char *value) > { > - tree t_str = build_string (strlen (value), value); > - gcc_assert (m_char_array_type_node); > - TREE_TYPE (t_str) = m_char_array_type_node; > - > - /* Convert to (const char*), loosely based on > - c/c-typeck.c: array_to_pointer_conversion, > - by taking address of start of string. */ > - tree t_addr = build1 (ADDR_EXPR, m_const_char_ptr, t_str); > + tree t_str = build_string_literal (strlen (value) + 1, value); > > - return new rvalue (this, t_addr); > + return new rvalue (this, t_str); > } In the above, the call to build_string with strlen is replaced with build_string_literal with strlen + 1. build_string's comment says: "Note that for a C string literal, LEN should include the trailing NUL." but has: length = len + offsetof (struct tree_string, str) + 1; and: TREE_STRING_LENGTH (s) = len; memcpy (s->string.str, str, len); s->string.str[len] = '\0'; suggesting that the "len" parameter is in fact the length *without* the trailing NUL, and that a trailing NUL is added by build_string. However build_string_literal has: t = build_string (len, str); elem = build_type_variant (char_type_node, 1, 0); index = build_index_type (size_int (len - 1)); suggesting that the len is passed directly to build_string (and thus ought to be strlen), but the build_index_type uses len - 1 (which suggests that len is the size of the string, rather than its length). What's the intended meaning of len in these functions? Thanks Dave