* [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).