libgccjit: handle long literals in playback::context::new_string_literal

Message ID gkrwoernjnj.fsf@arm.com
State New
Headers show
Series
  • libgccjit: handle long literals in playback::context::new_string_literal
Related show

Commit Message

Andrea Corallo Sept. 2, 2019, 9:16 a.m.
Hi all,
yesterday I've found an interesting bug in libgccjit.
Seems we have an hard limitation of 200 characters for literal strings.
Attempting to create longer strings lead to ICE during pass_expand
while performing a sanity check in get_constant_size.

Tracking down the issue seems the code we have was inspired from
c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type
is actually defined with a size of 200.
The comment that follows that point sounded premonitory :) :)

/* Make a type for arrays of characters.
   With luck nothing will ever really depend on the length of this
   array type.  */

At least in the current implementation the type is set by
fix_string_type were the actual string length is taken in account.

I attach a patch updating the logic accordingly and a new testcase
 for that.

make check-jit is passing clean.

Best Regards
  Andrea

gcc/jit/ChangeLog
2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	* jit-playback.h
	(gcc::jit::recording::context m_recording_ctxt): Remove
	m_char_array_type_node field.
	* jit-playback.c
	(playback::context::context) Remove m_char_array_type_node from member
	initializer list.
	(playback::context::new_string_literal) Fix logic to handle string
	length > 200.

gcc/testsuite/ChangeLog
2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.
	* jit.dg/test-long-string-literal.c: New testcase.

Comments

Andrea Corallo Sept. 9, 2019, 2:47 p.m. | #1
Andrea Corallo writes:

> Hi all,

> yesterday I've found an interesting bug in libgccjit.

> Seems we have an hard limitation of 200 characters for literal strings.

> Attempting to create longer strings lead to ICE during pass_expand

> while performing a sanity check in get_constant_size.

>

> Tracking down the issue seems the code we have was inspired from

> c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type

> is actually defined with a size of 200.

> The comment that follows that point sounded premonitory :) :)

>

> /* Make a type for arrays of characters.

>    With luck nothing will ever really depend on the length of this

>    array type.  */

>

> At least in the current implementation the type is set by

> fix_string_type were the actual string length is taken in account.

>

> I attach a patch updating the logic accordingly and a new testcase

>  for that.

>

> make check-jit is passing clean.

>

> Best Regards

>   Andrea

>

> gcc/jit/ChangeLog

> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	* jit-playback.h

> 	(gcc::jit::recording::context m_recording_ctxt): Remove

> 	m_char_array_type_node field.

> 	* jit-playback.c

> 	(playback::context::context) Remove m_char_array_type_node from member

> 	initializer list.

> 	(playback::context::new_string_literal) Fix logic to handle string

> 	length > 200.

>

> gcc/testsuite/ChangeLog

> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.

> 	* jit.dg/test-long-string-literal.c: New testcase.

> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c

> index 9eeb2a7..a26b8d3 100644

> --- a/gcc/jit/jit-playback.c

> +++ b/gcc/jit/jit-playback.c

> @@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)

>    : 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 ());

> @@ -670,9 +669,12 @@ 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;

> +  /* Compare with c-family/c-common.c: fix_string_type.  */

> +  size_t len = strlen (value);

> +  tree i_type = build_index_type (size_int (len));

> +  tree a_type = build_array_type (char_type_node, i_type);

> +  tree t_str = build_string (len, value);

> +  TREE_TYPE (t_str) = a_type;

>

>    /* Convert to (const char*), loosely based on

>       c/c-typeck.c: array_to_pointer_conversion,

> @@ -2703,10 +2705,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 --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h

> index d4b148e..801f610 100644

> --- a/gcc/jit/jit-playback.h

> +++ b/gcc/jit/jit-playback.h

> @@ -322,7 +322,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.  */

> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h

> index 0272e6f8..1b3d561 100644

> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h

> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h

> @@ -220,6 +220,13 @@

>  #undef create_code

>  #undef verify_code

>

> +/* test-long-string-literal.c */

> +#define create_code create_code_long_string_literal

> +#define verify_code verify_code_long_string_literal

> +#include "test-long-string-literal.c"

> +#undef create_code

> +#undef verify_code

> +

>  /* test-sum-of-squares.c */

>  #define create_code create_code_sum_of_squares

>  #define verify_code verify_code_sum_of_squares

> diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c

> new file mode 100644

> index 0000000..882567c

> --- /dev/null

> +++ b/gcc/testsuite/jit.dg/test-long-string-literal.c

> @@ -0,0 +1,48 @@

> +#include <stdlib.h>

> +#include <stdio.h>

> +#include <string.h>

> +

