<div dir="ltr">+ more asan folks, please CC them to the code reviews. <div>Also please make sure llvm-commits is CC-ed (cfe-commits for clang changes)<br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Oct 31, 2017 at 2:29 PM, Walter Lee <span dir="ltr"><<a href="mailto:waltl@google.com" target="_blank">waltl@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">I've prepared a preliminary set of patches that makes ASan work with<br>
32-byte shadow granularity, and I would like to get some feedback on<br>
those patches as well as my general plan.<br>
<br>
Some background: I am porting ASan to the Myriad platform. I'm<br>
looking to break up that port into components that may be<br>
useful/relevant to other platforms -- the first of those pieces is the<br>
ability to use a coarser shadow granularity. This is important for us<br>
because Myriad has a limited amount of physical memory and no virtual<br>
memory, so it is important to limit the amount of shadow memory<br>
required.<br>
<br>
My end-goal for this part is to be able to configure a build that<br>
overrides the default shadow granularity, that can cleanly run the<br>
clang/llvm test suite for 32-byte shadow granularity on i386/x86_64,<br>
so we can set up buildbot for that configuration.<br>
<br>
My basic plan:<br>
1. Add build support to override of default shadow scale.<br>
2. Fix various issues with 32-byte shadow granularity.<br>
3. Propose improvements to 32-byte shadow granularity support.<br>
4. Make test suite run cleanly for 32-byte shadow granularity.<br>
5. Set up build bot for 32-byte shadow granularity on i386/x86_64.<br>
<br>
My initial set of patches adds the build support and makes some<br>
essential fixes to the compiler and run-time. They are:<br>
<br>
<a href="https://reviews.llvm.org/D39469" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39469</a> [asan] Add cmake hook to override<br>
default shadow scale<br>
<a href="https://reviews.llvm.org/D39470" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39470</a> [asan] Fix size/alignment issues with<br>
non-default shadow scale<br>
<a href="https://reviews.llvm.org/D39471" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39471</a> [asan] Fix small X86_64 ShadowOffset<br>
for non-default shadow scale<br>
<a href="https://reviews.llvm.org/D39472" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39472</a> [asan] Ensure that the minimum redzone<br>
is at least SHADOW_GRANULARITY<br>
<a href="https://reviews.llvm.org/D39473" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39473</a> [sanitizers] Increase alignment of low<br>
level allocator<br>
<a href="https://reviews.llvm.org/D39474" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39474</a> [asan] Avoid assert failure for<br>
non-default shadow scale<br>
<a href="https://reviews.llvm.org/D39475" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D39475</a> [asan] Improve stack error reports for<br>
large shadow granularity<br>
<br>
The following features don't work yet, but can be fixed:<br>
<br>
- i386/x86_64 assembly instrumentation.<br></blockquote><div>That's fine. It doesn't really work in regular mode either. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Prelink support. The memory map with MidMem for this configuration<br>
is more complicated that that expected by compiler_rt. The current<br>
HighShadow would overlap with MidMem so would need to be adjusted.<br>
It's not clear to me that whether is an important feature?<br></blockquote><div><br></div><div>you may safely ignore this for 32-bit granularity, but please keep it working in regular mode. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Intra object overflow. This appears to be an experimental feature,<br>
and I have not ported the padding insertion for different<br>
granularity.<br></blockquote><div><br></div><div>Yep, ignore it. </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Some features will not work as well with the larger shadow<br>
granularity:<br>
<br>
- Stack errors: it seems sensible not to insert 32-byte sentinels<br>
between every object, but the result is that some stack overflow<br>
gets reported as unknown or use-after-scope. I have a patch that<br>
improves on the default behavior, but there remains cases where the<br>
error reports will not be as good.<br></blockquote><div><br></div><div>Hmm. Not sure what's the problem here. It's totally fine to insert 32-byte redzone around stack objects. </div><div>(in 32-byte granularity mode)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- __asan_poison_memory_region is now more limited. A typical case<br>
that doesn't work is the poisoning of 8-byte or 16-byte that maps to<br>
the middle of a shadow byte.<br></blockquote><div><br></div><div>yep. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
For testing, I have a few questions:<br>
<br>
- Would it make sense to provide an internal compiler flag to set the<br>
shadow granularity, so that there we can at least run the<br>
instrumentation tests for 32-byte granularity in normal builds?<br></blockquote><div><br></div><div>I'd prefer a proper flag, like -fsanitize-address-granularity=N (8,16,32)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
- Is there a reasonable subset of tests I can port to 32-byte<br>
granularity to provide reasonable coverage, or should I aim to port<br>
all tests?<br></blockquote><div><br></div><div>Let's see what tests won't work out of the box and decide. </div><div>We can mark all failing tests as</div><div> UNSUPPORTED: 32-bit-granularity </div><div>but ideally we shouldn't have to mark too many of those. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
For now, I've tested my changes against check-all (for default shadow<br>
granularity), and also set up a 32-byte shadow granularity build and<br>
manually inspected the failures to ensure that they are not<br>
unexpected.<br>
<br>
Thanks,<br>
<br>
Walter<br>
</blockquote></div><br></div></div></div>