[PATCH] D40423: [ARM][AArch64] Workaround ARM/AArch64 percularity in clearing icache.

[Adhemerval Zanella via Phabricator via llvm-commits]

zatrazz added inline comments.


================
Comment at: lib/Support/Unix/Memory.inc:134
+      return MemoryBlock();
+  }
 
----------------
I think instead of issue a mmap plus two mprotects for MF_EXEC, it would be better if we could just mmap with read permission, call InvalidateInstructionCache and then mmap to the expected protection. Something as:

```
int adjustAllocateMappedProtectionFlags (unsigned Flags)
{
#if defined(__NetBSD__) && defined(PROT_MPROTECT)
  Protect |= PROT_MPROTECT(PROT_READ | PROT_WRITE | PROT_EXEC);
#endif
#if defined(__arm__) || defined(__aarch64__)
  Protect |= PROT_READ;
#endif
  return Protect;
}

[...]

MemoryBlock
Memory::allocateMappedMemory(size_t NumBytes,
                             const MemoryBlock *const NearBlock,
                             unsigned PFlags,
                             std::error_code &EC) {
  [...]
  int Protect = getPosixProtectionFlags(PFlags);

  Protect = adjustAllocateMappedProtectionFlags (Protect);
  [...]
```

And then issue the InvalidateInstructionCache without requiring to mprotect to PROT_READ first:

```
std::error_code
Memory::protectMappedMemory(const MemoryBlock &M, unsigned Flags) {
  [...]
  bool InvalidateCache = (Flags & MF_EXEC);

#if defined(__arm__) || defined(__aarch64__)
  // Certain ARM implementations treat icache clear instruction as a memory read,
  // and CPU segfaults on trying to clear cache on !PROT_READ page.  Therefore we need
  // to temporarily add PROT_READ for the sake of flushing the instruction caches.
  if (InvalidateCache && !(Protect & PROT_READ)) {
    Memory::InvalidateInstructionCache(M.Address, M.Size);
    InvalidateCache = false;
  }
#endif
```


https://reviews.llvm.org/D40423