> +#include "libgccjit.h"

> +

> +#include "harness.h"

> +

> +const char very_long_string[] =

> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

> +  "abcabcabcabcabcabcabcabcabcabca";

> +

> +void

> +create_code (gcc_jit_context *ctxt, void *user_data)

> +{

> +  /* Build the test_fn.  */

> +  gcc_jit_function *f =

> +    gcc_jit_context_new_function (

> +      ctxt, NULL,

> +      GCC_JIT_FUNCTION_EXPORTED,

> +      gcc_jit_context_get_type(ctxt,

> +			       GCC_JIT_TYPE_CONST_CHAR_PTR),

> +				"test_long_string_literal",

> +				0, NULL, 0);

> +  gcc_jit_block *blk =

> +    gcc_jit_function_new_block (f, "init_block");

> +  gcc_jit_rvalue *res =

> +    gcc_jit_context_new_string_literal (ctxt, very_long_string);

> +

> +  gcc_jit_block_end_with_return (blk, NULL, res);

> +}

> +

> +void

> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)

> +{

> +  typedef const char *(*fn_type) (void);

> +  CHECK_NON_NULL (result);

> +  fn_type test_long_string_literal =

> +    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");

> +  CHECK_NON_NULL (test_long_string_literal);

> +

> +  /* Call the JIT-generated function.  */

> +  const char *str = test_long_string_literal ();

> +  CHECK_NON_NULL (str);

> +  CHECK_VALUE (strcmp (str, very_long_string), 0);

> +}


Kindly pinging

Bests
  Andrea
Andrea Corallo Sept. 30, 2019, 9:18 a.m. | #2
Andrea Corallo writes:

> Andrea Corallo writes:

>

>> Hi all,

>> yesterday I've found an interesting bug in libgccjit.

>> Seems we have an hard limitation of 200 characters for literal strings.

>> Attempting to create longer strings lead to ICE during pass_expand

>> while performing a sanity check in get_constant_size.

>>

>> Tracking down the issue seems the code we have was inspired from

>> c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type

>> is actually defined with a size of 200.

>> The comment that follows that point sounded premonitory :) :)

>>

>> /* Make a type for arrays of characters.

>>    With luck nothing will ever really depend on the length of this

>>    array type.  */

>>

>> At least in the current implementation the type is set by

>> fix_string_type were the actual string length is taken in account.

>>

>> I attach a patch updating the logic accordingly and a new testcase

>>  for that.

>>

>> make check-jit is passing clean.

>>

>> Best Regards

>>   Andrea

>>

>> gcc/jit/ChangeLog

>> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>

>> 	* jit-playback.h

>> 	(gcc::jit::recording::context m_recording_ctxt): Remove

>> 	m_char_array_type_node field.

>> 	* jit-playback.c

>> 	(playback::context::context) Remove m_char_array_type_node from member

>> 	initializer list.

>> 	(playback::context::new_string_literal) Fix logic to handle string

>> 	length > 200.

>>

>> gcc/testsuite/ChangeLog

>> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>

>> 	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.

>> 	* jit.dg/test-long-string-literal.c: New testcase.

>> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c

>> index 9eeb2a7..a26b8d3 100644

>> --- a/gcc/jit/jit-playback.c

>> +++ b/gcc/jit/jit-playback.c

>> @@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)

>>    : 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 ());

>> @@ -670,9 +669,12 @@ 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;

>> +  /* Compare with c-family/c-common.c: fix_string_type.  */

>> +  size_t len = strlen (value);

>> +  tree i_type = build_index_type (size_int (len));

>> +  tree a_type = build_array_type (char_type_node, i_type);

>> +  tree t_str = build_string (len, value);

>> +  TREE_TYPE (t_str) = a_type;

>>

>>    /* Convert to (const char*), loosely based on

>>       c/c-typeck.c: array_to_pointer_conversion,

>> @@ -2703,10 +2705,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 --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h

>> index d4b148e..801f610 100644

>> --- a/gcc/jit/jit-playback.h

>> +++ b/gcc/jit/jit-playback.h

>> @@ -322,7 +322,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.  */

>> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h

>> index 0272e6f8..1b3d561 100644

>> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h

>> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h

>> @@ -220,6 +220,13 @@

>>  #undef create_code

>>  #undef verify_code

>>

>> +/* test-long-string-literal.c */

>> +#define create_code create_code_long_string_literal

>> +#define verify_code verify_code_long_string_literal

>> +#include "test-long-string-literal.c"

>> +#undef create_code

>> +#undef verify_code

>> +

>>  /* test-sum-of-squares.c */

>>  #define create_code create_code_sum_of_squares

>>  #define verify_code verify_code_sum_of_squares

>> diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c

>> new file mode 100644

>> index 0000000..882567c

>> --- /dev/null

>> +++ b/gcc/testsuite/jit.dg/test-long-string-literal.c

>> @@ -0,0 +1,48 @@

>> +#include <stdlib.h>

>> +#include <stdio.h>

>> +#include <string.h>

>> +

>> +#include "libgccjit.h"

>> +

>> +#include "harness.h"

>> +

>> +const char very_long_string[] =

>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>> +  "abcabcabcabcabcabcabcabcabcabca";

>> +

>> +void

>> +create_code (gcc_jit_context *ctxt, void *user_data)

>> +{

>> +  /* Build the test_fn.  */

>> +  gcc_jit_function *f =

>> +    gcc_jit_context_new_function (

>> +      ctxt, NULL,

>> +      GCC_JIT_FUNCTION_EXPORTED,

>> +      gcc_jit_context_get_type(ctxt,

>> +			       GCC_JIT_TYPE_CONST_CHAR_PTR),

>> +				"test_long_string_literal",

>> +				0, NULL, 0);

>> +  gcc_jit_block *blk =

>> +    gcc_jit_function_new_block (f, "init_block");

>> +  gcc_jit_rvalue *res =

>> +    gcc_jit_context_new_string_literal (ctxt, very_long_string);

>> +

>> +  gcc_jit_block_end_with_return (blk, NULL, res);

>> +}

>> +

>> +void

>> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)

>> +{

>> +  typedef const char *(*fn_type) (void);

>> +  CHECK_NON_NULL (result);

>> +  fn_type test_long_string_literal =

>> +    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");

>> +  CHECK_NON_NULL (test_long_string_literal);

>> +

>> +  /* Call the JIT-generated function.  */

>> +  const char *str = test_long_string_literal ();

>> +  CHECK_NON_NULL (str);

>> +  CHECK_VALUE (strcmp (str, very_long_string), 0);

>> +}

>

> Kindly pinging

>

> Bests

>   Andrea


Hi,
I'd like to ping this patch.

Bests
 Andrea
Andrea Corallo Nov. 12, 2019, 5:25 p.m. | #3
Andrea Corallo writes:

> Andrea Corallo writes:

>

>> Andrea Corallo writes:

>>

>>> Hi all,

>>> yesterday I've found an interesting bug in libgccjit.

>>> Seems we have an hard limitation of 200 characters for literal strings.

>>> Attempting to create longer strings lead to ICE during pass_expand

>>> while performing a sanity check in get_constant_size.

>>>

>>> Tracking down the issue seems the code we have was inspired from

>>> c-family/c-common.c:c_common_nodes_and_builtins were array_domain_type

>>> is actually defined with a size of 200.

>>> The comment that follows that point sounded premonitory :) :)

>>>

>>> /* Make a type for arrays of characters.

>>>    With luck nothing will ever really depend on the length of this

>>>    array type.  */

>>>

>>> At least in the current implementation the type is set by

>>> fix_string_type were the actual string length is taken in account.

>>>

>>> I attach a patch updating the logic accordingly and a new testcase

>>>  for that.

>>>

>>> make check-jit is passing clean.

>>>

>>> Best Regards

>>>   Andrea

>>>

>>> gcc/jit/ChangeLog

>>> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>>

>>> 	* jit-playback.h

>>> 	(gcc::jit::recording::context m_recording_ctxt): Remove

>>> 	m_char_array_type_node field.

>>> 	* jit-playback.c

>>> 	(playback::context::context) Remove m_char_array_type_node from member

>>> 	initializer list.

>>> 	(playback::context::new_string_literal) Fix logic to handle string

>>> 	length > 200.

>>>

>>> gcc/testsuite/ChangeLog

>>> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>>

>>> 	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.

>>> 	* jit.dg/test-long-string-literal.c: New testcase.

>>> diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c

>>> index 9eeb2a7..a26b8d3 100644

>>> --- a/gcc/jit/jit-playback.c

>>> +++ b/gcc/jit/jit-playback.c

>>> @@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)

>>>    : 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 ());

>>> @@ -670,9 +669,12 @@ 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;

>>> +  /* Compare with c-family/c-common.c: fix_string_type.  */

>>> +  size_t len = strlen (value);

>>> +  tree i_type = build_index_type (size_int (len));

>>> +  tree a_type = build_array_type (char_type_node, i_type);

>>> +  tree t_str = build_string (len, value);

>>> +  TREE_TYPE (t_str) = a_type;

>>>

