Fix not properly nul-terminated string constants in JIT

Message ID AM5PR0701MB2657D814EBE588723929D667E4210@AM5PR0701MB2657.eurprd07.prod.outlook.com
State New
Headers show
Series
  • Fix not properly nul-terminated string constants in JIT
Related show

Commit Message

Bernd Edlinger Aug. 5, 2018, 4:59 p.m.
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?


Thanks
Bernd.

Comments

Bernd Edlinger Aug. 26, 2018, 7:40 p.m. | #1
Ping...

This is just plain wrong, independent
of any STRING_CST semantic issues.

The original patch (retested on current trunk) is
here: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00370.html


On 08/05/18 18:59, 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?
> 
> 
> Thanks
> Bernd.
Bernd Edlinger Oct. 10, 2018, 7:43 p.m. | #2
Ping...


On 08/26/18 21:40, Bernd Edlinger wrote:
> Ping...
> 
> This is just plain wrong, independent
> of any STRING_CST semantic issues.
> 
> The original patch (retested on current trunk) is
> here: https://gcc.gnu.org/ml/gcc-patches/2018-08/msg00370.html
> 
> 
> On 08/05/18 18:59, 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?
>>
>>
>> Thanks
>> Bernd.

Patch

2018-08-05  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* jit-playback.c (playback::context::context): Remove
	m_char_array_type_node.
	(playback::context::new_string_literal): Use
	build_string_literal.
	(playback::context::replay): Remove m_char_array_type_node.
	* jit-playback.h (playback::context::m_char_array_type_node): Remove.

diff -pur gcc/jit/jit-playback.c gcc/jit/jit-playback.c
--- gcc/jit/jit-playback.c	2018-06-28 09:08:01.000000000 +0200
+++ gcc/jit/jit-playback.c	2018-08-05 15:58:15.815403219 +0200
@@ -81,7 +81,6 @@  playback::context::context (recording::c
   : log_user (ctxt->get_logger ()),
     m_recording_ctxt (ctxt),
     m_tempdir (NULL),
-    m_char_array_type_node (NULL),
     m_const_char_ptr (NULL)
 {
   JIT_LOG_SCOPE (get_logger ());
@@ -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);
 }
 
 /* Construct a playback::rvalue instance (wrapping a tree) for a
@@ -2633,10 +2625,6 @@  playback::context::
 replay ()
 {
   JIT_LOG_SCOPE (get_logger ());
-  /* Adapted from c-common.c:c_common_nodes_and_builtins.  */
-  tree array_domain_type = build_index_type (size_int (200));
-  m_char_array_type_node
-    = build_array_type (char_type_node, array_domain_type);
 
   m_const_char_ptr
     = build_pointer_type (build_qualified_type (char_type_node,
diff -pur gcc/jit/jit-playback.h gcc/jit/jit-playback.h
--- gcc/jit/jit-playback.h	2018-01-03 11:03:58.000000000 +0100
+++ gcc/jit/jit-playback.h	2018-08-05 15:58:52.988918367 +0200
@@ -316,7 +316,6 @@  private:
 
   auto_vec<function *> m_functions;
   auto_vec<tree> m_globals;
-  tree m_char_array_type_node;
   tree m_const_char_ptr;
 
   /* Source location handling.  */