On 10/9/23 22:13, Kito Cheng wrote:
The target attribute which proposed in [1], target attribute allow user
to specify a local setting per-function basis.
The syntax of target attribute is `__attribute__((target("<ATTR-STRING>")))`.
and the syntax of `<ATTR-STRING>` describes below:
```
ATTR-STRING := ATTR-STRING ';' ATTR
| ATTR
ATTR := ARCH-ATTR
| CPU-ATTR
| TUNE-ATTR
ARCH-ATTR := 'arch=' EXTENSIONS-OR-FULLARCH
EXTENSIONS-OR-FULLARCH := <EXTENSIONS>
| <FULLARCHSTR>
EXTENSIONS := <EXTENSION> ',' <EXTENSIONS>
| <EXTENSION>
FULLARCHSTR := <full-arch-string>
EXTENSION := <OP> <EXTENSION-NAME> <VERSION>
OP := '+'
VERSION := [0-9]+ 'p' [0-9]+
| [1-9][0-9]*
|
EXTENSION-NAME := Naming rule is defined in RISC-V ISA manual
CPU-ATTR := 'cpu=' <valid-cpu-name>
TUNE-ATTR := 'tune=' <valid-tune-name>
```
[1] https://github.com/riscv-non-isa/riscv-c-api-doc/pull/35
gcc/ChangeLog:
* config.gcc (riscv): Add riscv-target-attr.o.
* config/riscv/riscv-opts.h (TARGET_MIN_VLEN_OPTS): New.
* config/riscv/riscv-protos.h (riscv_declare_function_size) New.
(riscv_option_valid_attribute_p): New.
(riscv_override_options_internal): New.
(struct riscv_tune_info): New.
(riscv_parse_tune): New.
* config/riscv/riscv-target-attr.cc
(class riscv_target_attr_parser): New.
(struct riscv_attribute_info): New.
(riscv_attributes): New.
(riscv_target_attr_parser::parse_arch):
(riscv_target_attr_parser::handle_arch):
(riscv_target_attr_parser::handle_cpu):
(riscv_target_attr_parser::handle_tune):
(riscv_target_attr_parser::update_settings):
(riscv_process_one_target_attr):
(num_occurences_in_str):
(riscv_process_target_attr):
(riscv_option_valid_attribute_p):
* config/riscv/riscv.cc: Include target-globals.h and
riscv-subset.h.
(struct riscv_tune_info): Move to riscv-protos.h.
(get_tune_str):
(riscv_parse_tune):
(riscv_declare_function_size):
(riscv_option_override): Build target_option_default_node and
target_option_current_node.
(riscv_save_restore_target_globals):
(riscv_option_restore):
(riscv_previous_fndecl):
(riscv_set_current_function): Apply the target attribute.
(TARGET_OPTION_RESTORE): Define.
(TARGET_OPTION_VALID_ATTRIBUTE_P): Ditto.
* config/riscv/riscv.h (SWITCHABLE_TARGET): Define to 1.
(ASM_DECLARE_FUNCTION_SIZE) Define.
* config/riscv/riscv.opt (mtune=): Add Save attribute.
(mcpu=): Ditto.
(mcmodel=): Ditto.
* config/riscv/t-riscv: Add build rule for riscv-target-attr.o
* doc/extend.texi: Add doc for target attribute.
gcc/testsuite/ChangeLog:
* gcc.target/riscv/target-attr-01.c: New.
* gcc.target/riscv/target-attr-02.c: Ditto.
* gcc.target/riscv/target-attr-03.c: Ditto.
* gcc.target/riscv/target-attr-04.c: Ditto.
* gcc.target/riscv/target-attr-05.c: Ditto.
* gcc.target/riscv/target-attr-06.c: Ditto.
* gcc.target/riscv/target-attr-07.c: Ditto.
* gcc.target/riscv/target-attr-bad-01.c: Ditto.
* gcc.target/riscv/target-attr-bad-02.c: Ditto.
* gcc.target/riscv/target-attr-bad-03.c: Ditto.
* gcc.target/riscv/target-attr-bad-04.c: Ditto.
* gcc.target/riscv/target-attr-bad-05.c: Ditto.
* gcc.target/riscv/target-attr-bad-06.c: Ditto.
* gcc.target/riscv/target-attr-bad-07.c: Ditto.
* gcc.target/riscv/target-attr-warning-01.c: Ditto.
* gcc.target/riscv/target-attr-warning-02.c: Ditto.
* gcc.target/riscv/target-attr-warning-03.c: Ditto.
I probably would have split the save/restore state out as a separate
patch, but it's not a big deal. A bit surprised we didn't already have
that (and thus SWITCHABLE_TARGET) enabled already. But no worries about
that.
And just so I can do the right thing WRT RISE, what's the state of an
implementation for LLVM?
diff --git a/gcc/config/riscv/riscv-target-attr.cc
b/gcc/config/riscv/riscv-target-attr.cc
new file mode 100644
index 00000000000..33cff2c222f
--- /dev/null
+++ b/gcc/config/riscv/riscv-target-attr.cc
+ /* Parsing the extension list like "+<ext>[,+<ext>]*". */
+ size_t len = strlen (str);
+ char *str_to_check = (char *) alloca (len + 1);
If STR is under user control, then this could be a potential security
issue if they were to provide a crazy long string (at least until we
implement stack-clash protection -- next year).
In general we should avoid alloca unless we know the amount of memory
consumed is relatively small and we have a high degree of confidence the
code isn't going to be called inside a loop (and not inlined into a loop
in the caller). I might even go so far as to say no programmer should
ever use alloca.
Clearly I spent way too much time in my life fighting security issues
resulting from code mis-using alloca, even by folks who write pretty
good code in general.
+
+/* Parse ARG_STR which contains the definition of one target attribute.
+ Show appropriate errors if any or return true if the attribute is valid. */
+
+static bool
+riscv_process_one_target_attr (char *arg_str,
+ location_t loc,
+ riscv_target_attr_parser &attr_parser)
+{
+ size_t len = strlen (arg_str);
+
+ if (len == 0)
+ {
+ error_at (loc, "malformed %<target()%> attribute");
+ return false;
+ }
+
+ char *str_to_check = (char *) alloca (len + 1);
alloca again :-)
Generally OK. I would suggest avoiding the allocas. malloc is orders
of magnitude slower, but nowhere near as bad from a security standpoint.
Assuming you're agreeable to adjusting the code to avoid alloca, we'll
do a quick turnaround on the v3 -- I'll just audit the return paths to
make sure we don't leak and we'll be good to go.
jeff
