Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] openrisc: uninitialized return value causes kprobe test to unconditionally fail - [Medium] openrisc: flaky test in test_kprobes_adrp due to missing alignment --
commit d52f067996dd8c21d999dffa9e89c92071ac2e93 Author: Sahil Siddiq <[email protected]> openrisc: Add unit tests for KProbes on branch instructions Tests already exist lib/tests to verify the generic functionality of KProbes. Add unit tests to verify that KProbes work on instructions that require simulation in the OpenRISC ISA, and that the corresponding delay slots are handled correctly. > diff --git a/arch/openrisc/kernel/tests/test_kprobes_asm.S > b/arch/openrisc/kernel/tests/test_kprobes_asm.S > new file mode 100644 > index 0000000000000..a9fcf99b55ae1 > --- /dev/null > +++ b/arch/openrisc/kernel/tests/test_kprobes_asm.S [ ... ] > +SYM_FUNC_START(test_kprobes_adrp) > + l.addi r13,r0,0 > + l.addi r15,r9,0 > + l.addi r17,r0,-8192 > + l.jal 1f > + l.nop > +1: l.and r19,r9,r17 > +test_kprobes_adrp_addr: > + l.adrp r21,0 > + l.sfeq r21,r19 [Severity: Medium] Will this test be flaky due to a lack of alignment directives? The test verifies the l.adrp instruction by evaluating if it computes the same page base as the 1: label. It derives the page address of 1: by masking its address with -8192, and compares it to the page base computed by l.adrp on the next instruction. Since there is no alignment directive, the 1: label and the l.adrp instruction might straddle an 8KB page boundary during linkage (e.g., if 1: is at 0x1FFC and l.adrp is at 0x2000). If they cross the boundary, the two page bases won't match, causing the test to incorrectly branch away to the failure path. > + l.bnf 2f > + l.nop > + l.movhi r13,KPROBE_TEST_MAGIC_UPPER > + l.ori r13,r13,KPROBE_TEST_MAGIC_LOWER > +2: l.addi r9,r15,0 > + l.jr r9 [Severity: Medium] Does this function omit setting the return register, causing the kprobe test to unconditionally fail? The inline assembly successfully loads the magic value into r13 but omits the l.addi r11, r13, 0 instruction required by the OpenRISC calling convention to move it into the return register (r11) before returning. Because of this, test_kprobes_adrp() will return whatever garbage data was in r11. > + l.nop > +SYM_FUNC_END(test_kprobes_adrp) -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=3
