eugenis added a comment. In D86000#2219322 <https://reviews.llvm.org/D86000#2219322>, @jfb wrote:
> In D86000#2219288 <https://reviews.llvm.org/D86000#2219288>, @vsk wrote: > >> It'd be nice to fold the new check into an existing sanitizer group to bring >> this to a wider audience. Do you foresee adoption issues for existing >> -fsanitize=integer adopters? Fwiw some recently-added implicit conversion >> checks were folded in without much/any pushback. > > `integer` does "not actually UB checks", right? I can certainly put it in > there if you think I won't get yelled at 😄 I'd support this. "integer" includes unsigned-integer-overflow, it's only recommended to the truly fearless developers :) ================ Comment at: clang/test/Driver/fsanitize.c:911 +// CHECK-unsigned-shift-base-RECOVER-NOT: "-fsanitize-trap=unsigned-shift-base" +// CHECK-unsigned-shift-base-NORECOVER-NOT: "-fno-sanitize-recover=unsigned-shift-base" +// CHECK-unsigned-shift-base-NORECOVER-NOT: "-fsanitize-recover=unsigned-shift-base" ---------------- vsk wrote: > jfb wrote: > > vsk wrote: > > > Not sure I follow this one. Why is 'NORECOVER' not expecting to see > > > "-fno-sanitize-recover=unsigned-shift-base"? > > I have no idea! Other parts of this file do this: > > ``` > > // CHECK-implicit-conversion-NORECOVER-NOT: > > "-fno-sanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation|implicit-integer-sign-change),?){3}"}} > > // ??? > > // CHECK-implicit-integer-arithmetic-value-change-NORECOVER-NOT: > > "-fno-sanitize-recover={{((implicit-signed-integer-truncation|implicit-integer-sign-change),?){2}"}} > > // ??? > > // CHECK-implicit-integer-truncation-NORECOVER-NOT: > > "-fno-sanitize-recover={{((implicit-unsigned-integer-truncation|implicit-signed-integer-truncation),?){2}"}} > > // ??? > > ``` > > I was hoping someone who's touched this before would know. > The CHECK-implicit-conversion-NORECOVER-NOT regex looks fishy. There's more > parens in there than I'd expect: perhaps that explains why the test passes? > > Just above your change, I see something that's closer to what I expect: > > ``` > // RUN: %clang -fsanitize=float-divide-by-zero %s > -fno-sanitize-recover=float-divide-by-zero -### 2>&1 | FileCheck %s > --check-prefixes=CHECK-DIVBYZERO,CHECK-DIVBYZERO-NORECOVER > ... > // CHECK-DIVBYZERO-NORECOVER-NOT: "-fsanitize-recover=float-divide-by-zero" > ``` I think that + the following line mean that the frontend depends on the default behavior (neither recover nor no-recover). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86000/new/ https://reviews.llvm.org/D86000 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
