This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][gcc] libgccjit: handle long literals in playback::context::new_string_literal
- From: Andrea Corallo <Andrea dot Corallo at arm dot com>
- To: David Malcolm <dmalcolm at redhat dot com>
- Cc: Andrea Corallo <Andrea dot Corallo at arm dot com>, "jit at gcc dot gnu dot org" <jit at gcc dot gnu dot org>, "gcc-patches at gcc dot gnu dot org" <gcc-patches at gcc dot gnu dot org>, nd <nd at arm dot com>
- Date: Tue, 12 Nov 2019 17:25:31 +0000
- Subject: Re: [PATCH][gcc] libgccjit: handle long literals in playback::context::new_string_literal
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=arm.com; dmarc=pass action=none header.from=arm.com; dkim=pass header.d=arm.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=44qVplPMVxK+zkwgXb6MzezrRF3sZyzTrKTZR3+mOn8=; b=Np2agJPDv/JXI1aLt0RlwfIRNcynsqzckI5kNFgwCkgIiZBmIfiT5a+9f2GkVLqIWUnhQ1qt47ycABkcHyeI86rOveq0WrSEicG3vpls7t3M7tezfBSpKX7yqAQjxaLvoIYXVrA2sPpyr7HRY6EfPB8uTc4aTdxSBPi8lNFGfuJwqrRx4dHZbGRl+hK69OfS2L5sWoeCWWi0KCqvO40LuwVSHGac7yDWUYI2E6FJ1GXLam/zF+UXA2pkbvD7eVk9uzVs8/+fcD79a0rWP0tBwKNy54+yZyQhgk9rxFwTDmu5aHv/FpTU8zI3PU7sRP2Fskyicv5Xyuowwuihs346fg==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=VJKGqDhJW2nVDW/59Zw8HNwnf1UE02CM2gn8sb4JOGZPARaCUiDrL1JP2WmkvKOseUvxm2/bjCnnjJzPvPpDd8AVB5kkFZHjf4G7l9SVm+PPYHt47ln7oHhdaRgtpdVxQkS6g84KnDOgf6Ycb/trnjWEaEe3g63+KAQgrQDNOHVyPQNVs2O3OwXhzes/gEk75aR1cITxyrRneO4N6jS7+CJmt2hsyNVC5Yj2SUfPgY5DPRR+TmkQgergnZJqwKSVa9PcQD/9mJwdnH/vZQ/ACF0mDG8YkcWBP/SE9/ntjlhpTrP7ussAC3eInHsfz1SJW+PwGeCVjPtom56b0QnZVg==
- Original-authentication-results: spf=none (sender IP is ) smtp.mailfrom=Andrea dot Corallo at arm dot com;
- References: <gkrwoernjnj.fsf@arm.com> <gkrftl5zfuq.fsf@arm.com> <gkrimpauoql.fsf@arm.com>
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