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.cora...@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.cora...@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