r286699 - [c++1z] Support constant folding for __builtin_strchr and __builtin_memchr.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 15 17:09:15 PST 2016
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
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161115/6ba880d2/attachment-0001.html>
More information about the cfe-commits
mailing list