On 03/10/2023 16:18, Victor Do Nascimento wrote:
In implementing the ACLE read/write system register builtins it was
observed that leaving argument type checking to be done at expand-time
meant that poorly-formed function calls were being "fixed" by certain
optimization passes, meaning bad code wasn't being properly picked up
in checking.
Example:
const char *regname = "amcgcr_el0";
long long a = __builtin_aarch64_rsr64 (regname);
is reduced by the ccp1 pass to
long long a = __builtin_aarch64_rsr64 ("amcgcr_el0");
As these functions require an argument of STRING_CST type, there needs
to be a check carried out by the front-end capable of picking this up.
The introduced `check_general_builtin_call' function will be called by
the TARGET_CHECK_BUILTIN_CALL hook whenever a call to a builtin
belonging to the AARCH64_BUILTIN_GENERAL category is encountered,
carrying out any appropriate checks associated with a particular
builtin function code.
Doesn't this prevent reasonable wrapping of the __builtin... names with
something more palatable? Eg:
static inline __attribute__(("always_inline")) long long get_sysreg_ll
(const char *regname)
{
return __builtin_aarch64_rsr64 (regname);
}
...
long long x = get_sysreg_ll("amcgcr_el0");
...
?
R.
gcc/ChangeLog:
* gcc/config/aarch64/aarch64-builtins.cc (check_general_builtin_call):
New.
* gcc/config/aarch64/aarch64-c.cc (aarch64_check_builtin_call):
Add check_general_builtin_call call.
* gcc/config/aarch64/aarch64-protos.h (check_general_builtin_call):
New.
gcc/testsuite/ChangeLog:
* gcc/testsuite/gcc.target/aarch64/acle/rwsr-2.c: New.
---
gcc/config/aarch64/aarch64-builtins.cc | 33 +++++++++++++++++++
gcc/config/aarch64/aarch64-c.cc | 4 +--
gcc/config/aarch64/aarch64-protos.h | 3 ++
.../gcc.target/aarch64/acle/rwsr-2.c | 15 +++++++++
4 files changed, 53 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/gcc.target/aarch64/acle/rwsr-2.c
diff --git a/gcc/config/aarch64/aarch64-builtins.cc
b/gcc/config/aarch64/aarch64-builtins.cc
index d8bb2a989a5..6734361f4f4 100644
--- a/gcc/config/aarch64/aarch64-builtins.cc
+++ b/gcc/config/aarch64/aarch64-builtins.cc
@@ -2126,6 +2126,39 @@ aarch64_general_builtin_decl (unsigned code, bool)
return aarch64_builtin_decls[code];
}
+bool
+check_general_builtin_call (location_t location, vec<location_t>,
+ unsigned int code, tree fndecl,
+ unsigned int nargs ATTRIBUTE_UNUSED, tree *args)
+{
+ switch (code)
+ {
+ case AARCH64_RSR:
+ case AARCH64_RSRP:
+ case AARCH64_RSR64:
+ case AARCH64_RSRF:
+ case AARCH64_RSRF64:
+ case AARCH64_WSR:
+ case AARCH64_WSRP:
+ case AARCH64_WSR64:
+ case AARCH64_WSRF:
+ case AARCH64_WSRF64:
+ if (TREE_CODE (args[0]) == VAR_DECL
+ || TREE_CODE (TREE_TYPE (args[0])) != POINTER_TYPE
+ || TREE_CODE (TREE_OPERAND (TREE_OPERAND (args[0], 0) , 0))
+ != STRING_CST)
+ {
+ const char *fn_name, *err_msg;
+ fn_name = IDENTIFIER_POINTER (DECL_NAME (fndecl));
+ err_msg = "first argument to %<%s%> must be a string literal";
+ error_at (location, err_msg, fn_name);
+ return false;
+ }
+ }
+ /* Default behavior. */
+ return true;
+}
+
typedef enum
{
SIMD_ARG_COPY_TO_REG,
diff --git a/gcc/config/aarch64/aarch64-c.cc b/gcc/config/aarch64/aarch64-c.cc
index 578ec6f45b0..6e2b83b8308 100644
--- a/gcc/config/aarch64/aarch64-c.cc
+++ b/gcc/config/aarch64/aarch64-c.cc
@@ -338,8 +338,8 @@ aarch64_check_builtin_call (location_t loc, vec<location_t>
arg_loc,
switch (code & AARCH64_BUILTIN_CLASS)
{
case AARCH64_BUILTIN_GENERAL:
- return true;
-
+ return check_general_builtin_call (loc, arg_loc, subcode, orig_fndecl,
+ nargs, args);
case AARCH64_BUILTIN_SVE:
return aarch64_sve::check_builtin_call (loc, arg_loc, subcode,
orig_fndecl, nargs, args);
diff --git a/gcc/config/aarch64/aarch64-protos.h
b/gcc/config/aarch64/aarch64-protos.h
index a134e2fcf8e..9ef96ff511f 100644
--- a/gcc/config/aarch64/aarch64-protos.h
+++ b/gcc/config/aarch64/aarch64-protos.h
@@ -990,6 +990,9 @@ tree aarch64_general_builtin_rsqrt (unsigned int);
void handle_arm_acle_h (void);
void handle_arm_neon_h (void);
+bool check_general_builtin_call (location_t, vec<location_t>, unsigned int,
+ tree, unsigned int, tree *);
+
namespace aarch64_sve {
void init_builtins ();
void handle_arm_sve_h ();
diff --git a/gcc/testsuite/gcc.target/aarch64/acle/rwsr-2.c
b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-2.c
new file mode 100644
index 00000000000..72e5fb75b21
--- /dev/null
+++ b/gcc/testsuite/gcc.target/aarch64/acle/rwsr-2.c
@@ -0,0 +1,15 @@
+/* Test the __arm_[r,w]sr ACLE intrinsics family. */
+/* Ensure that illegal behavior is rejected by the compiler. */
+
+/* { dg-do compile } */
+/* { dg-options "-O3 -march=armv8.4-a" } */
+
+#include <arm_acle.h>
+
+void
+test_non_const_sysreg_name ()
+{
+ const char *regname = "trcseqstr";
+ long long a = __arm_rsr64 (regname); /* { dg-error "first argument to
'__builtin_aarch64_rsr64' must be a string literal" } */
+ __arm_wsr64 (regname, a); /* { dg-error "first argument to
'__builtin_aarch64_wsr64' must be a string literal" } */
+}
