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

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

Some more thoughts.

- For a [VA start, VA end) interval that is entirely unmapped it is safe for the cache flush to continue without error as by definition the addresses won't be in the cache.
- If we can split [VA start, VA end) into two sub-ranges [Mapped VA start, Mapped VA end), [Mapped VA end, Unmapped VA end) then when the function returns all the Mapped addresses will have been cleared, we don't care about the unmapped addresses.
- If the [VA start, VA end) is sparsely mapped into non contiguous chunks of mapped and non-mapped then there is a potential problem if the program is written to assume that all addresses in the interval have been flushed.

Speaking to my colleague that works on QEMU, a common use case for this function is for people writing JITs, in essence write instructions to a buffer, call __builtin_cache flush with the address range of the buffer. In this case the buffer will be mapped and there should be no data aborts. A potential situation where an abort might occur is if the Jit is a bit sloppy about giving precise address ranges for what it has modified, especially if the address space is sparsely mapped.

It would be useful to find out what application crashed, and whether the compiler-rt abort was a genuine bug in the application, or a legitimate use where there was no expectation that all addresses were mapped.

Pragmatically I think the compiler-rt implementation is effectively adding an extra pre-condition to the builtin that all virtual addresses in the range are mapped. In return it offers the post-condition that all addresses in the range have been definitely cleared. My opinion is that we shouldn't change the contract of the builtin (defined by its implementation) without some kind of extra documentation.


https://reviews.llvm.org/D37788