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 <dmalc...@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
From 1bd8a95ffd8e30bf3b65a2948f9ebba22e691bd6 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <boua...@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
