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

Renato Golin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Sep 18 07:39:25 PDT 2017


rengolin added a comment.

Some more info on this. At the time Android did its implementation, the Linux kernel used[1] to call `do_cache_op`, ignoring the return result and then returning zero. Now [2], it returns whatever `do_cache_op` returns, which ends up being a very long sequence of uses and definitions, boiling down to SVC.

The GCC manual [3] doesn't say anything about failures, but it does say either one of two things will happen:

- instructions are emitted in-line to clear the instruction cache
- call to the __clear_cache function in libgcc is made

it's not unreasonable to assume "emit instruction" (singular) to be just SVC, no checks. Their own implementation in libgcc seems to confirm that.

However, the actual behaviour is probably historical (as shown from the kernel evolution), because x86 doesn't need to flush the code cache, while ARM does.

On the debug side of things, compiler-rt is implementing a function that will be on the stack trace after RT aborts, so easy to spot that it was a failed cache clearing attempt, with all registers as witness. If that function gets inlined, then walking back the assembly code will get you the SVE call, and then the register state.

I understand that NaCl and QEMU are a pain to debug, but diagnosing the problems of clearing the cache can help find much harder errors elsewhere.

So, I'm in two minds about this. While I understand the frustration of virtualisation users, I find it hard to concede without harder evidence (which so far has been anecdotal).

Being pragmatic, I would be ok with a change that had a non-default pre-processor guard (something like UNSAFE_CLEAR_CACHE), which platforms are free to set in their build systems, avoiding the abort.

In that case, I'd like people that are building compiler-rt on multiple platforms (like Peter, Saleem, Joerg) to make sure the solution is good for them.

[1] http://elixir.free-electrons.com/linux/v2.6.24/source/arch/arm/kernel/traps.c#L468
[2] http://elixir.free-electrons.com/linux/v4.14-rc1/source/arch/arm/kernel/traps.c#L631
[3] https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html


https://reviews.llvm.org/D37788





More information about the llvm-commits mailing list