>>>    /* Convert to (const char*), loosely based on

>>>       c/c-typeck.c: array_to_pointer_conversion,

>>> @@ -2703,10 +2705,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 --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h

>>> index d4b148e..801f610 100644

>>> --- a/gcc/jit/jit-playback.h

>>> +++ b/gcc/jit/jit-playback.h

>>> @@ -322,7 +322,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.  */

>>> diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h

>>> index 0272e6f8..1b3d561 100644

>>> --- a/gcc/testsuite/jit.dg/all-non-failing-tests.h

>>> +++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h

>>> @@ -220,6 +220,13 @@

>>>  #undef create_code

>>>  #undef verify_code

>>>

>>> +/* test-long-string-literal.c */

>>> +#define create_code create_code_long_string_literal

>>> +#define verify_code verify_code_long_string_literal

>>> +#include "test-long-string-literal.c"

>>> +#undef create_code

>>> +#undef verify_code

>>> +

>>>  /* test-sum-of-squares.c */

>>>  #define create_code create_code_sum_of_squares

>>>  #define verify_code verify_code_sum_of_squares

>>> diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c

>>> new file mode 100644

>>> index 0000000..882567c

>>> --- /dev/null

>>> +++ b/gcc/testsuite/jit.dg/test-long-string-literal.c

>>> @@ -0,0 +1,48 @@

>>> +#include <stdlib.h>

>>> +#include <stdio.h>

>>> +#include <string.h>

>>> +

>>> +#include "libgccjit.h"

>>> +

>>> +#include "harness.h"

>>> +

>>> +const char very_long_string[] =

>>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>>> +  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"

>>> +  "abcabcabcabcabcabcabcabcabcabca";

>>> +

>>> +void

>>> +create_code (gcc_jit_context *ctxt, void *user_data)

>>> +{

>>> +  /* Build the test_fn.  */

>>> +  gcc_jit_function *f =

>>> +    gcc_jit_context_new_function (

>>> +      ctxt, NULL,

>>> +      GCC_JIT_FUNCTION_EXPORTED,

>>> +      gcc_jit_context_get_type(ctxt,

>>> +			       GCC_JIT_TYPE_CONST_CHAR_PTR),

>>> +				"test_long_string_literal",

>>> +				0, NULL, 0);

>>> +  gcc_jit_block *blk =

>>> +    gcc_jit_function_new_block (f, "init_block");

>>> +  gcc_jit_rvalue *res =

>>> +    gcc_jit_context_new_string_literal (ctxt, very_long_string);

>>> +

>>> +  gcc_jit_block_end_with_return (blk, NULL, res);

>>> +}

>>> +

>>> +void

>>> +verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)

>>> +{

>>> +  typedef const char *(*fn_type) (void);

>>> +  CHECK_NON_NULL (result);

>>> +  fn_type test_long_string_literal =

>>> +    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");

>>> +  CHECK_NON_NULL (test_long_string_literal);

>>> +

>>> +  /* Call the JIT-generated function.  */

>>> +  const char *str = test_long_string_literal ();

>>> +  CHECK_NON_NULL (str);

>>> +  CHECK_VALUE (strcmp (str, very_long_string), 0);

>>> +}

>>

>> Kindly pinging

>>

>> Bests

>>   Andrea

>

> Hi,

> I'd like to ping this patch.

>

> Bests

>  Andrea


Pinging again.

Bests
  Andrea
David Malcolm March 6, 2020, 3:18 a.m. | #4
On Mon, 2019-09-02 at 09:16 +0000, Andrea Corallo wrote:
> Hi all,

> yesterday I've found an interesting bug in libgccjit.

> Seems we have an hard limitation of 200 characters for literal

> strings.

> Attempting to create longer strings lead to ICE during pass_expand

> while performing a sanity check in get_constant_size.

> 

> Tracking down the issue seems the code we have was inspired from

> c-family/c-common.c:c_common_nodes_and_builtins were

> array_domain_type

> is actually defined with a size of 200.

> The comment that follows that point sounded premonitory :) :)

> 

> /* Make a type for arrays of characters.

>    With luck nothing will ever really depend on the length of this

>    array type.  */

> 

> At least in the current implementation the type is set by

> fix_string_type were the actual string length is taken in account.

> 

> I attach a patch updating the logic accordingly and a new testcase

>  for that.

> 

> make check-jit is passing clean.

> 

> Best Regards

>   Andrea


Sorry about the long delay in reviewing this patch.

> gcc/jit/ChangeLog

> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

> 

>         * jit-playback.h

>         (gcc::jit::recording::context m_recording_ctxt): Remove

                     ^^^^^^^^^
		     "playback" here
		     
>         m_char_array_type_node field.


[...]

> @@ -670,9 +669,12 @@ 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;

