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 15:39:24 PST 2016


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?

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