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