<div style="font-family: arial, helvetica, sans-serif; font-size: 10pt">Hi Joey,<br><br><div class="gmail_quote">On Thu, Nov 22, 2012 at 3:39 PM, Joey Gouly <span dir="ltr"><<a href="mailto:joey.gouly@arm.com" target="_blank">joey.gouly@arm.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Richard & Nuno,<br>
<div class="im"><br>
> > What I'm not sure is if 'bounds' should be included in the ubsan group.<br>
> > While it also checks for undefined behaviour, it won't produce any nice<br>
> > diagnostics like the other ubsan checkers. This bound checker just<br>
crashes<br>
> > the program when it detects a violation.<br>
><br>
> I agree that it would be preferable to produce a call into the ubsan<br>
> runtime to produce a diagnostic for this, but I don't think that needs<br>
> to block this patch. We should decide one way or the other, though,<br>
> and not link in the ubsan runtime for -fsanitize=bounds if we're not<br>
> going to use it.<br>
<br>
</div>I was planning on looking into using ubsan for diagnostics after this<br>
initial patch.<br>
I updated the patch to not link with the ubsan-rt if -fsanitize=bounds is<br>
passed.<br></blockquote><div><br></div><div>Sounds good. Can you please also test this behavior in test/Driver/darwin-sanitizer-ld.c?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

Ok to commit?<br>
<div class="im"><br>
> Presumably there will be a followup patch to LLVM, to remove the<br>
> vestigial 'Penalty' argument; we could at that time add an argument to<br>
> the createBoundsCheckingPass function to specify how to handle a<br>
> failure.<br>
<br>
</div>There is a patch to remove the penalty arg, it has been approved for<br>
committing,<br>
but I'm waiting for the Clang patch to land before I commit it. I have also<br>
started to look into adding a new parameter to select between traps and<br>
runtime calls.<br>
<br>
Thanks,<br>
Joey<br>
<div class="HOEnZb"><div class="h5"><br>
> FWIW, I would prefer if 'bounds' was included in the ubsan group. I'm just<br>
> raising the concern that not everyone may agree.<br>
><br>
> Nuno<br>
><br>
> ----- Original Message ----- From: "Joey Gouly" <<a href="mailto:joey.gouly@arm.com">joey.gouly@arm.com</a>><br>
> To: <<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a>>; <<a href="mailto:richard@metafoo.co.uk">richard@metafoo.co.uk</a>><br>
> Sent: Wednesday, November 21, 2012 11:52 AM<br>
> Subject: [cfe-commits] [PATCH] PR14306: Move -fbounds-checking<br>
> to-fsanitize=bounds<br>
><br>
><br>
><br>
> Hi all,<br>
><br>
> Attached is the patch to change the -fbounds-checking flag to<br>
> -fsanitize=bounds, and also put it under the ubsan flag as well.<br>
><br>
> Note: I removed the bounds checking penalty parameter, but that is in a<br>
> separate patch.<br>
><br>
> Please review!<br>
><br>
> Thanks,<br>
> Joey<br>
<br>
<br>
<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</div></div></blockquote></div><br><br clear="all"><div><br></div>-- <br><div>Alexey Samsonov, MSK</div><br>
</div>