r286699 - [c++1z] Support constant folding for __builtin_strchr and __builtin_memchr.
Vedant Kumar via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 15 17:35:58 PST 2016
Thank you!
The bot recovered after r287066.
vedant
> On Nov 15, 2016, at 5:09 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Tue, Nov 15, 2016 at 3:39 PM, Vedant Kumar <vsk at apple.com> wrote:
> Hi Richard,
>
> Our internal ASan bot started failing after this and r286678:
>
> ASAN:DEADLYSIGNAL
> =================================================================
> ==95464==ERROR: AddressSanitizer: stack-overflow on address 0x7fff56c68b40 (pc 0x000110c582a2 bp 0x7fff56c69970 sp 0x7fff56c68b40 T0)
> #0 0x110c582a1 in clang::StmtVisitorBase<clang::make_const_ptr, (anonymous namespace)::LValueExprEvaluator, bool>::Visit(clang::Stmt const*) StmtVisitor.h:40
>
> SUMMARY: AddressSanitizer: stack-overflow StmtVisitor.h:40 in clang::StmtVisitorBase<clang::make_const_ptr, (anonymous namespace)::LValueExprEvaluator, bool>::Visit(clang::Stmt co
> nst*)
> ==95464==ABORTING
> 0 clang 0x000000010c000da2 llvm::sys::RunSignalHandlers() + 194
> 1 clang 0x000000010c004963 SignalHandler(int) + 595
> 2 libsystem_platform.dylib 0x00007fff841bd52a _sigtramp + 26
> Stack dump:
> 0. Program arguments: /Users/buildslave/Desktop/smooshbase-trunk-svn/clang-build/./bin/clang -cc1 -internal-isystem /Users/buildslave/Desktop/smooshbase-trunk-svn/clang-build/bin/../lib/clang/4.0.0/include -nostdsysteminc -triple i686-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions -verify -std=c++11 -pedantic /Users/buildslave/Desktop/smooshbase-trunk-svn/clang/src/tools/clang/test/SemaCXX/constant-expression-cxx11.cpp -Wno-comment -Wno-tautological-pointer-compare -Wno-boo$-conversion
> 1. /Users/buildslave/Desktop/smooshbase-trunk-svn/clang/src/tools/clang/test/SemaCXX/constant-expression-cxx11.cpp:1457:1: current parser token '}'
> 2. /Users/buildslave/Desktop/smooshbase-trunk-svn/clang/src/tools/clang/test/SemaCXX/constant-expression-cxx11.cpp:1435:1: parsing namespace 'RecursiveOpaqueExpr'
> /Users/buildslave/Desktop/smooshbase-trunk-svn/clang-build/tools/clang/test/SemaCXX/Output/constant-expression-cxx11.cpp.script: line 1: 95464 Abort trap: 6 /Users/buildslave/Desktop/smooshbase-trunk-svn/clang-build/./bin/clang -cc1 -internal-isystem /Users/buildslave/Desktop/smooshbase-trunk-svn/clang-build/bin/../lib/clang/4.0.0/include -nostdsysteminc -triple i686-linux -Wno-string-plus-int -Wno-pointer-arith -Wno-zero-length-array -fsyntax-only -fcxx-exceptions -verify -std=c++11 -pedantic /Users/buildslave/Desktop/smooshbase-trunk-svn/clang/src/tools/clang/test/SemaCXX/constant-expression-cxx11.cpp -Wno-comment -Wno-tautological-pointer-compare -Wno-bool-conversion
>
> It seems to hit an infinite loop:
>
> frame #17: 0x00000001083d48ab clang`::Visit() [inlined] HandleConditionalOperator<clang::ConditionalOperator> + 165 at ExprConstant.cpp:4212
> frame #18: 0x00000001083d4806 clang`::Visit() [inlined] VisitConditionalOperator at ExprConstant.cpp:4331
> frame #19: 0x00000001083d4806 clang`::Visit() + 33158 at StmtNodes.inc:129
> frame #20: 0x000000010835c43c clang`::Evaluate() + 1468 at ExprConstant.cpp:9155
> frame #21: 0x00000001083bad54 clang`::EvaluateStmt() + 15572 at ExprConstant.cpp:3663
> frame #22: 0x00000001083b9147 clang`::EvaluateStmt() + 8391 at ExprConstant.cpp:3673
> frame #23: 0x0000000108360829 clang`::HandleFunctionCall() + 1657 at ExprConstant.cpp:3994
> frame #24: 0x000000010842f681 clang`::handleCallExpr() + 3697 at ExprConstant.cpp:4438
> frame #25: 0x00000001083ea4c5 clang`::VisitCallExpr() [inlined] VisitCallExpr + 54 at ExprConstant.cpp:4355
> frame #26: 0x00000001083ea48f clang`::VisitCallExpr() + 11407 at ExprConstant.cpp:6937
> frame #27: 0x00000001083ccdcd clang`::Visit() + 1869 at Expr.h:1705
> frame #28: 0x000000010835c43c clang`::Evaluate() + 1468 at ExprConstant.cpp:9155
> frame #29: 0x00000001083cf094 clang`::Visit() [inlined] VisitBinaryConditionalOperator + 145 at ExprConstant.cpp:4307
> frame #30: 0x00000001083cf003 clang`::Visit() + 10627 at StmtNodes.inc:123
> frame #31: 0x00000001083d1b7f clang`::Visit() + 21759 at ExprCXX.h:2998
> frame #32: 0x00000001083d48ab clang`::Visit() [inlined] HandleConditionalOperator<clang::ConditionalOperator> + 165 at ExprConstant.cpp:4212
>
> The failure disappears when I revert both commits locally.
>
> Unfortunately there doesn't seem to be a green dragon bot that would catch this.
>
> Could you take a look?
>
> Looks like both changes increased the stack usage in constant expression evaluation, and the combination of the two pushed this test over the stack limit for (some) ASan builds.
>
> I've outlined the handling of calls to builtin functions in r287066; hopefully that will sufficiently reduce the stack usage for unoptimized builds, but there's a chance the ASan build will just inline it again and we'll need to try harder...
>
> thanks,
> vedant
>
> > On Nov 11, 2016, at 5:39 PM, Richard Smith via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> >
> > Author: rsmith
> > Date: Fri Nov 11 19:39:56 2016
> > New Revision: 286699
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=286699&view=rev
> > Log:
> > [c++1z] Support constant folding for __builtin_strchr and __builtin_memchr.
> >
> > Modified:
> > cfe/trunk/lib/AST/ExprConstant.cpp
> > cfe/trunk/test/SemaCXX/constexpr-string.cpp
> >
> > Modified: cfe/trunk/lib/AST/ExprConstant.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=286699&r1=286698&r2=286699&view=diff
> > ==============================================================================
> > --- cfe/trunk/lib/AST/ExprConstant.cpp (original)
> > +++ cfe/trunk/lib/AST/ExprConstant.cpp Fri Nov 11 19:39:56 2016
> > @@ -5256,7 +5256,7 @@ bool PointerExprEvaluator::VisitCallExpr
> > if (IsStringLiteralCall(E))
> > return Success(E);
> >
> > - switch (E->getBuiltinCallee()) {
> > + switch (unsigned BuiltinOp = E->getBuiltinCallee()) {
> > case Builtin::BI__builtin_addressof:
> > return EvaluateLValue(E->getArg(0), Result, Info);
> > case Builtin::BI__builtin_assume_aligned: {
> > @@ -5324,6 +5324,65 @@ bool PointerExprEvaluator::VisitCallExpr
> >
> > return true;
> > }
> > +
> > + case Builtin::BIstrchr:
> > + case Builtin::BImemchr:
> > + if (Info.getLangOpts().CPlusPlus11)
> > + Info.CCEDiag(E, diag::note_constexpr_invalid_function)
> > + << /*isConstexpr*/0 << /*isConstructor*/0
> > + << (BuiltinOp == Builtin::BIstrchr ? "'strchr'" : "'memchr'");
> > + else
> > + Info.CCEDiag(E, diag::note_invalid_subexpr_in_const_expr);
> > + // Fall through.
> > + case Builtin::BI__builtin_strchr:
> > + case Builtin::BI__builtin_memchr: {
> > + if (!Visit(E->getArg(0)))
> > + return false;
> > + APSInt Desired;
> > + if (!EvaluateInteger(E->getArg(1), Desired, Info))
> > + return false;
> > + uint64_t MaxLength = uint64_t(-1);
> > + if (BuiltinOp != Builtin::BIstrchr &&
> > + BuiltinOp != Builtin::BI__builtin_strchr) {
> > + APSInt N;
> > + if (!EvaluateInteger(E->getArg(2), N, Info))
> > + return false;
> > + MaxLength = N.getExtValue();
> > + }
> > +
> > + QualType CharTy = Info.Ctx.CharTy;
> > + bool IsStrchr = (BuiltinOp != Builtin::BImemchr &&
> > + BuiltinOp != Builtin::BI__builtin_memchr);
> > +
> > + // strchr compares directly to the passed integer, and therefore
> > + // always fails if given an int that is not a char.
> > + if (IsStrchr &&
> > + !APSInt::isSameValue(HandleIntToIntCast(Info, E, CharTy,
> > + E->getArg(1)->getType(),
> > + Desired),
> > + Desired))
> > + return ZeroInitialization(E);
> > +
> > + // memchr compares by converting both sides to unsigned char. That's also
> > + // correct for strchr if we get this far.
> > + uint64_t DesiredVal = Desired.trunc(Info.Ctx.getCharWidth()).getZExtValue();
> > +
> > + for (; MaxLength; --MaxLength) {
> > + APValue Char;
> > + if (!handleLValueToRValueConversion(Info, E, CharTy, Result, Char) ||
> > + !Char.isInt())
> > + return false;
> > + if (Char.getInt().getZExtValue() == DesiredVal)
> > + return true;
> > + if (IsStrchr && !Char.getInt())
> > + break;
> > + if (!HandleLValueArrayAdjustment(Info, E, Result, CharTy, 1))
> > + return false;
> > + }
> > + // Not found: return nullptr.
> > + return ZeroInitialization(E);
> > + }
> > +
> > default:
> > return ExprEvaluatorBaseTy::VisitCallExpr(E);
> > }
> > @@ -7065,7 +7124,7 @@ bool IntExprEvaluator::VisitCallExpr(con
> > }
> >
> > // Slow path: scan the bytes of the string looking for the terminating 0.
> > - QualType CharTy = E->getArg(0)->getType()->getPointeeType();
> > + QualType CharTy = Info.Ctx.CharTy;
> > for (uint64_t Strlen = 0; /**/; ++Strlen) {
> > APValue Char;
> > if (!handleLValueToRValueConversion(Info, E, CharTy, String, Char) ||
> > @@ -7108,7 +7167,7 @@ bool IntExprEvaluator::VisitCallExpr(con
> > }
> > bool StopAtNull = (BuiltinOp != Builtin::BImemcmp &&
> > BuiltinOp != Builtin::BI__builtin_memcmp);
> > - QualType CharTy = E->getArg(0)->getType()->getPointeeType();
> > + QualType CharTy = Info.Ctx.CharTy;
> > for (; MaxLength; --MaxLength) {
> > APValue Char1, Char2;
> > if (!handleLValueToRValueConversion(Info, E, CharTy, String1, Char1) ||
> >
> > Modified: cfe/trunk/test/SemaCXX/constexpr-string.cpp
> > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/constexpr-string.cpp?rev=286699&r1=286698&r2=286699&view=diff
> > ==============================================================================
> > --- cfe/trunk/test/SemaCXX/constexpr-string.cpp (original)
> > +++ cfe/trunk/test/SemaCXX/constexpr-string.cpp Fri Nov 11 19:39:56 2016
> > @@ -1,4 +1,5 @@
> > // RUN: %clang_cc1 %s -std=c++1z -fsyntax-only -verify -pedantic
> > +// RUN: %clang_cc1 %s -std=c++1z -fsyntax-only -verify -pedantic -fno-signed-char
> >
> > # 4 "/usr/include/string.h" 1 3 4
> > extern "C" {
> > @@ -9,9 +10,12 @@ extern "C" {
> > extern int strcmp(const char *s1, const char *s2);
> > extern int strncmp(const char *s1, const char *s2, size_t n);
> > extern int memcmp(const char *s1, const char *s2, size_t n); // expected-note {{here}}
> > +
> > + extern char *strchr(const char *s, int c);
> > + extern void *memchr(const void *s, int c, size_t n);
> > }
> >
> > -# 15 "SemaCXX/constexpr-string.cpp" 2
> > +# 19 "SemaCXX/constexpr-string.cpp" 2
> > namespace Strlen {
> > constexpr int n = __builtin_strlen("hello"); // ok
> > constexpr int m = strlen("hello"); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strlen' cannot be used in a constant expression}}
> > @@ -66,3 +70,37 @@ namespace StrcmpEtc {
> > constexpr int b = strncmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strncmp' cannot be used in a constant expression}}
> > constexpr int c = memcmp("hello", "world", 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memcmp' cannot be used in a constant expression}}
> > }
> > +
> > +namespace StrchrEtc {
> > + constexpr const char *kStr = "abca\xff\0d";
> > + constexpr char kFoo[] = {'f', 'o', 'o'};
> > + static_assert(__builtin_strchr(kStr, 'a') == kStr);
> > + static_assert(__builtin_strchr(kStr, 'b') == kStr + 1);
> > + static_assert(__builtin_strchr(kStr, 'c') == kStr + 2);
> > + static_assert(__builtin_strchr(kStr, 'd') == nullptr);
> > + static_assert(__builtin_strchr(kStr, 'e') == nullptr);
> > + static_assert(__builtin_strchr(kStr, '\0') == kStr + 5);
> > + static_assert(__builtin_strchr(kStr, 'a' + 256) == nullptr);
> > + static_assert(__builtin_strchr(kStr, 'a' - 256) == nullptr);
> > + static_assert(__builtin_strchr(kStr, '\xff') == kStr + 4);
> > + static_assert(__builtin_strchr(kStr, '\xff' + 256) == nullptr);
> > + static_assert(__builtin_strchr(kStr, '\xff' - 256) == nullptr);
> > + static_assert(__builtin_strchr(kFoo, 'o') == kFoo + 1);
> > + static_assert(__builtin_strchr(kFoo, 'x') == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
> > + static_assert(__builtin_strchr(nullptr, 'x') == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}}
> > +
> > + static_assert(__builtin_memchr(kStr, 'a', 0) == nullptr);
> > + static_assert(__builtin_memchr(kStr, 'a', 1) == kStr);
> > + static_assert(__builtin_memchr(kStr, '\0', 5) == nullptr);
> > + static_assert(__builtin_memchr(kStr, '\0', 6) == kStr + 5);
> > + static_assert(__builtin_memchr(kStr, '\xff', 8) == kStr + 4);
> > + static_assert(__builtin_memchr(kStr, '\xff' + 256, 8) == kStr + 4);
> > + static_assert(__builtin_memchr(kStr, '\xff' - 256, 8) == kStr + 4);
> > + static_assert(__builtin_memchr(kFoo, 'x', 3) == nullptr);
> > + static_assert(__builtin_memchr(kFoo, 'x', 4) == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
> > + static_assert(__builtin_memchr(nullptr, 'x', 3) == nullptr); // expected-error {{not an integral constant}} expected-note {{dereferenced null}}
> > + static_assert(__builtin_memchr(nullptr, 'x', 0) == nullptr); // FIXME: Should we reject this?
> > +
> > + constexpr bool a = !strchr("hello", 'h'); // expected-error {{constant expression}} expected-note {{non-constexpr function 'strchr' cannot be used in a constant expression}}
> > + constexpr bool b = !memchr("hello", 'h', 3); // expected-error {{constant expression}} expected-note {{non-constexpr function 'memchr' cannot be used in a constant expression}}
> > +}
> >
> >
> > _______________________________________________
> > cfe-commits mailing list
> > cfe-commits at lists.llvm.org
> > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list