[cfe-commits] [PATCH] PR14306: Move -fbounds-checking to-fsanitize=bounds

[Richard Smith]

Hi!

On Wed, Nov 21, 2012 at 11:25 AM, Nuno Lopes <nunoplopes at sapo.pt> wrote:
> Thanks, this was on my TODO list for a while..
> The patch looks good to me, but I'll let a clang expert pronounce the final
> veredict.

LGTM

> What I'm not sure is if 'bounds' should be included in the ubsan group.
> While it also checks for undefined behaviour, it won't produce any nice
> diagnostics like the other ubsan checkers. This bound checker just crashes
> the program when it detects a violation.

I agree that it would be preferable to produce a call into the ubsan
runtime to produce a diagnostic for this, but I don't think that needs
to block this patch. We should decide one way or the other, though,
and not link in the ubsan runtime for -fsanitize=bounds if we're not
going to use it.

Presumably there will be a followup patch to LLVM, to remove the
vestigial 'Penalty' argument; we could at that time add an argument to
the createBoundsCheckingPass function to specify how to handle a
failure.

> FWIW, I would prefer if 'bounds' was included in the ubsan group. I'm just
> raising the concern that not everyone may agree.
>
> Nuno
>
> ----- Original Message ----- From: "Joey Gouly" <joey.gouly at arm.com>
> To: <cfe-commits at cs.uiuc.edu>; <richard at metafoo.co.uk>
> Sent: Wednesday, November 21, 2012 11:52 AM
> Subject: [cfe-commits] [PATCH] PR14306: Move -fbounds-checking
> to-fsanitize=bounds
>
>
>
> Hi all,
>
> Attached is the patch to change the -fbounds-checking flag to
> -fsanitize=bounds, and also put it under the ubsan flag as well.
>
> Note: I removed the bounds checking penalty parameter, but that is in a
> separate patch.
>
> Please review!
>
> Thanks,
> Joey