* [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
@ 2023-12-07 22:34 Antoni Boucher
2023-12-08 1:15 ` David Malcolm
0 siblings, 1 reply; 4+ messages in thread
From: Antoni Boucher @ 2023-12-07 22:34 UTC (permalink / raw)
To: jit, gcc-patches; +Cc: David Malcolm
[-- Attachment #1: Type: text/plain, Size: 232 bytes --]
Hi.
This patch adds checks gcc_jit_block_add_assignment_op to make sure it
is only ever called on numeric types.
With the previous patch, this might require a change to also allow
vector types here.
Thanks for the review.
[-- Attachment #2: 0001-libgccjit-Add-type-checks-in-gcc_jit_block_add_assig.patch --]
[-- Type: text/x-patch, Size: 4440 bytes --]
From 932048619671c61af224708a3da484b9f54a30a3 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Wed, 18 Oct 2023 18:33:18 -0400
Subject: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
gcc/jit/ChangeLog:
* libgccjit.cc (RETURN_IF_FAIL_PRINTF3): New macro.
(gcc_jit_block_add_assignment_op): Add numeric checks.
gcc/testsuite/ChangeLog:
* jit.dg/test-error-bad-assignment-op.c: New test.
---
gcc/jit/libgccjit.cc | 21 +++++++
.../jit.dg/test-error-bad-assignment-op.c | 57 +++++++++++++++++++
2 files changed, 78 insertions(+)
create mode 100644 gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 0451b4df7f9..eb6817a0a99 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -267,6 +267,16 @@ struct gcc_jit_extended_asm : public gcc::jit::recording::extended_asm
} \
JIT_END_STMT
+#define RETURN_IF_FAIL_PRINTF3(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2) \
+ JIT_BEGIN_STMT \
+ if (!(TEST_EXPR)) \
+ { \
+ jit_error ((CTXT), (LOC), "%s: " ERR_FMT, \
+ __func__, (A0), (A1), (A2)); \
+ return; \
+ } \
+ JIT_END_STMT
+
#define RETURN_IF_FAIL_PRINTF4(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3) \
JIT_BEGIN_STMT \
if (!(TEST_EXPR)) \
@@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op (gcc_jit_block *block,
lvalue->get_type ()->get_debug_string (),
rvalue->get_debug_string (),
rvalue->get_type ()->get_debug_string ());
+ // TODO: check if it is a numeric vector?
+ RETURN_IF_FAIL_PRINTF3 (
+ lvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric (), ctxt, loc,
+ "gcc_jit_block_add_assignment_op %s has non-numeric lvalue %s (type: %s)",
+ gcc::jit::binary_op_reproducer_strings[op],
+ lvalue->get_debug_string (), lvalue->get_type ()->get_debug_string ());
+ RETURN_IF_FAIL_PRINTF3 (
+ rvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric (), ctxt, loc,
+ "gcc_jit_block_add_assignment_op %s has non-numeric rvalue %s (type: %s)",
+ gcc::jit::binary_op_reproducer_strings[op],
+ rvalue->get_debug_string (), rvalue->get_type ()->get_debug_string ());
gcc::jit::recording::statement *stmt = block->add_assignment_op (loc, lvalue, op, rvalue);
diff --git a/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c b/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
new file mode 100644
index 00000000000..683ebbfb1fe
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
@@ -0,0 +1,57 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+
+ void
+ test_fn ()
+ {
+ const char *variable;
+ variable += "test";
+ }
+
+ and verify that the API complains about the mismatching types
+ in the assignments.
+ */
+ gcc_jit_type *void_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+ gcc_jit_type *const_char_ptr_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CONST_CHAR_PTR);
+
+ gcc_jit_function *func =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ void_type,
+ "test_fn",
+ 0, NULL,
+ 0);
+
+ gcc_jit_lvalue *variable = gcc_jit_function_new_local (func, NULL, const_char_ptr_type, "variable");
+ gcc_jit_block *initial =
+ gcc_jit_function_new_block (func, "initial");
+ gcc_jit_rvalue *string =
+ gcc_jit_context_new_string_literal (ctxt, "test");
+ gcc_jit_block_add_assignment_op (initial, NULL, variable, GCC_JIT_BINARY_OP_PLUS, string);
+
+ gcc_jit_block_end_with_void_return (initial, NULL);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ CHECK_VALUE (result, NULL);
+
+ /* Verify that the correct error messages were emitted. */
+ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt),
+ "gcc_jit_block_add_assignment_op:"
+ " gcc_jit_block_add_assignment_op GCC_JIT_BINARY_OP_PLUS"
+ " has non-numeric lvalue variable (type: const char *)");
+}
+
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
2023-12-07 22:34 [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op Antoni Boucher
@ 2023-12-08 1:15 ` David Malcolm
2023-12-21 13:36 ` Antoni Boucher
0 siblings, 1 reply; 4+ messages in thread
From: David Malcolm @ 2023-12-08 1:15 UTC (permalink / raw)
To: Antoni Boucher, jit, gcc-patches
On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote:
> Hi.
> This patch adds checks gcc_jit_block_add_assignment_op to make sure
> it
> is only ever called on numeric types.
>
> With the previous patch, this might require a change to also allow
> vector types here.
>
> Thanks for the review.
Thanks for the patch.
[...snip...]
> @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op (gcc_jit_block *block,
> lvalue->get_type ()->get_debug_string (),
> rvalue->get_debug_string (),
> rvalue->get_type ()->get_debug_string ());
> + // TODO: check if it is a numeric vector?
> + RETURN_IF_FAIL_PRINTF3 (
> + lvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric (), ctxt, loc,
> + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue %s (type: %s)",
> + gcc::jit::binary_op_reproducer_strings[op],
> + lvalue->get_debug_string (), lvalue->get_type ()->get_debug_string ());
The condition being tested here should probably just be:
lvalue->get_type ()->is_numeric ()
since otherwise if the lvalue's type is numeric and the rvalue's type
fails to be, then the user would incorrectly get a message about the
lvalue.
> + RETURN_IF_FAIL_PRINTF3 (
> + rvalue->get_type ()->is_numeric () && rvalue->get_type ()->is_numeric (), ctxt, loc,
> + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue %s (type: %s)",
> + gcc::jit::binary_op_reproducer_strings[op],
> + rvalue->get_debug_string (), rvalue->get_type ()->get_debug_string ());
The condition being tested here seems to have a redundant repeated:
&& rvalue->get_type ()->is_numeric ()
Am I missing something, or is that a typo?
[...snip...]
The patch is OK otherwise.
Thanks
Dave
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
2023-12-08 1:15 ` David Malcolm
@ 2023-12-21 13:36 ` Antoni Boucher
2024-02-17 16:54 ` Antoni Boucher
0 siblings, 1 reply; 4+ messages in thread
From: Antoni Boucher @ 2023-12-21 13:36 UTC (permalink / raw)
To: David Malcolm, jit, gcc-patches
[-- Attachment #1: Type: text/plain, Size: 2069 bytes --]
Hi.
Here's the updated patch.
Thanks.
On Thu, 2023-12-07 at 20:15 -0500, David Malcolm wrote:
> On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote:
> > Hi.
> > This patch adds checks gcc_jit_block_add_assignment_op to make sure
> > it
> > is only ever called on numeric types.
> >
> > With the previous patch, this might require a change to also allow
> > vector types here.
> >
> > Thanks for the review.
>
> Thanks for the patch.
>
> [...snip...]
>
> > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op
> > (gcc_jit_block *block,
> > lvalue->get_type ()->get_debug_string (),
> > rvalue->get_debug_string (),
> > rvalue->get_type ()->get_debug_string ());
> > + // TODO: check if it is a numeric vector?
> > + RETURN_IF_FAIL_PRINTF3 (
> > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()-
> > >is_numeric (), ctxt, loc,
> > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue %s
> > (type: %s)",
> > + gcc::jit::binary_op_reproducer_strings[op],
> > + lvalue->get_debug_string (), lvalue->get_type ()-
> > >get_debug_string ());
>
> The condition being tested here should probably just be:
>
> lvalue->get_type ()->is_numeric ()
>
> since otherwise if the lvalue's type is numeric and the rvalue's type
> fails to be, then the user would incorrectly get a message about the
> lvalue.
>
> > + RETURN_IF_FAIL_PRINTF3 (
> > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()-
> > >is_numeric (), ctxt, loc,
> > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue %s
> > (type: %s)",
> > + gcc::jit::binary_op_reproducer_strings[op],
> > + rvalue->get_debug_string (), rvalue->get_type ()-
> > >get_debug_string ());
>
> The condition being tested here seems to have a redundant repeated:
> && rvalue->get_type ()->is_numeric ()
>
> Am I missing something, or is that a typo?
>
> [...snip...]
>
> The patch is OK otherwise.
>
> Thanks
> Dave
>
>
>
[-- Attachment #2: 0001-libgccjit-Add-type-checks-in-gcc_jit_block_add_assig.patch --]
[-- Type: text/x-patch, Size: 4363 bytes --]
From a93b029db4622ff6385715ff9cdaf1be5ffa5657 Mon Sep 17 00:00:00 2001
From: Antoni Boucher <bouanto@zoho.com>
Date: Wed, 18 Oct 2023 18:33:18 -0400
Subject: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
gcc/jit/ChangeLog:
* libgccjit.cc (RETURN_IF_FAIL_PRINTF3): New macro.
(gcc_jit_block_add_assignment_op): Add numeric checks.
gcc/testsuite/ChangeLog:
* jit.dg/test-error-bad-assignment-op.c: New test.
---
gcc/jit/libgccjit.cc | 21 +++++++
.../jit.dg/test-error-bad-assignment-op.c | 57 +++++++++++++++++++
2 files changed, 78 insertions(+)
create mode 100644 gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
diff --git a/gcc/jit/libgccjit.cc b/gcc/jit/libgccjit.cc
index 0451b4df7f9..10d23e7fcf6 100644
--- a/gcc/jit/libgccjit.cc
+++ b/gcc/jit/libgccjit.cc
@@ -267,6 +267,16 @@ struct gcc_jit_extended_asm : public gcc::jit::recording::extended_asm
} \
JIT_END_STMT
+#define RETURN_IF_FAIL_PRINTF3(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2) \
+ JIT_BEGIN_STMT \
+ if (!(TEST_EXPR)) \
+ { \
+ jit_error ((CTXT), (LOC), "%s: " ERR_FMT, \
+ __func__, (A0), (A1), (A2)); \
+ return; \
+ } \
+ JIT_END_STMT
+
#define RETURN_IF_FAIL_PRINTF4(TEST_EXPR, CTXT, LOC, ERR_FMT, A0, A1, A2, A3) \
JIT_BEGIN_STMT \
if (!(TEST_EXPR)) \
@@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op (gcc_jit_block *block,
lvalue->get_type ()->get_debug_string (),
rvalue->get_debug_string (),
rvalue->get_type ()->get_debug_string ());
+ // TODO: check if it is a numeric vector?
+ RETURN_IF_FAIL_PRINTF3 (
+ lvalue->get_type ()->is_numeric (), ctxt, loc,
+ "gcc_jit_block_add_assignment_op %s has non-numeric lvalue %s (type: %s)",
+ gcc::jit::binary_op_reproducer_strings[op],
+ lvalue->get_debug_string (), lvalue->get_type ()->get_debug_string ());
+ RETURN_IF_FAIL_PRINTF3 (
+ rvalue->get_type ()->is_numeric (), ctxt, loc,
+ "gcc_jit_block_add_assignment_op %s has non-numeric rvalue %s (type: %s)",
+ gcc::jit::binary_op_reproducer_strings[op],
+ rvalue->get_debug_string (), rvalue->get_type ()->get_debug_string ());
gcc::jit::recording::statement *stmt = block->add_assignment_op (loc, lvalue, op, rvalue);
diff --git a/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c b/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
new file mode 100644
index 00000000000..683ebbfb1fe
--- /dev/null
+++ b/gcc/testsuite/jit.dg/test-error-bad-assignment-op.c
@@ -0,0 +1,57 @@
+#include <stdlib.h>
+#include <stdio.h>
+
+#include "libgccjit.h"
+
+#include "harness.h"
+
+void
+create_code (gcc_jit_context *ctxt, void *user_data)
+{
+ /* Let's try to inject the equivalent of:
+
+ void
+ test_fn ()
+ {
+ const char *variable;
+ variable += "test";
+ }
+
+ and verify that the API complains about the mismatching types
+ in the assignments.
+ */
+ gcc_jit_type *void_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_VOID);
+ gcc_jit_type *const_char_ptr_type =
+ gcc_jit_context_get_type (ctxt, GCC_JIT_TYPE_CONST_CHAR_PTR);
+
+ gcc_jit_function *func =
+ gcc_jit_context_new_function (ctxt, NULL,
+ GCC_JIT_FUNCTION_EXPORTED,
+ void_type,
+ "test_fn",
+ 0, NULL,
+ 0);
+
+ gcc_jit_lvalue *variable = gcc_jit_function_new_local (func, NULL, const_char_ptr_type, "variable");
+ gcc_jit_block *initial =
+ gcc_jit_function_new_block (func, "initial");
+ gcc_jit_rvalue *string =
+ gcc_jit_context_new_string_literal (ctxt, "test");
+ gcc_jit_block_add_assignment_op (initial, NULL, variable, GCC_JIT_BINARY_OP_PLUS, string);
+
+ gcc_jit_block_end_with_void_return (initial, NULL);
+}
+
+void
+verify_code (gcc_jit_context *ctxt, gcc_jit_result *result)
+{
+ CHECK_VALUE (result, NULL);
+
+ /* Verify that the correct error messages were emitted. */
+ CHECK_STRING_VALUE (gcc_jit_context_get_first_error (ctxt),
+ "gcc_jit_block_add_assignment_op:"
+ " gcc_jit_block_add_assignment_op GCC_JIT_BINARY_OP_PLUS"
+ " has non-numeric lvalue variable (type: const char *)");
+}
+
--
2.43.0
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op
2023-12-21 13:36 ` Antoni Boucher
@ 2024-02-17 16:54 ` Antoni Boucher
0 siblings, 0 replies; 4+ messages in thread
From: Antoni Boucher @ 2024-02-17 16:54 UTC (permalink / raw)
To: David Malcolm, jit, gcc-patches
David: Ping.
On Thu, 2023-12-21 at 08:36 -0500, Antoni Boucher wrote:
> Hi.
> Here's the updated patch.
> Thanks.
>
> On Thu, 2023-12-07 at 20:15 -0500, David Malcolm wrote:
> > On Thu, 2023-12-07 at 17:34 -0500, Antoni Boucher wrote:
> > > Hi.
> > > This patch adds checks gcc_jit_block_add_assignment_op to make
> > > sure
> > > it
> > > is only ever called on numeric types.
> > >
> > > With the previous patch, this might require a change to also
> > > allow
> > > vector types here.
> > >
> > > Thanks for the review.
> >
> > Thanks for the patch.
> >
> > [...snip...]
> >
> > > @@ -2890,6 +2900,17 @@ gcc_jit_block_add_assignment_op
> > > (gcc_jit_block *block,
> > > lvalue->get_type ()->get_debug_string (),
> > > rvalue->get_debug_string (),
> > > rvalue->get_type ()->get_debug_string ());
> > > + // TODO: check if it is a numeric vector?
> > > + RETURN_IF_FAIL_PRINTF3 (
> > > + lvalue->get_type ()->is_numeric () && rvalue->get_type ()-
> > > > is_numeric (), ctxt, loc,
> > > + "gcc_jit_block_add_assignment_op %s has non-numeric lvalue
> > > %s
> > > (type: %s)",
> > > + gcc::jit::binary_op_reproducer_strings[op],
> > > + lvalue->get_debug_string (), lvalue->get_type ()-
> > > > get_debug_string ());
> >
> > The condition being tested here should probably just be:
> >
> > lvalue->get_type ()->is_numeric ()
> >
> > since otherwise if the lvalue's type is numeric and the rvalue's
> > type
> > fails to be, then the user would incorrectly get a message about
> > the
> > lvalue.
> >
> > > + RETURN_IF_FAIL_PRINTF3 (
> > > + rvalue->get_type ()->is_numeric () && rvalue->get_type ()-
> > > > is_numeric (), ctxt, loc,
> > > + "gcc_jit_block_add_assignment_op %s has non-numeric rvalue
> > > %s
> > > (type: %s)",
> > > + gcc::jit::binary_op_reproducer_strings[op],
> > > + rvalue->get_debug_string (), rvalue->get_type ()-
> > > > get_debug_string ());
> >
> > The condition being tested here seems to have a redundant repeated:
> > && rvalue->get_type ()->is_numeric ()
> >
> > Am I missing something, or is that a typo?
> >
> > [...snip...]
> >
> > The patch is OK otherwise.
> >
> > Thanks
> > Dave
> >
> >
> >
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-17 16:54 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-07 22:34 [PATCH] libgccjit: Add type checks in gcc_jit_block_add_assignment_op Antoni Boucher
2023-12-08 1:15 ` David Malcolm
2023-12-21 13:36 ` Antoni Boucher
2024-02-17 16:54 ` Antoni Boucher
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).