> +  /* Compare with c-family/c-common.c: fix_string_type.  */

> +  size_t len = strlen (value);

> +  tree i_type = build_index_type (size_int (len));

> +  tree a_type = build_array_type (char_type_node, i_type);

> +  tree t_str = build_string (len, value);

> +  TREE_TYPE (t_str) = a_type;


This code works with string lengths and string sizes which always
requires a little extra care.  I'd like to see at least a comment
discussing this, as it's not immediately clear to me that we're
correctly handling the NUL terminator here.

Consider the string "foo".
This has strlen == 3, and its size is 4 chars.

build_string's comment says "Note that for a C string literal, LEN
should include the trailing NUL."

However, build_string appears to allocate one byte more than LEN, and
write a NUL in that final position.

fix_string_type has:

  int length = TREE_STRING_LENGTH (value);

where tree.h has:

  /* In C terms, this is sizeof, not strlen.  */
  #define TREE_STRING_LENGTH(NODE) (STRING_CST_CHECK (NODE)-
>string.length)


and:
  nchars = length / charsz;

where for our purposes charsz == 1 and so nchars == length.

fix_string_type has:

  i_type = build_index_type (size_int (nchars - 1));

So I *think* the patch is correct, but there ought to be a comment at
least.

Or maybe use TREE_STRING_LENGTH (t_str) to more closely follow
fix_string_type?

[...]

Also, please add an assertion and comment to the testcase to assert
that the very long string is indeed longer than the previous limit of
200.

Thanks
Dave
Andrea Corallo March 7, 2020, 9:43 p.m. | #5
David Malcolm <dmalcolm@redhat.com> writes:

> On Mon, 2019-09-02 at 09:16 +0000, Andrea Corallo wrote:

>> Hi all,

>> yesterday I've found an interesting bug in libgccjit.

>> Seems we have an hard limitation of 200 characters for literal

>> strings.

>> Attempting to create longer strings lead to ICE during pass_expand

>> while performing a sanity check in get_constant_size.

>>

>> Tracking down the issue seems the code we have was inspired from

>> c-family/c-common.c:c_common_nodes_and_builtins were

>> array_domain_type

>> is actually defined with a size of 200.

>> The comment that follows that point sounded premonitory :) :)

>>

>> /* Make a type for arrays of characters.

>>    With luck nothing will ever really depend on the length of this

>>    array type.  */

>>

>> At least in the current implementation the type is set by

>> fix_string_type were the actual string length is taken in account.

>>

>> I attach a patch updating the logic accordingly and a new testcase

>>  for that.

>>

>> make check-jit is passing clean.

>>

>> Best Regards

>>   Andrea

>

> Sorry about the long delay in reviewing this patch.

>

>> gcc/jit/ChangeLog

>> 2019-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>>

>>         * jit-playback.h

>>         (gcc::jit::recording::context m_recording_ctxt): Remove

>                      ^^^^^^^^^

> 		     "playback" here

>

>>         m_char_array_type_node field.

>

> [...]

>

>> @@ -670,9 +669,12 @@ 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;

>> +  /* Compare with c-family/c-common.c: fix_string_type.  */

>> +  size_t len = strlen (value);

>> +  tree i_type = build_index_type (size_int (len));

>> +  tree a_type = build_array_type (char_type_node, i_type);

>> +  tree t_str = build_string (len, value);

>> +  TREE_TYPE (t_str) = a_type;

>

> This code works with string lengths and string sizes which always

> requires a little extra care.  I'd like to see at least a comment

> discussing this, as it's not immediately clear to me that we're

> correctly handling the NUL terminator here.

>

> Consider the string "foo".

> This has strlen == 3, and its size is 4 chars.

>

> build_string's comment says "Note that for a C string literal, LEN

> should include the trailing NUL."

>

> However, build_string appears to allocate one byte more than LEN, and

> write a NUL in that final position.

>

> fix_string_type has:

>

>   int length = TREE_STRING_LENGTH (value);

>

> where tree.h has:

>

>   /* In C terms, this is sizeof, not strlen.  */

>   #define TREE_STRING_LENGTH(NODE) (STRING_CST_CHECK (NODE)-

>>string.length)

>

> and:

>   nchars = length / charsz;

>

> where for our purposes charsz == 1 and so nchars == length.

>

> fix_string_type has:

>

>   i_type = build_index_type (size_int (nchars - 1));

>

> So I *think* the patch is correct, but there ought to be a comment at

> least.

>

> Or maybe use TREE_STRING_LENGTH (t_str) to more closely follow

> fix_string_type?

>


