[clang] 673ef8c - Re-land "[clang][Interp] Make sure we push integers of the correct size"

Timm Bäder via cfe-commits cfe-commits at lists.llvm.org
Fri Aug 18 01:26:14 PDT 2023


Author: Timm Bäder
Date: 2023-08-18T10:25:54+02:00
New Revision: 673ef8ceaece6c9a7194474ef7d97b3b240c0dc5

URL: https://github.com/llvm/llvm-project/commit/673ef8ceaece6c9a7194474ef7d97b3b240c0dc5
DIFF: https://github.com/llvm/llvm-project/commit/673ef8ceaece6c9a7194474ef7d97b3b240c0dc5.diff

LOG: Re-land "[clang][Interp] Make sure we push integers of the correct size"

This also re-reverts the commit implementing __builtin_strlen.

I was unable to reproduce the msan issue with an msan-enabled build (for
infrastructure reasons), but I think fixing the target-dependent int
sizes in the implementation of __builtin_fpclassify should work.

Added: 
    

Modified: 
    clang/lib/AST/Interp/Integral.h
    clang/lib/AST/Interp/InterpBuiltin.cpp
    clang/test/AST/Interp/builtin-functions.cpp

Removed: 
    


################################################################################
diff  --git a/clang/lib/AST/Interp/Integral.h b/clang/lib/AST/Interp/Integral.h
index de588ab8c9f191..8d5edbb5b764ed 100644
--- a/clang/lib/AST/Interp/Integral.h
+++ b/clang/lib/AST/Interp/Integral.h
@@ -94,6 +94,7 @@ template <unsigned Bits, bool Signed> class Integral final {
   explicit operator unsigned() const { return V; }
   explicit operator int64_t() const { return V; }
   explicit operator uint64_t() const { return V; }
+  explicit operator int32_t() const { return V; }
 
   APSInt toAPSInt() const {
     return APSInt(APInt(Bits, static_cast<uint64_t>(V), Signed), !Signed);

diff  --git a/clang/lib/AST/Interp/InterpBuiltin.cpp b/clang/lib/AST/Interp/InterpBuiltin.cpp
index 5277d05c82adfb..c607368f3b6582 100644
--- a/clang/lib/AST/Interp/InterpBuiltin.cpp
+++ b/clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -21,17 +21,86 @@ static T getParam(const InterpFrame *Frame, unsigned Index) {
   return Frame->getParam<T>(Offset);
 }
 
+PrimType getIntPrimType(const InterpState &S) {
+  const TargetInfo &TI = S.getCtx().getTargetInfo();
+  unsigned IntWidth = TI.getIntWidth();
+
+  if (IntWidth == 32)
+    return PT_Sint32;
+  else if (IntWidth == 16)
+    return PT_Sint16;
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
 /// Peek an integer value from the stack into an APSInt.
-static APSInt peekToAPSInt(InterpStack &Stk, PrimType T) {
+static APSInt peekToAPSInt(InterpStack &Stk, PrimType T, size_t Offset = 0) {
+  if (Offset == 0)
+    Offset = align(primSize(T));
+
   APSInt R;
   INT_TYPE_SWITCH(T, {
-    T Val = Stk.peek<T>();
+    T Val = Stk.peek<T>(Offset);
     R = APSInt(APInt(T::bitWidth(), static_cast<uint64_t>(Val), T::isSigned()));
   });
 
   return R;
 }
 
+/// Pushes \p Val to the stack, as a target-dependent 'int'.
+static void pushInt(InterpState &S, int32_t Val) {
+  PrimType IntType = getIntPrimType(S);
+  if (IntType == PT_Sint32)
+    S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Val));
+  else if (IntType == PT_Sint16)
+    S.Stk.push<Integral<16, true>>(Integral<16, true>::from(Val));
+  else
+    llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static bool retInt(InterpState &S, CodePtr OpPC, APValue &Result) {
+  PrimType IntType = getIntPrimType(S);
+  if (IntType == PT_Sint32)
+    return Ret<PT_Sint32>(S, OpPC, Result);
+  else if (IntType == PT_Sint16)
+    return Ret<PT_Sint16>(S, OpPC, Result);
+  llvm_unreachable("Int isn't 16 or 32 bit?");
+}
+
+static void pushSizeT(InterpState &S, uint64_t Val) {
+  const TargetInfo &TI = S.getCtx().getTargetInfo();
+  unsigned SizeTWidth = TI.getTypeWidth(TI.getSizeType());
+
+  switch (SizeTWidth) {
+  case 64:
+    S.Stk.push<Integral<64, false>>(Integral<64, false>::from(Val));
+    break;
+  case 32:
+    S.Stk.push<Integral<32, false>>(Integral<32, false>::from(Val));
+    break;
+  case 16:
+    S.Stk.push<Integral<16, false>>(Integral<16, false>::from(Val));
+    break;
+  default:
+    llvm_unreachable("We don't handle this size_t size.");
+  }
+}
+
+static bool retSizeT(InterpState &S, CodePtr OpPC, APValue &Result) {
+  const TargetInfo &TI = S.getCtx().getTargetInfo();
+  unsigned SizeTWidth = TI.getTypeWidth(TI.getSizeType());
+
+  switch (SizeTWidth) {
+  case 64:
+    return Ret<PT_Uint64>(S, OpPC, Result);
+  case 32:
+    return Ret<PT_Uint32>(S, OpPC, Result);
+  case 16:
+    return Ret<PT_Uint16>(S, OpPC, Result);
+  }
+
+  llvm_unreachable("size_t isn't 64 or 32 bit?");
+}
+
 static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
                                    const InterpFrame *Frame) {
   const Pointer &A = getParam<Pointer>(Frame, 0);
@@ -67,7 +136,35 @@ static bool interp__builtin_strcmp(InterpState &S, CodePtr OpPC,
       break;
   }
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
+  return true;
+}
+
+static bool interp__builtin_strlen(InterpState &S, CodePtr OpPC,
+                                   const InterpFrame *Frame) {
+  const Pointer &StrPtr = getParam<Pointer>(Frame, 0);
+
+  if (!CheckArray(S, OpPC, StrPtr))
+    return false;
+
+  if (!CheckLive(S, OpPC, StrPtr, AK_Read))
+    return false;
+
+  assert(StrPtr.getFieldDesc()->isPrimitiveArray());
+
+  size_t Len = 0;
+  for (size_t I = StrPtr.getIndex();; ++I, ++Len) {
+    const Pointer &ElemPtr = StrPtr.atIndex(I);
+
+    if (!CheckRange(S, OpPC, ElemPtr, AK_Read))
+      return false;
+
+    uint8_t Val = ElemPtr.deref<uint8_t>();
+    if (Val == 0)
+      break;
+  }
+
+  pushSizeT(S, Len);
   return true;
 }
 
@@ -200,7 +297,7 @@ static bool interp__builtin_isnan(InterpState &S, CodePtr OpPC,
                                   const InterpFrame *Frame, const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isNan()));
+  pushInt(S, Arg.isNan());
   return true;
 }
 
