[PATCH] D85378: Fix off-by-one error in size of the required shadow memory passed to `MapDynamicShadow`.

Dan Liew via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 6 11:57:11 PDT 2020


delcypher added a comment.

In D85378#2200000 <https://reviews.llvm.org/D85378#2200000>, @yln wrote:

>> This latent bug was likely hidden because the shadow memory size is
>> always page aligned due to being allocated by mmap.
>
> Is this bug still latent, or does it manifest now?  Can you describe what happens when it is triggered?

@yln

It manifests on  Darwin (iOS) if you manually examine the size passed to `MapDynamicShadow()`. If you stick a `Report("shadow_size_bytes: %p\n");` in the implementation of `MapDynamicShadow()` and then run a test binary then...

Before this patch

  ==282==shadow_size_bytes: 0x0001f7ffffff

notice how this size is **not aligned to the iOS page size (16KiB)**

With this patch:

  ==289==shadow_size_bytes: 0x0001f8000000

Notice how the size is now aligned to a 16 KiB page boundary.

On iOS I don't think this ever caused problems because the `MapDynamicShadow()` implementation finds a region that is guaranteed to be page aligned size.

However I don't think it makes sense to be passing a non-page-aligned size to `MapDynamicShadow()`. Maybe I should add a `CHECK()` for it too so that this doesn't get broken in the future?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D85378/new/

https://reviews.llvm.org/D85378



More information about the llvm-commits mailing list