Mmmm when I wrote the patch I looked into the build_string code missing
the comment and that was my understanding too.  But looking better now
in the compiler I think the patch is *not* correct.  The trouble is that
if we call build_string (3, "foo") (as we would do now) we set
TREE_STRING_LENGTH to 3 and this appears to be wrong.

Actually build_string can be called using the strlen as argument (see
for example the various definition of DEF_ATTR_STRING) but apparently
this is not how is done for C strings in the frontends.  Given that this
is what the comment suggests and that modifying the patch with the +1
keeps it functional I think doing what the front-end does is the right
thing.

> Also, please add an assertion and comment to the testcase to assert

> that the very long string is indeed longer than the previous limit of

> 200.


Ack

> Thanks

> Dave


Thanks for reviewing

  Andrea
Andrea Corallo March 9, 2020, 10:20 p.m. | #6
Hi all,

second version of the patch for the 200 characters limit for literal
strings addressing comments.

make check-jit is passing clean.

Best Regards
  Andrea

gcc/jit/ChangeLog
2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	* jit-playback.h
	(gcc::jit::playback::context m_recording_ctxt): Remove
	m_char_array_type_node field.
	* jit-playback.c
	(playback::context::context) Remove m_char_array_type_node from member
	initializer list.
	(playback::context::new_string_literal) Fix logic to handle string
	length > 200.

gcc/testsuite/ChangeLog
2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.
	* jit.dg/test-long-string-literal.c: New testcase.
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 904cc167a0c..074434a9f6b 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -322,7 +322,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.  */
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index da687002a72..d2c8bb4c154 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)
   : 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 ());
@@ -670,9 +669,14 @@ 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;
+  /* Compare with c-family/c-common.c: fix_string_type.  */
+  size_t len = strlen (value);
+  tree i_type = build_index_type (size_int (len));
+  tree a_type = build_array_type (char_type_node, i_type);
+  /* build_string len parameter must include NUL terminator when
+     building C strings.  */
+  tree t_str = build_string (len + 1, value);
+  TREE_TYPE (t_str) = a_type;
 
   /* Convert to (const char*), loosely based on
      c/c-typeck.c: array_to_pointer_conversion,
@@ -2701,10 +2705,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 --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 0272e6f846f..1b3d5618779 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -220,6 +220,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-long-string-literal.c */
+#define create_code create_code_long_string_literal
+#define verify_code verify_code_long_string_literal
+#include "test-long-string-literal.c"
+#undef create_code
+#undef verify_code
+
 /* test-sum-of-squares.c */
 #define create_code create_code_sum_of_squares
 #define verify_code verify_code_sum_of_squares
diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c
new file mode 100644
index 00000000000..6caaa781c0b
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-long-string-literal.c
@@ -0,0 +1,54 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <assert.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+const char very_long_string[] =
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabca";
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Build the test_fn.  */
+  gcc_jit_function *f =
+    gcc_jit_context_new_function (
+      ctxt, NULL,
+      GCC_JIT_FUNCTION_EXPORTED,
+      gcc_jit_context_get_type(ctxt,
+			       GCC_JIT_TYPE_CONST_CHAR_PTR),
+				"test_long_string_literal",
+				0, NULL, 0);
+  gcc_jit_block *blk =
+    gcc_jit_function_new_block (f, "init_block");
+
+  /* very_long_string is longer than 200 characters to specifically
+     check that the previous limitation no longer apply.  */
+
+  assert (sizeof (very_long_string) > 200);
+  gcc_jit_rvalue *res =
+    gcc_jit_context_new_string_literal (ctxt, very_long_string);
+
+  gcc_jit_block_end_with_return (blk, NULL, res);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef const char *(*fn_type) (void);
+  CHECK_NON_NULL (result);
+  fn_type test_long_string_literal =
+    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");
+  CHECK_NON_NULL (test_long_string_literal);
+
+  /* Call the JIT-generated function.  */
+  const char *str = test_long_string_literal ();
+  CHECK_NON_NULL (str);
+  CHECK_VALUE (strcmp (str, very_long_string), 0);
+}
Andrea Corallo March 18, 2020, 10:52 p.m. | #7
Andrea Corallo <andrea.corallo@arm.com> writes:

> Hi all,

>

> second version of the patch for the 200 characters limit for literal

> strings addressing comments.

>

> make check-jit is passing clean.

>

> Best Regards

>   Andrea

>

> gcc/jit/ChangeLog

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	* jit-playback.h

> 	(gcc::jit::playback::context m_recording_ctxt): Remove

> 	m_char_array_type_node field.

> 	* jit-playback.c

> 	(playback::context::context) Remove m_char_array_type_node from member

> 	initializer list.

> 	(playback::context::new_string_literal) Fix logic to handle string

> 	length > 200.

>