@@ -211,10 +308,9 @@ static bool interp__builtin_isinf(InterpState &S, CodePtr OpPC,
   bool IsInf = Arg.isInf();
 
   if (CheckSign)
-    S.Stk.push<Integral<32, true>>(
-        Integral<32, true>::from(IsInf ? (Arg.isNegative() ? -1 : 1) : 0));
+    pushInt(S, IsInf ? (Arg.isNegative() ? -1 : 1) : 0);
   else
-    S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isInf()));
+    pushInt(S, Arg.isInf());
   return true;
 }
 
@@ -223,7 +319,7 @@ static bool interp__builtin_isfinite(InterpState &S, CodePtr OpPC,
                                      const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isFinite()));
+  pushInt(S, Arg.isFinite());
   return true;
 }
 
@@ -232,7 +328,7 @@ static bool interp__builtin_isnormal(InterpState &S, CodePtr OpPC,
                                      const Function *F) {
   const Floating &Arg = S.Stk.peek<Floating>();
 
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Arg.isNormal()));
+  pushInt(S, Arg.isNormal());
   return true;
 }
 
@@ -249,12 +345,12 @@ static bool interp__builtin_isfpclass(InterpState &S, CodePtr OpPC,
 
   int32_t Result =
       static_cast<int32_t>((F.classify() & FPClassArg).getZExtValue());
-  S.Stk.push<Integral<32, true>>(Integral<32, true>::from(Result));
+  pushInt(S, Result);
 
   return true;
 }
 
