| Message ID | tencent_AE90AA59903A628E87E9F80E563DA5BA5508@qq.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | riscv: ftrace: Add parentheses in macro definitions of make_call_t0 and make_call_ra | expand |
| Context | Check | Description |
|---|---|---|
| bjorn/pre-ci_am | success | Success |
| bjorn/build-rv32-defconfig | success | build-rv32-defconfig |
| bjorn/build-rv64-clang-allmodconfig | success | build-rv64-clang-allmodconfig |
| bjorn/build-rv64-gcc-allmodconfig | success | build-rv64-gcc-allmodconfig |
| bjorn/build-rv64-nommu-k210-defconfig | success | build-rv64-nommu-k210-defconfig |
| bjorn/build-rv64-nommu-k210-virt | success | build-rv64-nommu-k210-virt |
| bjorn/checkpatch | warning | checkpatch |
| bjorn/dtb-warn-rv64 | success | dtb-warn-rv64 |
| bjorn/header-inline | success | header-inline |
| bjorn/kdoc | success | kdoc |
| bjorn/module-param | success | module-param |
| bjorn/verify-fixes | success | verify-fixes |
| bjorn/verify-signedoff | success | verify-signedoff |
Hi Juhan, On 06/02/2025 20:28, Juhan Jin wrote: > This patch adds parentheses to parameters caller and callee of macros > make_call_t0 and make_call_ra. Every existing invocation of these two > macros uses a single variable for each argument, so the absence of the > parentheses seems okay. However, future invocations might use more > complex expressions as arguments. For example, a future invocation might > look like this: make_call_t0(a - b, c, call). Without parentheses in the > macro definition, the macro invocation expands to: > > ... > unsigned int offset = (unsigned long) c - (unsigned long) a - b; > ... > > which is clearly wrong. > > The use of parentheses ensures arguments are correctly evaluated and > potentially saves future users of make_call_t0 and make_call_ra debugging > trouble. > > Fixes: 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half") > Signed-off-by: Juhan Jin <juhan.jin@foxmail.com> > --- > arch/riscv/include/asm/ftrace.h | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h > index c4721ce44ca4..2636ee00ccf0 100644 > --- a/arch/riscv/include/asm/ftrace.h > +++ b/arch/riscv/include/asm/ftrace.h > @@ -92,7 +92,7 @@ struct dyn_arch_ftrace { > #define make_call_t0(caller, callee, call) \ > do { \ > unsigned int offset = \ > - (unsigned long) callee - (unsigned long) caller; \ > + (unsigned long) (callee) - (unsigned long) (caller); \ > call[0] = to_auipc_t0(offset); \ > call[1] = to_jalr_t0(offset); \ > } while (0) > @@ -108,7 +108,7 @@ do { \ > #define make_call_ra(caller, callee, call) \ > do { \ > unsigned int offset = \ > - (unsigned long) callee - (unsigned long) caller; \ > + (unsigned long) (callee) - (unsigned long) (caller); \ > call[0] = to_auipc_ra(offset); \ > call[1] = to_jalr_ra(offset); \ > } while (0) > > base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b Probably not worth going into fixes since the problem is not around for now, but that's still a good catch: Reviewed-by: Alexandre Ghiti <alexghiti@rivosinc.com> Thanks, Alex
diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h index c4721ce44ca4..2636ee00ccf0 100644 --- a/arch/riscv/include/asm/ftrace.h +++ b/arch/riscv/include/asm/ftrace.h @@ -92,7 +92,7 @@ struct dyn_arch_ftrace { #define make_call_t0(caller, callee, call) \ do { \ unsigned int offset = \ - (unsigned long) callee - (unsigned long) caller; \ + (unsigned long) (callee) - (unsigned long) (caller); \ call[0] = to_auipc_t0(offset); \ call[1] = to_jalr_t0(offset); \ } while (0) @@ -108,7 +108,7 @@ do { \ #define make_call_ra(caller, callee, call) \ do { \ unsigned int offset = \ - (unsigned long) callee - (unsigned long) caller; \ + (unsigned long) (callee) - (unsigned long) (caller); \ call[0] = to_auipc_ra(offset); \ call[1] = to_jalr_ra(offset); \ } while (0)
This patch adds parentheses to parameters caller and callee of macros make_call_t0 and make_call_ra. Every existing invocation of these two macros uses a single variable for each argument, so the absence of the parentheses seems okay. However, future invocations might use more complex expressions as arguments. For example, a future invocation might look like this: make_call_t0(a - b, c, call). Without parentheses in the macro definition, the macro invocation expands to: ... unsigned int offset = (unsigned long) c - (unsigned long) a - b; ... which is clearly wrong. The use of parentheses ensures arguments are correctly evaluated and potentially saves future users of make_call_t0 and make_call_ra debugging trouble. Fixes: 6724a76cff85 ("riscv: ftrace: Reduce the detour code size to half") Signed-off-by: Juhan Jin <juhan.jin@foxmail.com> --- arch/riscv/include/asm/ftrace.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b