[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. martong marked 2 inline comments as done. Closed by commit rG2df120784af4: [analyzer] Fix assertion in simplifySymbolCast (authored by martong). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 Files: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/produce-symbolcast_x86.cpp Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((reg_$0) / 30)}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((reg_$0) / 30))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((reg_$0) / 30)))}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1103,6 +1103,10 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support simplification from non-integers. + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((reg_$0) / 30)}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((reg_$0) / 30))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((reg_$0) / 30)))}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1103,6 +1103,10 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support simplification from non-integers. + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land. LGTM Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1082 + // FIXME support cast from non-integers. + // E.g (char)(double)(double x) -> (char)(double x) + if (!RT->isIntegralOrEnumerationType()) martong wrote: > steakhal wrote: > > Well, I would expect the same, but crosscheck this with the actual dump. > Please elaborate. nvm Comment at: clang/test/Analysis/produce-symbolcast_x86.cpp:16 + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}} + char C = D; martong wrote: > steakhal wrote: > > Place here a FIXME that we should not have two double casts. > The `(double)(double)` problem is handled in the parent patch. Please check > that out. Yeey, awesome! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
martong marked 2 inline comments as done. martong added inline comments. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1082 + // FIXME support cast from non-integers. + // E.g (char)(double)(double x) -> (char)(double x) + if (!RT->isIntegralOrEnumerationType()) steakhal wrote: > Well, I would expect the same, but crosscheck this with the actual dump. Please elaborate. Comment at: clang/test/Analysis/produce-symbolcast_x86.cpp:16 + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}} + char C = D; steakhal wrote: > Place here a FIXME that we should not have two double casts. The `(double)(double)` problem is handled in the parent patch. Please check that out. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
martong updated this revision to Diff 437938. martong added a comment. - Rebase on dependent patch Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 Files: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/produce-symbolcast_x86.cpp Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((reg_$0) / 30)}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((reg_$0) / 30))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((reg_$0) / 30)))}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1104,6 +1104,10 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support simplification from non-integers. + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((reg_$0) / 30)}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((reg_$0) / 30))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((reg_$0) / 30)))}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1104,6 +1104,10 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support simplification from non-integers. + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
steakhal added a comment. Oh I forgot to submit my review. My bad. Please investigate why we have two double casts there. Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1082 + // FIXME support cast from non-integers. + // E.g (char)(double)(double x) -> (char)(double x) + if (!RT->isIntegralOrEnumerationType()) Well, I would expect the same, but crosscheck this with the actual dump. Comment at: clang/test/Analysis/produce-symbolcast_x86.cpp:16 + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}} + char C = D; Place here a FIXME that we should not have two double casts. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
martong added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D126779/new/ https://reviews.llvm.org/D126779 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D126779: [analyzer] Fix assertion in simplifySymbolCast
martong created this revision. martong added reviewers: ASDenysPetrov, steakhal, NoQ, vabridgers. Herald added subscribers: manas, gamesh411, dkrupp, donat.nagy, Szelethus, mikhail.ramalho, a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun. Herald added a reviewer: Szelethus. Herald added a project: All. martong requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Added a new test code that fails an assertion in the baseline. That is because `getAPSIntType` works only with integral types. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D126779 Files: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp clang/test/Analysis/produce-symbolcast_x86.cpp Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((double) ((reg_$0) / 30)))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((double) ((reg_$0) / 30}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1078,6 +1078,11 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support cast from non-integers. + // E.g (char)(double)(double x) -> (char)(double x) + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); Index: clang/test/Analysis/produce-symbolcast_x86.cpp === --- clang/test/Analysis/produce-symbolcast_x86.cpp +++ clang/test/Analysis/produce-symbolcast_x86.cpp @@ -11,6 +11,15 @@ template void clang_analyzer_dump(T); +void test_double(int n) { + double D = n / 30; + clang_analyzer_dump(D); // expected-warning{{(double) ((double) ((reg_$0) / 30))}} + char C = D; + clang_analyzer_dump(C); // expected-warning{{(char) ((double) ((double) ((reg_$0) / 30)))}} + int I = C; // assertion should not fail here! + clang_analyzer_dump(I); // expected-warning{{(int) ((char) ((double) ((double) ((reg_$0) / 30}} +} + void test_schar(schar x) { clang_analyzer_dump(x); // expected-warning{{reg_$0}} Index: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp === --- clang/lib/StaticAnalyzer/Core/SValBuilder.cpp +++ clang/lib/StaticAnalyzer/Core/SValBuilder.cpp @@ -1078,6 +1078,11 @@ SymbolRef RootSym = cast(SE)->getOperand(); QualType RT = RootSym->getType().getCanonicalType(); + // FIXME support cast from non-integers. + // E.g (char)(double)(double x) -> (char)(double x) + if (!RT->isIntegralOrEnumerationType()) +return makeNonLoc(SE, T, CastTy); + BasicValueFactory = getBasicValueFactory(); APSIntType CTy = BVF.getAPSIntType(CastTy); APSIntType TTy = BVF.getAPSIntType(T); ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits