Re: exercising current aarch64 kprobe support with systemtap

[William Cohen <wcohen at redhat dot com>]

On 06/10/2016 10:37 AM, David Long wrote:
> On 06/10/2016 10:03 AM, William Cohen wrote:
>> On 06/10/2016 09:42 AM, Pratyush Anand wrote:
>>> On 10/06/2016:01:49:10 AM, David Long wrote:
>>>> Attached are incremental diffs I hope will fix the latest systemtap
>>>> failures, without abandoning atomic sequence checking.  I'm trying to avoid
>>>> the hex constants but I don't think the insn.c functions help in this case.
>>>
>>> It will save us from current problem by checking "stp x29,x30,[sp,...]"
>>> instruction and returning false if matches. However, we will have to find some
>>> way to recognize .word instructions.
>>>
>>> * An assembly function may not start with "stp x29,x30,[sp,...]", e.g.
>>>   __dma_map_area(), _cpu_resume etc. However, it could be least likely that a
>>>   .word instruction exists before start of assembly function and that too
>>>   contains a word value which could be misleading.
>>>
>>> * But major issue is, what if someone instruments a kprobe at an address which
>>>   contains  .word values. Instruction will never hit, so probe function will not
>>>   be called, but when real code reads that .word value, it reads a wrong value.
>>>
>>> Can GCC provide some compiler option where .word values are located into a
>>> specific area?
>>>
>>> ~Pratyush
>>
>> Hi Dave and Pratyush,
>>
>> Expecting the instruction to the stp x29, x30, [sp,...] would be pretty fragile.  The compiler might not generate that for some very simple function or with certain types of optimization. If the compiler could generate a sentinel word before the start of each function that might be a more robust solution.  Maybe something like a breakpoint instruction or something that clearly would not be in an atomic region.
>>
> 
> I think this is still a reasonable improvement.  The case of both heuristics failing together has to be pretty rare and the result is to make the safer choice.
> 
> I'm looking at what gcc might provide to help.  I made need to talk to a compiler expert though, I've always found the gcc option list a bit overwhelming.
> 
>> -Will

Hi Dave,

Yes, I know what you mean about gcc options.  I am asking some the RH gcc people to see if they know of anything like that.

This additional heuristic should have a comment in there about what exactly that magic set of bits should be matching.  I assume that it is:
"stp x29, x30, [sp, #xxx]!".  Is that correct?

Once the kernel patched with it is built. I will give it a try. Thanks,

-Will

>>>
>>>>
>>>> -dl
>>>>
>>>
>>>> diff --git a/arch/arm64/kernel/kprobes-arm64.c b/arch/arm64/kernel/kprobes-arm64.c
>>>> index 28b9c5b..36b4ea5 100644
>>>> --- a/arch/arm64/kernel/kprobes-arm64.c
>>>> +++ b/arch/arm64/kernel/kprobes-arm64.c
>>>> @@ -127,7 +127,9 @@ is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t *scan_end)
>>>>            * atomic region starts from exclusive load and ends with
>>>>            * exclusive store.
>>>>            */
>>>> -        if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
>>>> +        if ((le32_to_cpu(*scan_start) & 0xffc07fff) == 0xa9807bfd)
>>>> +            return false;
>>>> +        else if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
>>>>               return false;
>>>>           else if (aarch64_insn_is_load_ex(le32_to_cpu(*scan_start)))
>>>>               return true;
>>>
>>
>