> gcc/testsuite/ChangeLog

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

>

> 	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.

> 	* jit.dg/test-long-string-literal.c: New testcase.


Kind ping, okay for trunk?
Patrick Palka via Gcc-patches March 21, 2020, 1:41 a.m. | #8
On Mon, 2020-03-09 at 22:20 +0000, Andrea Corallo wrote:
> Hi all,

> 

> second version of the patch for the 200 characters limit for literal

> strings addressing comments.

> 

> make check-jit is passing clean.

> 

> Best Regards

>   Andrea

> 

> gcc/jit/ChangeLog

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

> 

> 	* jit-playback.h

> 	(gcc::jit::playback::context m_recording_ctxt): Remove

> 	m_char_array_type_node field.

> 	* jit-playback.c

> 	(playback::context::context) Remove m_char_array_type_node from

> member

> 	initializer list.

> 	(playback::context::new_string_literal) Fix logic to handle

> string

> 	length > 200.

> 

> gcc/testsuite/ChangeLog

> 2020-??-??  Andrea Corallo  <andrea.corallo@arm.com>

> 

> 	* jit.dg/all-non-failing-tests.h: Add test-long-string-

> literal.c.

> 	* jit.dg/test-long-string-literal.c: New testcase.

> 


diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 0272e6f846f..1b3d5618779 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -220,6 +220,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-long-string-literal.c */
+#define create_code create_code_long_string_literal
+#define verify_code verify_code_long_string_literal
+#include "test-long-string-literal.c"
+#undef create_code
+#undef verify_code
+

Please adjust where you add this so that these things are in
alphabetical order.

An entry also needs to be added to the "testcases" array at the end of
the header for this to do anything.

OK with that change.

David
Andrea Corallo March 23, 2020, 6:04 p.m. | #9
Hi all,

attached the final version of the patch for the 200 characters limit
for literal strings addressing comments on the missing "testcases"
array entry (apologies) and alphabetic order.

make check-jit is passing clean.

Pushed into trunk.

Best Regards
  Andrea

gcc/jit/ChangeLog
2020-03-23  Andrea Corallo  <andrea.corallo@arm.com>

	* jit-playback.h
	(gcc::jit::playback::context m_recording_ctxt): Remove
	m_char_array_type_node field.
	* jit-playback.c
	(playback::context::context) Remove m_char_array_type_node from member
	initializer list.
	(playback::context::new_string_literal) Fix logic to handle string
	length > 200.

gcc/testsuite/ChangeLog
2020-03-23  Andrea Corallo  <andrea.corallo@arm.com>

	* jit.dg/all-non-failing-tests.h: Add test-long-string-literal.c.
	* jit.dg/test-long-string-literal.c: New testcase.
diff --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index 904cc167a0c..074434a9f6b 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -322,7 +322,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.  */
diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index da687002a72..d2c8bb4c154 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -88,7 +88,6 @@ playback::context::context (recording::context *ctxt)
   : 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 ());
@@ -670,9 +669,14 @@ 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;
+  /* Compare with c-family/c-common.c: fix_string_type.  */
+  size_t len = strlen (value);
+  tree i_type = build_index_type (size_int (len));
+  tree a_type = build_array_type (char_type_node, i_type);
+  /* build_string len parameter must include NUL terminator when
+     building C strings.  */
+  tree t_str = build_string (len + 1, value);
+  TREE_TYPE (t_str) = a_type;
 
   /* Convert to (const char*), loosely based on
      c/c-typeck.c: array_to_pointer_conversion,
@@ -2701,10 +2705,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 --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 0272e6f846f..b2acc74ae95 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -178,6 +178,13 @@
 #undef create_code
 #undef verify_code
 
+/* test-long-string-literal.c */
+#define create_code create_code_long_string_literal
+#define verify_code verify_code_long_string_literal
+#include "test-long-string-literal.c"
+#undef create_code
+#undef verify_code
+
 /* test-quadratic.c */
 #define create_code create_code_quadratic
 #define verify_code verify_code_quadratic