-/// Five int32 values followed by one floating value.
+/// Five int values followed by one floating value.
 static bool interp__builtin_fpclassify(InterpState &S, CodePtr OpPC,
                                        const InterpFrame *Frame,
                                        const Function *Func) {
@@ -277,11 +373,13 @@ static bool interp__builtin_fpclassify(InterpState &S, CodePtr OpPC,
   }
 
   // The last argument is first on the stack.
-  unsigned Offset = align(primSize(PT_Float)) +
-                    ((1 + (4 - Index)) * align(primSize(PT_Sint32)));
+  assert(Index <= 4);
+  unsigned IntSize = primSize(getIntPrimType(S));
+  unsigned Offset =
+      align(primSize(PT_Float)) + ((1 + (4 - Index)) * align(IntSize));
 
-  const Integral<32, true> &I = S.Stk.peek<Integral<32, true>>(Offset);
-  S.Stk.push<Integral<32, true>>(I);
+  APSInt I = peekToAPSInt(S.Stk, getIntPrimType(S), Offset);
+  pushInt(S, I.getZExtValue());
   return true;
 }
 
@@ -312,7 +410,11 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
     return RetVoid(S, OpPC, Dummy);
   case Builtin::BI__builtin_strcmp:
     if (interp__builtin_strcmp(S, OpPC, Frame))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
