[PATCH] D37788: [ARM] builtins: Do not abort for NULL pointer in clear_cache.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

>From reading the code and the disassembly this looks like it isn't checking for a NULL start address but is actually checking the value in r0 after the svc call that will clear the cache has completed. Looking in http://elixir.free-electrons.com/linux/latest/source/arch/arm/kernel/traps.c [*] I can see that the call may use non zero return status to signal an error. Am I missing something here?

I don't know enough about the OS to know whether this check makes sense for Android or ChromeOS, if it is usefull on Linux perhaps you might want to conditionally compile it out rather than remove it.

[X] Extract from web-page.

  static inline int
  do_cache_op(unsigned long start, unsigned long end, int flags)
  {
  	if (end < start || flags)
  		return -EINVAL;
  
  	if (!access_ok(VERIFY_READ, start, end - start))
  		return -EFAULT;
  
  	return __do_cache_op(start, end);
  }
  ...
  /*
   * Handle all unrecognised system calls.
   *  0x9f0000 - 0x9fffff are some more esoteric system calls
   */
  #define NR(x) ((__ARM_NR_##x) - __ARM_NR_BASE)
  asmlinkage int arm_syscall(int no, struct pt_regs *regs)
  {
  ...
  /*
  	 * Flush a region from virtual address 'r0' to virtual address 'r1'
  	 * _exclusive_.  There is no alignment requirement on either address;
  	 * user space does not need to know the hardware cache layout.
  	 *
  	 * r2 contains flags.  It should ALWAYS be passed as ZERO until it
  	 * is defined to be something else.  For now we ignore it, but may
  	 * the fires of hell burn in your belly if you break this rule. ;)
  	 *
  	 * (at a later date, we may want to allow this call to not flush
  	 * various aspects of the cache.  Passing '0' will guarantee that
  	 * everything necessary gets flushed to maintain consistency in
  	 * the specified region).
  	 */
  	case NR(cacheflush):
  		return do_cache_op(regs->ARM_r0, regs->ARM_r1, regs->ARM_r2);


https://reviews.llvm.org/D37788