[PATCH] libgccjit: Add option to allow special characters in function names

[PATCH] libgccjit: Add option to allow special characters in function names

[Antoni Boucher]

[-- Attachment #1: Type: text/plain, Size: 182 bytes --]

Hi.
This patch adds a new option to allow special characters like . and $
in function names.
This is useful to allow for mangling using those characters.
Thanks for the review.

[-- Attachment #2: 0001-libgccjit-Add-option-to-allow-special-characters-in-.patch --]
[-- Type: text/x-patch, Size: 5948 bytes --]

From 89a92e561ca83a622dcc22a6500ca2b17551edb1 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Thu, 15 Feb 2024 17:03:22 -0500
Subject: [PATCH] libgccjit: Add option to allow special characters in function
 names

gcc/jit/ChangeLog:

	* docs/topics/contexts.rst: Add documentation for new option.
	* jit-recording.cc (recording::context::get_bool_option): New
	method.
	* jit-recording.h (get_bool_option): New method.
	* libgccjit.cc (gcc_jit_context_new_function): Allow special
	characters in function names.
	* libgccjit.h (enum gcc_jit_bool_option): New option.

gcc/testsuite/ChangeLog:

	* jit.dg/test-special-chars.c: New test.
---
 gcc/jit/docs/topics/contexts.rst          |  4 +++
 gcc/jit/jit-recording.cc                  | 15 ++++++++-
 gcc/jit/jit-recording.h                   |  3 ++
 gcc/jit/libgccjit.cc                      |  8 +++--
 gcc/jit/libgccjit.h                       |  3 ++
 gcc/testsuite/jit.dg/test-special-chars.c | 41 +++++++++++++++++++++++
 6 files changed, 71 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-special-chars.c

diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
index 10a0e50f9f6..4af75ea7418 100644
--- a/gcc/jit/docs/topics/contexts.rst
+++ b/gcc/jit/docs/topics/contexts.rst
@@ -453,6 +453,10 @@ Boolean options
      If true, the :type:`gcc_jit_context` will not clean up intermediate files
      written to the filesystem, and will display their location on stderr.
 
+  .. macro:: GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
+
+     If true, allow special characters like . and $ in function names.
+
 .. function:: void \
               gcc_jit_context_set_bool_allow_unreachable_blocks (gcc_jit_context *ctxt, \
                                                                  int bool_value)
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 83a8b299b91..962c37ee87e 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -1511,6 +1511,18 @@ recording::context::set_bool_option (enum gcc_jit_bool_option opt,
   log_bool_option (opt);
 }
 
+int
+recording::context::get_bool_option (enum gcc_jit_bool_option opt)
+{
+  if (opt < 0 || opt >= GCC_JIT_NUM_BOOL_OPTIONS)
+    {
+      add_error (NULL,
+		 "unrecognized (enum gcc_jit_bool_option) value: %i", opt);
+      return 0;
+    }
+  return m_bool_options[opt];
+}
+
 void
 recording::context::set_inner_bool_option (enum inner_bool_option inner_opt,
 					   int value)
@@ -1863,7 +1875,8 @@ static const char * const
   "GCC_JIT_BOOL_OPTION_DUMP_SUMMARY",
   "GCC_JIT_BOOL_OPTION_DUMP_EVERYTHING",
   "GCC_JIT_BOOL_OPTION_SELFCHECK_GC",
-  "GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES"
+  "GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES",
+  "GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES",
 };
 
 static const char * const
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index 4833b2d3f52..377b60c662c 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -267,6 +267,9 @@ public:
   set_bool_option (enum gcc_jit_bool_option opt,
 		   int value);
 
+  int
+  get_bool_option (enum gcc_jit_bool_option opt);
+
   void
   set_inner_bool_option (enum inner_bool_option inner_opt,
 			 int value);
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 5a1308b2b8c..3c5ff5a2a59 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -1222,10 +1222,13 @@ gcc_jit_context_new_function (gcc_jit_context *ctxt,
      Eventually we'll need some way to interact with e.g. C++ name
      mangling.  */
   {
+    int special_chars_allowed
+      = ctxt->get_bool_option (GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES);
     /* Leading char: */
     char ch = *name;
     RETURN_NULL_IF_FAIL_PRINTF2 (
-	ISALPHA (ch) || ch == '_',
+	ISALPHA (ch) || ch == '_' || (special_chars_allowed
+	  && (ch == '.' || ch == '$')),
 	ctxt, loc,
 	"name \"%s\" contains invalid character: '%c'",
 	name, ch);
@@ -1233,7 +1236,8 @@ gcc_jit_context_new_function (gcc_jit_context *ctxt,
     for (const char *ptr = name + 1; (ch = *ptr); ptr++)
       {
 	RETURN_NULL_IF_FAIL_PRINTF2 (
-	  ISALNUM (ch) || ch == '_',
+	  ISALNUM (ch) || ch == '_' || (special_chars_allowed
+	    && (ch == '.' || ch == '$')),
 	  ctxt, loc,
 	  "name \"%s\" contains invalid character: '%c'",
 	  name, ch);
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index d45b767c262..694f45f34e5 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -244,6 +244,9 @@ enum gcc_jit_bool_option
      their location on stderr.  */
   GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES,
 
+  /* If true, allow special characters like . and $ in function names.  */
+  GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES,
+
   GCC_JIT_NUM_BOOL_OPTIONS
 };
 
diff --git a/gcc/testsuite/jit.dg/test-special-chars.c b/gcc/testsuite/jit.dg/test-special-chars.c
new file mode 100644
index 00000000000..1bca71164d8
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-special-chars.c
@@ -0,0 +1,41 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  gcc_jit_context_set_bool_option (ctxt,
+    GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES, 1);
+
+  /* Let's try to inject the equivalent of:
+       void
+       name$with.special_chars (void)
+       {
+       }
+   */
+  gcc_jit_type *void_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+
+  /* Build the test_fn.  */
+  gcc_jit_function *test_fn =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  void_type,
+				  "name$with.special_chars",
+				  0, NULL,
+				  0);
+
+  gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL);
+  gcc_jit_block_end_with_void_return (
+    block, NULL);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+}
-- 
2.43.0

Re: [PATCH] libgccjit: Add option to allow special characters in function names

[David Malcolm]

On Thu, 2024-02-15 at 17:08 -0500, Antoni Boucher wrote:
> Hi.
> This patch adds a new option to allow special characters like . and $
> in function names.
> This is useful to allow for mangling using those characters.
> Thanks for the review.

Thanks for the patch.

> diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
> index 10a0e50f9f6..4af75ea7418 100644
> --- a/gcc/jit/docs/topics/contexts.rst
> +++ b/gcc/jit/docs/topics/contexts.rst
> @@ -453,6 +453,10 @@ Boolean options
>       If true, the :type:`gcc_jit_context` will not clean up intermediate files
>       written to the filesystem, and will display their location on stderr.
>  
> +  .. macro:: GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
> +
> +     If true, allow special characters like . and $ in function names.

The documentation and the comment in libgccjit.h say:
  "allow special characters like . and $ in function names."
and on reading the implementation, the special characters are exactly
'.' and '$'.

The API seems rather arbitrary and inflexible to me; why the choice of
those characters?  Presumably those are the ones that Rust's mangling
scheme uses, but do other mangling schemes require other chars?

How about an API for setting the valid chars, something like:

extern void
gcc_jit_context_set_valid_symbol_chars (gcc_jit_context *ctxt,
                                        const char *chars);

to specify the chars that are valid in addition to underscore and
alphanumeric.

In your case you'd call:

  gcc_jit_context_set_valid_symbol_chars (ctxt, ".$");

Or is that overkill?

Dave

Re: [PATCH] libgccjit: Add option to allow special characters in function names

[Iain Sandoe]

> On 20 Feb 2024, at 20:50, David Malcolm <dmalcolm@redhat.com> wrote:
> 
> On Thu, 2024-02-15 at 17:08 -0500, Antoni Boucher wrote:
>> Hi.
>> This patch adds a new option to allow special characters like . and $
>> in function names.
>> This is useful to allow for mangling using those characters.
>> Thanks for the review.
> 
> Thanks for the patch.
> 
>> diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
>> index 10a0e50f9f6..4af75ea7418 100644
>> --- a/gcc/jit/docs/topics/contexts.rst
>> +++ b/gcc/jit/docs/topics/contexts.rst
>> @@ -453,6 +453,10 @@ Boolean options
>>      If true, the :type:`gcc_jit_context` will not clean up intermediate files
>>      written to the filesystem, and will display their location on stderr.
>> 
>> +  .. macro:: GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
>> +
>> +     If true, allow special characters like . and $ in function names.
> 
> The documentation and the comment in libgccjit.h say:
>  "allow special characters like . and $ in function names."
> and on reading the implementation, the special characters are exactly
> '.' and '$'.
> 
> The API seems rather arbitrary and inflexible to me; why the choice of
> those characters?  Presumably those are the ones that Rust's mangling
> scheme uses, but do other mangling schemes require other chars?
> 
> How about an API for setting the valid chars, something like:
> 
> extern void
> gcc_jit_context_set_valid_symbol_chars (gcc_jit_context *ctxt,
>                                        const char *chars);
> 
> to specify the chars that are valid in addition to underscore and
> alphanumeric.
> 
> In your case you'd call:
> 
>  gcc_jit_context_set_valid_symbol_chars (ctxt, ".$");
> 
> Or is that overkill?

If we ever wanted to support objective-c (NeXT runtime) then we’d need to
be able to support +,-,[,] space and : at least.  The interesting thing there is
that most assemblers do not support that either (and the symbols then need
to be quoted into the assembler) .

So, it’s not (IMO) overkill considering at least one potential extension.

Iain

> 
> Dave
> 

Re: [PATCH] libgccjit: Add option to allow special characters in function names

[Antoni Boucher]

[-- Attachment #1: Type: text/plain, Size: 2456 bytes --]

Hi and thanks for the review.
I thought it would be a bit weird to have an option to change which 
characters are allowed, but I can't think of a better solution.
Here's the updated patch that now allow arbitrary characters.

Le 2024-02-20 à 16 h 05, Iain Sandoe a écrit :
> 
> 
>> On 20 Feb 2024, at 20:50, David Malcolm <dmalcolm@redhat.com> wrote:
>>
>> On Thu, 2024-02-15 at 17:08 -0500, Antoni Boucher wrote:
>>> Hi.
>>> This patch adds a new option to allow special characters like . and $
>>> in function names.
>>> This is useful to allow for mangling using those characters.
>>> Thanks for the review.
>>
>> Thanks for the patch.
>>
>>> diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
>>> index 10a0e50f9f6..4af75ea7418 100644
>>> --- a/gcc/jit/docs/topics/contexts.rst
>>> +++ b/gcc/jit/docs/topics/contexts.rst
>>> @@ -453,6 +453,10 @@ Boolean options
>>>       If true, the :type:`gcc_jit_context` will not clean up intermediate files
>>>       written to the filesystem, and will display their location on stderr.
>>>
>>> +  .. macro:: GCC_JIT_BOOL_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
>>> +
>>> +     If true, allow special characters like . and $ in function names.
>>
>> The documentation and the comment in libgccjit.h say:
>>   "allow special characters like . and $ in function names."
>> and on reading the implementation, the special characters are exactly
>> '.' and '$'.
>>
>> The API seems rather arbitrary and inflexible to me; why the choice of
>> those characters?  Presumably those are the ones that Rust's mangling
>> scheme uses, but do other mangling schemes require other chars?
>>
>> How about an API for setting the valid chars, something like:
>>
>> extern void
>> gcc_jit_context_set_valid_symbol_chars (gcc_jit_context *ctxt,
>>                                         const char *chars);
>>
>> to specify the chars that are valid in addition to underscore and
>> alphanumeric.
>>
>> In your case you'd call:
>>
>>   gcc_jit_context_set_valid_symbol_chars (ctxt, ".$");
>>
>> Or is that overkill?
> 
> If we ever wanted to support objective-c (NeXT runtime) then we’d need to
> be able to support +,-,[,] space and : at least.  The interesting thing there is
> that most assemblers do not support that either (and the symbols then need
> to be quoted into the assembler) .
> 
> So, it’s not (IMO) overkill considering at least one potential extension.
> 
> Iain
> 
>>
>> Dave
>>
> 

[-- Attachment #2: 0001-libgccjit-Add-option-to-allow-special-characters-in-.patch --]
[-- Type: text/x-patch, Size: 6381 bytes --]

From 1bd8a95ffd8e30bf3b65a2948f9ebba22e691bd6 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Thu, 15 Feb 2024 17:03:22 -0500
Subject: [PATCH] libgccjit: Add option to allow special characters in function
 names

gcc/jit/ChangeLog:

	* docs/topics/contexts.rst: Add documentation for new option.
	* jit-recording.cc (recording::context::get_str_option): New
	method.
	* jit-recording.h (get_str_option): New method.
	* libgccjit.cc (gcc_jit_context_new_function): Allow special
	characters in function names.
	* libgccjit.h (enum gcc_jit_str_option): New option.

gcc/testsuite/ChangeLog:

	* jit.dg/test-special-chars.c: New test.
---
 gcc/jit/docs/topics/contexts.rst          |  8 +++--
 gcc/jit/jit-recording.cc                  | 17 ++++++++--
 gcc/jit/jit-recording.h                   |  3 ++
 gcc/jit/libgccjit.cc                      |  8 +++--
 gcc/jit/libgccjit.h                       |  3 ++
 gcc/testsuite/jit.dg/test-special-chars.c | 41 +++++++++++++++++++++++
 6 files changed, 74 insertions(+), 6 deletions(-)
 create mode 100644 gcc/testsuite/jit.dg/test-special-chars.c

diff --git a/gcc/jit/docs/topics/contexts.rst b/gcc/jit/docs/topics/contexts.rst
index 10a0e50f9f6..e7950cee961 100644
--- a/gcc/jit/docs/topics/contexts.rst
+++ b/gcc/jit/docs/topics/contexts.rst
@@ -317,13 +317,17 @@ String Options
    copy of the underlying string, so it is valid to pass in a pointer to
    an on-stack buffer.
 
-   There is just one string option specified this way:
-
    .. macro:: GCC_JIT_STR_OPTION_PROGNAME
 
       The name of the program, for use as a prefix when printing error
       messages to stderr.  If `NULL`, or default, "libgccjit.so" is used.
 
+  .. macro:: GCC_JIT_STR_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES
+
+      Special characters to allow in function names.
+      This string contains all characters that should not be rejected by
+      libgccjit. Ex.: ".$"
+
 Boolean options
 ***************
 
diff --git a/gcc/jit/jit-recording.cc b/gcc/jit/jit-recording.cc
index 68a2e860c1f..a656b9a0a98 100644
--- a/gcc/jit/jit-recording.cc
+++ b/gcc/jit/jit-recording.cc
@@ -1362,6 +1362,18 @@ recording::context::set_str_option (enum gcc_jit_str_option opt,
   log_str_option (opt);
 }
 
+const char*
+recording::context::get_str_option (enum gcc_jit_str_option opt)
+{
+  if (opt < 0 || opt >= GCC_JIT_NUM_STR_OPTIONS)
+    {
+      add_error (NULL,
+		 "unrecognized (enum gcc_jit_str_option) value: %i", opt);
+      return NULL;
+    }
+  return m_str_options[opt];
+}
+
 /* Set the given integer option for this context, or add an error if
    it's not recognized.
 
@@ -1703,7 +1715,8 @@ recording::context::dump_to_file (const char *path, bool update_locations)
 
 static const char * const
  str_option_reproducer_strings[GCC_JIT_NUM_STR_OPTIONS] = {
-  "GCC_JIT_STR_OPTION_PROGNAME"
+  "GCC_JIT_STR_OPTION_PROGNAME",
+  "GCC_JIT_STR_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES",
 };
 
 static const char * const
@@ -1720,7 +1733,7 @@ static const char * const
   "GCC_JIT_BOOL_OPTION_DUMP_SUMMARY",
   "GCC_JIT_BOOL_OPTION_DUMP_EVERYTHING",
   "GCC_JIT_BOOL_OPTION_SELFCHECK_GC",
-  "GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES"
+  "GCC_JIT_BOOL_OPTION_KEEP_INTERMEDIATES",
 };
 
 static const char * const
diff --git a/gcc/jit/jit-recording.h b/gcc/jit/jit-recording.h
index d8d16f4fe29..1799f0ee434 100644
--- a/gcc/jit/jit-recording.h
+++ b/gcc/jit/jit-recording.h
@@ -231,6 +231,9 @@ public:
   set_str_option (enum gcc_jit_str_option opt,
 		  const char *value);
 
+  const char*
+  get_str_option (enum gcc_jit_str_option opt);
+
   void
   set_int_option (enum gcc_jit_int_option opt,
 		  int value);
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index f40a9781405..b866dc17646 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -1191,10 +1191,13 @@ gcc_jit_context_new_function (gcc_jit_context *ctxt,
      Eventually we'll need some way to interact with e.g. C++ name
      mangling.  */
   {
+    const char* special_chars_allowed
+      = ctxt->get_str_option (GCC_JIT_STR_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES);
     /* Leading char: */
     char ch = *name;
     RETURN_NULL_IF_FAIL_PRINTF2 (
-	ISALPHA (ch) || ch == '_',
+	ISALPHA (ch) || ch == '_' || (special_chars_allowed
+	  && strchr (special_chars_allowed, ch)),
 	ctxt, loc,
 	"name \"%s\" contains invalid character: '%c'",
 	name, ch);
@@ -1202,7 +1205,8 @@ gcc_jit_context_new_function (gcc_jit_context *ctxt,
     for (const char *ptr = name + 1; (ch = *ptr); ptr++)
       {
 	RETURN_NULL_IF_FAIL_PRINTF2 (
-	  ISALNUM (ch) || ch == '_',
+	  ISALNUM (ch) || ch == '_' || (special_chars_allowed
+	    && strchr (special_chars_allowed, ch)),
 	  ctxt, loc,
 	  "name \"%s\" contains invalid character: '%c'",
 	  name, ch);
diff --git a/gcc/jit/libgccjit.h b/gcc/jit/libgccjit.h
index 74e847b2dec..1239d9a3e19 100644
--- a/gcc/jit/libgccjit.h
+++ b/gcc/jit/libgccjit.h
@@ -172,6 +172,9 @@ enum gcc_jit_str_option
      messages to stderr.  If NULL, or default, "libgccjit.so" is used.  */
   GCC_JIT_STR_OPTION_PROGNAME,
 
+  /* Special characters to allow in function names.  */
+  GCC_JIT_STR_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES,
+
   GCC_JIT_NUM_STR_OPTIONS
 };
 
diff --git a/gcc/testsuite/jit.dg/test-special-chars.c b/gcc/testsuite/jit.dg/test-special-chars.c
new file mode 100644
index 00000000000..8cd77323050
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-special-chars.c
@@ -0,0 +1,41 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+  gcc_jit_context_set_str_option (ctxt,
+    GCC_JIT_STR_OPTION_SPECIAL_CHARS_IN_FUNC_NAMES, "$.");
+
+  /* Let's try to inject the equivalent of:
+       void
+       name$with.special_chars (void)
+       {
+       }
+   */
+  gcc_jit_type *void_type =
+    gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+
+  /* Build the test_fn.  */
+  gcc_jit_function *test_fn =
+    gcc_jit_context_new_function (ctxt, NULL,
+				  GCC_JIT_FUNCTION_EXPORTED,
+				  void_type,
+				  "name$with.special_chars",
+				  0, NULL,
+				  0);
+
+  gcc_jit_block *block = gcc_jit_function_new_block (test_fn, NULL);
+  gcc_jit_block_end_with_void_return (
+    block, NULL);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+  CHECK_NON_NULL (result);
+}
-- 
2.44.0