@@ -342,6 +349,9 @@ const struct testcase testcases[] = {
   {"long_names",
    create_code_long_names,
    verify_code_long_names},
+  {"long_string_literal",
+   create_code_long_string_literal,
+   verify_code_long_string_literal},
   {"quadratic",
    create_code_quadratic,
    verify_code_quadratic},
diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c
new file mode 100644
index 00000000000..6caaa781c0b
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-long-string-literal.c
@@ -0,0 +1,54 @@
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+#include <assert.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+const char very_long_string[] =
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabca";
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Build the test_fn.  */
+  gcc_jit_function *f =
+    gcc_jit_context_new_function (
+      ctxt, NULL,
+      GCC_JIT_FUNCTION_EXPORTED,
+      gcc_jit_context_get_type(ctxt,
+			       GCC_JIT_TYPE_CONST_CHAR_PTR),
+				"test_long_string_literal",
+				0, NULL, 0);
+  gcc_jit_block *blk =
+    gcc_jit_function_new_block (f, "init_block");
+
+  /* very_long_string is longer than 200 characters to specifically
+     check that the previous limitation no longer apply.  */
+
+  assert (sizeof (very_long_string) > 200);
+  gcc_jit_rvalue *res =
+    gcc_jit_context_new_string_literal (ctxt, very_long_string);
+
+  gcc_jit_block_end_with_return (blk, NULL, res);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef const char *(*fn_type) (void);
+  CHECK_NON_NULL (result);
+  fn_type test_long_string_literal =
+    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");
+  CHECK_NON_NULL (test_long_string_literal);
+
+  /* Call the JIT-generated function.  */
+  const char *str = test_long_string_literal ();
+  CHECK_NON_NULL (str);
+  CHECK_VALUE (strcmp (str, very_long_string), 0);
+}

Patch

diff --git a/gcc/jit/jit-playback.c b/gcc/jit/jit-playback.c
index 9eeb2a7..a26b8d3 100644
--- a/gcc/jit/jit-playback.c
+++ b/gcc/jit/jit-playback.c
@@ -88,7 +88,6 @@  playback::context::context (recording::context *ctxt)
   : 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 ());
@@ -670,9 +669,12 @@  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;
+  /* Compare with c-family/c-common.c: fix_string_type.  */
+  size_t len = strlen (value);
+  tree i_type = build_index_type (size_int (len));
+  tree a_type = build_array_type (char_type_node, i_type);
+  tree t_str = build_string (len, value);
+  TREE_TYPE (t_str) = a_type;
 
   /* Convert to (const char*), loosely based on
      c/c-typeck.c: array_to_pointer_conversion,
@@ -2703,10 +2705,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 --git a/gcc/jit/jit-playback.h b/gcc/jit/jit-playback.h
index d4b148e..801f610 100644
--- a/gcc/jit/jit-playback.h
+++ b/gcc/jit/jit-playback.h
@@ -322,7 +322,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.  */
diff --git a/gcc/testsuite/jit.dg/all-non-failing-tests.h b/gcc/testsuite/jit.dg/all-non-failing-tests.h
index 0272e6f8..1b3d561 100644
--- a/gcc/testsuite/jit.dg/all-non-failing-tests.h
+++ b/gcc/testsuite/jit.dg/all-non-failing-tests.h
@@ -220,6 +220,13 @@ 
 #undef create_code
 #undef verify_code
 
+/* test-long-string-literal.c */
+#define create_code create_code_long_string_literal
+#define verify_code verify_code_long_string_literal
+#include "test-long-string-literal.c"
+#undef create_code
+#undef verify_code
+
 /* test-sum-of-squares.c */
 #define create_code create_code_sum_of_squares
 #define verify_code verify_code_sum_of_squares
diff --git a/gcc/testsuite/jit.dg/test-long-string-literal.c b/gcc/testsuite/jit.dg/test-long-string-literal.c
new file mode 100644
index 0000000..882567c
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-long-string-literal.c
@@ -0,0 +1,48 @@ 
+#include <stdlib.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+const char very_long_string[] =
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabcabc"
+  "abcabcabcabcabcabcabcabcabcabca";
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  /* Build the test_fn.  */
+  gcc_jit_function *f =
+    gcc_jit_context_new_function (
+      ctxt, NULL,
+      GCC_JIT_FUNCTION_EXPORTED,
+      gcc_jit_context_get_type(ctxt,
+			       GCC_JIT_TYPE_CONST_CHAR_PTR),
+				"test_long_string_literal",
+				0, NULL, 0);
+  gcc_jit_block *blk =
+    gcc_jit_function_new_block (f, "init_block");
+  gcc_jit_rvalue *res =
+    gcc_jit_context_new_string_literal (ctxt, very_long_string);
+
+  gcc_jit_block_end_with_return (blk, NULL, res);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  typedef const char *(*fn_type) (void);
+  CHECK_NON_NULL (result);
+  fn_type test_long_string_literal =
+    (fn_type)gcc_jit_result_get_code (result, "test_long_string_literal");
+  CHECK_NON_NULL (test_long_string_literal);
+
+  /* Call the JIT-generated function.  */
+  const char *str = test_long_string_literal ();
+  CHECK_NON_NULL (str);
+  CHECK_VALUE (strcmp (str, very_long_string), 0);
+}