Re: [linux-next:master 6470/6646] include/linux/kprobes.h:477:9: error: implicit declaration of function 'kprobe_fault_handler'; did you mean 'kprobe_page_fault'?

[Anshuman Khandual <anshuman.khandual@arm.com>]

Hello Andrew,

On 06/18/2019 07:37 AM, Andrew Morton wrote:
> On Sat, 15 Jun 2019 10:55:07 +0800 kbuild test robot <lkp@intel.com> wrote:
> 
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> head:   f4788d37bc84e27ac9370be252afb451bf6ef718
>> commit: 4dd635bce90e8b6ed31c08cd654deca29f4d9d66 [6470/6646] mm, kprobes: generalize and rename notify_page_fault() as kprobe_page_fault()
>> config: mips-allmodconfig (attached as .config)
>> compiler: mips-linux-gcc (GCC) 7.4.0
>> reproduce:
>>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>>         chmod +x ~/bin/make.cross
>>         git checkout 4dd635bce90e8b6ed31c08cd654deca29f4d9d66
>>         # save the attached .config to linux build tree
>>         GCC_VERSION=7.4.0 make.cross ARCH=mips 
>>
>> If you fix the issue, kindly add following tag
>> Reported-by: kbuild test robot <lkp@intel.com>
>>
>> All errors (new ones prefixed by >>):
>>
>>    In file included from net//sctp/offload.c:11:0:
>>    include/linux/kprobes.h: In function 'kprobe_page_fault':
>>>> include/linux/kprobes.h:477:9: error: implicit declaration of function 'kprobe_fault_handler'; did you mean 'kprobe_page_fault'? [-Werror=implicit-function-declaration]
>>      return kprobe_fault_handler(regs, trap);
> 
> Urgh, OK, thanks.
> 
> kprobe_fault_handler() is only ever defined and referenced in arch code
> and generic code has no right to be assuming that the architecture
> actually provides it.  And so it is with mips (at least).

Hmm, so the problem really is that on mips arch even though CONFIG_KPROBES
is enabled, it does not export (though it defines) a kprobe_fault_handler()
implementation unlike all other architectures.

Now that generic code calls kprobe_fault_handler(), should not all arch be
providing one when they subscribe to CONFIG_KPROBES ? In which case mips
should just export it's existing definition to fix this build problem.

> 
> The !CONFIG_KPROBES stub version of kprobe_fault_handler() should not
> have been placed in include/linux/kprobes.h!  Each arch should have
> defined its own, if that proved necessary.

I guess its there in include/linux/kprobes.h! because !CONFIG_KPROBES stub
version for all archs will exactly look the same.

> 
> Oh well, ho hum.  Hopefully Anshuman will be able to come up with a fix
> for mips and any similarly-affected architectures.

Will export it's existing definition via arch/mips/include/asm/kprobes.h
unless that is problematic for other reasons. Another solution would be
to define an weak symbol in include/linux/kprobes.h for CONFIG_KPROBES.
But that will not be correct because kprobe_fault_handler() by all means
is always platform specific.

> 
> Also, please very carefully check that this patchset is correct for all
> architectures!  kprobe_fault_handler() could conceivably do different
> things on different architectures.

Agreed.