+    break;
+  case Builtin::BI__builtin_strlen:
+    if (interp__builtin_strlen(S, OpPC, Frame))
+      return retSizeT(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_nan:
   case Builtin::BI__builtin_nanf:
@@ -372,34 +474,34 @@ bool InterpretBuiltin(InterpState &S, CodePtr OpPC, const Function *F,
 
   case Builtin::BI__builtin_isnan:
     if (interp__builtin_isnan(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isinf:
     if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/false))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isinf_sign:
     if (interp__builtin_isinf(S, OpPC, Frame, F, /*Sign=*/true))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_isfinite:
     if (interp__builtin_isfinite(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_isnormal:
     if (interp__builtin_isnormal(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_isfpclass:
     if (interp__builtin_isfpclass(S, OpPC, Frame, F, Call))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
   case Builtin::BI__builtin_fpclassify:
     if (interp__builtin_fpclassify(S, OpPC, Frame, F))
-      return Ret<PT_Sint32>(S, OpPC, Dummy);
+      return retInt(S, OpPC, Dummy);
     break;
 
   case Builtin::BI__builtin_fabs:

diff  --git a/clang/test/AST/Interp/builtin-functions.cpp b/clang/test/AST/Interp/builtin-functions.cpp
index 85f10d7594d8c9..55ebab1122d58e 100644
--- a/clang/test/AST/Interp/builtin-functions.cpp
+++ b/clang/test/AST/Interp/builtin-functions.cpp
@@ -1,7 +1,11 @@
-// RUN: %clang_cc1 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -verify=ref %s -Wno-constant-evaluated
-// RUN: %clang_cc1 -std=c++20 -fexperimental-new-constant-interpreter %s -verify
-// RUN: %clang_cc1 -std=c++20 -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter -triple i686 %s -verify
+// RUN: %clang_cc1 -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
+// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -fexperimental-new-constant-interpreter %s -verify
+// RUN: %clang_cc1 -triple avr -std=c++20 -Wno-string-plus-int -verify=ref %s -Wno-constant-evaluated
 
 
 namespace strcmp {
@@ -38,6 +42,70 @@ namespace strcmp {
                                                                         // ref-note {{dereferenced one-past-the-end}}
 }
 
+/// Copied from constant-expression-cxx11.cpp
+namespace strlen {
+constexpr const char *a = "foo\0quux";
+  constexpr char b[] = "foo\0quux";
+  constexpr int f() { return 'u'; }
+  constexpr char c[] = { 'f', 'o', 'o', 0, 'q', f(), 'u', 'x', 0 };
+
+  static_assert(__builtin_strlen("foo") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux") == 3, "");
+  static_assert(__builtin_strlen("foo\0quux" + 4) == 4, "");
+
+  constexpr bool check(const char *p) {
+    return __builtin_strlen(p) == 3 &&
+           __builtin_strlen(p + 1) == 2 &&
+           __builtin_strlen(p + 2) == 1 &&
+           __builtin_strlen(p + 3) == 0 &&
+           __builtin_strlen(p + 4) == 4 &&
+           __builtin_strlen(p + 5) == 3 &&
+           __builtin_strlen(p + 6) == 2 &&
+           __builtin_strlen(p + 7) == 1 &&
+           __builtin_strlen(p + 8) == 0;
+  }
+
+  static_assert(check(a), "");
+  static_assert(check(b), "");
+  static_assert(check(c), "");
+
+  constexpr int over1 = __builtin_strlen(a + 9); // expected-error {{constant expression}} \
+                                                 // expected-note {{one-past-the-end}} \
+                                                 // expected-note {{in call to}} \
+                                                 // ref-error {{constant expression}} \
+                                                 // ref-note {{one-past-the-end}}
+  constexpr int over2 = __builtin_strlen(b + 9); // expected-error {{constant expression}} \
+                                                 // expected-note {{one-past-the-end}} \
+                                                 // expected-note {{in call to}} \
+                                                 // ref-error {{constant expression}} \
+                                                 // ref-note {{one-past-the-end}}
+  constexpr int over3 = __builtin_strlen(c + 9); // expected-error {{constant expression}} \
+                                                 // expected-note {{one-past-the-end}} \
+                                                 // expected-note {{in call to}} \
+                                                 // ref-error {{constant expression}} \
+                                                 // ref-note {{one-past-the-end}}
+
+  constexpr int under1 = __builtin_strlen(a - 1); // expected-error {{constant expression}} \
+                                                  // expected-note {{cannot refer to element -1}} \
+                                                  // ref-error {{constant expression}} \
+                                                  // ref-note {{cannot refer to element -1}}
+  constexpr int under2 = __builtin_strlen(b - 1); // expected-error {{constant expression}} \
+                                                  // expected-note {{cannot refer to element -1}} \
+                                                  // ref-error {{constant expression}} \
+                                                  // ref-note {{cannot refer to element -1}}
+  constexpr int under3 = __builtin_strlen(c - 1); // expected-error {{constant expression}} \
+                                                  // expected-note {{cannot refer to element -1}} \
+                                                  // ref-error {{constant expression}} \
+                                                  // ref-note {{cannot refer to element -1}}
+
+  constexpr char d[] = { 'f', 'o', 'o' }; // no nul terminator.
+  constexpr int bad = __builtin_strlen(d); // expected-error {{constant expression}} \
+                                           // expected-note {{one-past-the-end}} \
+                                           // expected-note {{in call to}} \
+                                           // ref-error {{constant expression}} \
+                                           // ref-note {{one-past-the-end}}
+}
+
 namespace nan {
   constexpr double NaN1 = __builtin_nan("");
 
@@ -96,10 +164,12 @@ namespace isfpclass {
   char isfpclass_pos_1    [!__builtin_isfpclass(1.0f, 0x0008) ? 1 : -1]; // fcNegNormal
   char isfpclass_pos_2    [__builtin_isfpclass(1.0L, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pos_3    [!__builtin_isfpclass(1.0, 0x0003) ? 1 : -1]; // fcSNan|fcQNan
+#ifndef __AVR__
   char isfpclass_pdenorm_0[__builtin_isfpclass(1.0e-40f, 0x0080) ? 1 : -1]; // fcPosSubnormal
   char isfpclass_pdenorm_1[__builtin_isfpclass(1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pdenorm_2[!__builtin_isfpclass(1.0e-40f, 0x003C) ? 1 : -1]; // fcNegative
   char isfpclass_pdenorm_3[!__builtin_isfpclass(1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+#endif
   char isfpclass_pzero_0  [__builtin_isfpclass(0.0f, 0x0060) ? 1 : -1]; // fcZero
   char isfpclass_pzero_1  [__builtin_isfpclass(0.0, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_pzero_2  [!__builtin_isfpclass(0.0L, 0x0020) ? 1 : -1]; // fcNegZero
@@ -109,9 +179,11 @@ namespace isfpclass {
   char isfpclass_nzero_2  [!__builtin_isfpclass(-0.0L, 0x0040) ? 1 : -1]; // fcPosZero
   char isfpclass_nzero_3  [!__builtin_isfpclass(-0.0, 0x0003) ? 1 : -1]; // fcNan
   char isfpclass_ndenorm_0[__builtin_isfpclass(-1.0e-40f, 0x0010) ? 1 : -1]; // fcNegSubnormal
-  char isfpclass_ndenorm_1[__builtin_isfpclass(-1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_ndenorm_2[!__builtin_isfpclass(-1.0e-40f, 0x03C0) ? 1 : -1]; // fcPositive
+#ifndef __AVR__
+  char isfpclass_ndenorm_1[__builtin_isfpclass(-1.0e-310, 0x01F8) ? 1 : -1]; // fcFinite
   char isfpclass_ndenorm_3[!__builtin_isfpclass(-1.0e-310, 0x0207) ? 1 : -1]; // ~fcFinite
+#endif
   char isfpclass_neg_0    [__builtin_isfpclass(-1.0, 0x0008) ? 1 : -1]; // fcNegNormal
   char isfpclass_neg_1    [!__builtin_isfpclass(-1.0f, 0x00100) ? 1 : -1]; // fcPosNormal
   char isfpclass_neg_2    [__builtin_isfpclass(-1.0L, 0x01F8) ? 1 : -1]; // fcFinite
@@ -136,9 +208,11 @@ namespace fpclassify {
   char classify_inf     [__builtin_fpclassify(-1, +1, -1, -1, -1, __builtin_inf())];
   char classify_neg_inf [__builtin_fpclassify(-1, +1, -1, -1, -1, -__builtin_inf())];
   char classify_normal  [__builtin_fpclassify(-1, -1, +1, -1, -1, 1.539)];
+#ifndef __AVR__
   char classify_normal2 [__builtin_fpclassify(-1, -1, +1, -1, -1, 1e-307)];
   char classify_denorm  [__builtin_fpclassify(-1, -1, -1, +1, -1, 1e-308)];
   char classify_denorm2 [__builtin_fpclassify(-1, -1, -1, +1, -1, -1e-308)];
+#endif
   char classify_zero    [__builtin_fpclassify(-1, -1, -1, -1, +1, 0.0)];
   char classify_neg_zero[__builtin_fpclassify(-1, -1, -1, -1, +1, -0.0)];
   char classify_subnorm [__builtin_fpclassify(-1, -1, -1, +1, -1, 1.0e-38f)];


        


More information about the cfe-commits mailing list