Fix bug 26547

via cfe-commits cfe-commits at lists.llvm.org
Thu Oct 11 09:34:29 PDT 2018


This is a patch to fix bug 26547
(https://bugs.llvm.org/show_bug.cgi?id=26547)

This is my first patch, so please give feedback! This should be correct
though, and I've tested it on both Windows x86 (where it does nothing), and
Linux x86 (where it correctly returns alignof(double) = 4, __alignof(double)
= 8).

I'm not sure where to put a unit test for this, and I'm also unsure how to
test for CPU and OS; as far as I know, it's only really an issue with x86
SysV's double.

---

diff --git a/include/clang/ASTMatchers/ASTMatchers.h
b/include/clang/ASTMatchers/ASTMatchers.h
index eb07ff70a9..3f8690bd8a 100644
--- a/include/clang/ASTMatchers/ASTMatchers.h
+++ b/include/clang/ASTMatchers/ASTMatchers.h
@@ -2425,8 +2425,9 @@ AST_MATCHER_P(UnaryExprOrTypeTraitExpr, ofKind,
UnaryExprOrTypeTrait, Kind) {
 /// alignof.
 inline internal::Matcher<Stmt> alignOfExpr(
     const internal::Matcher<UnaryExprOrTypeTraitExpr> &InnerMatcher) {
-  return stmt(unaryExprOrTypeTraitExpr(allOf(
-      ofKind(UETT_AlignOf), InnerMatcher)));
+  return stmt(unaryExprOrTypeTraitExpr(
+      allOf(anyOf(ofKind(UETT_AlignOf), ofKind(UETT_PreferredAlignOf)),
+            InnerMatcher)));
 }
 
 /// Same as unaryExprOrTypeTraitExpr, but only matching
diff --git a/include/clang/Basic/LangOptions.h
b/include/clang/Basic/LangOptions.h
index 30ee20395b..2ac4ae52e3 100644
--- a/include/clang/Basic/LangOptions.h
+++ b/include/clang/Basic/LangOptions.h
@@ -124,6 +124,13 @@ public:
     /// whether we reuse base class tail padding in some ABIs.
     Ver6,
 
+    /// Attempt to be ABI-compatible with code generated by Clang 7.0.x
+    /// (SVN r344257). This causes alignof (C++) and _Alignof (C11) to be
+    /// compatible with __alignof (i.e., return the preferred alignment)
+    /// rather than returning the required alignment.
+    /// see https://bugs.llvm.org/show_bug.cgi?id=26547 for explanation
+    Ver7,
+
     /// Conform to the underlying platform's C and C++ ABIs as closely
     /// as we can.
     Latest
@@ -273,8 +280,8 @@ public:
 /// Floating point control options
 class FPOptions {
 public:
-  FPOptions() : fp_contract(LangOptions::FPC_Off), 
-                fenv_access(LangOptions::FEA_Off) {}
+  FPOptions()
+      : fp_contract(LangOptions::FPC_Off),
fenv_access(LangOptions::FEA_Off) {}
 
   // Used for serializing.
   explicit FPOptions(unsigned I)
@@ -319,7 +326,7 @@ public:
   unsigned getInt() const { return fp_contract | (fenv_access << 2); }
 
 private:
-  /// Adjust BinaryOperator::FPFeatures to match the total bit-field size 
+  /// Adjust BinaryOperator::FPFeatures to match the total bit-field size
   /// of these two.
   unsigned fp_contract : 2;
   unsigned fenv_access : 1;
diff --git a/include/clang/Basic/TypeTraits.h
b/include/clang/Basic/TypeTraits.h
index bdb426834a..e4e54d4822 100644
--- a/include/clang/Basic/TypeTraits.h
+++ b/include/clang/Basic/TypeTraits.h
@@ -96,6 +96,12 @@ namespace clang {
   /// Names for the "expression or type" traits.
   enum UnaryExprOrTypeTrait {
     UETT_SizeOf,
+    // used for GCC's __alignof,
+    UETT_PreferredAlignOf,
+    // used for C's _Alignof and C++'s alignof
+    // this distinction is important because __alignof
+    // returns preferred alignment
+    // _Alignof and alignof return the required alignment
     UETT_AlignOf,
     UETT_VecStep,
     UETT_OpenMPRequiredSimdAlign,
diff --git a/lib/AST/ASTDumper.cpp b/lib/AST/ASTDumper.cpp
index 38a2fe9caf..56592b8c65 100644
--- a/lib/AST/ASTDumper.cpp
+++ b/lib/AST/ASTDumper.cpp
@@ -2272,6 +2272,9 @@ void ASTDumper::VisitUnaryExprOrTypeTraitExpr(
   case UETT_SizeOf:
     OS << " sizeof";
     break;
+  case UETT_PreferredAlignOf:
+    OS << " __alignof";
+    break;
   case UETT_AlignOf:
     OS << " alignof";
     break;
diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
index 651981374d..7ff90f176f 100644
--- a/lib/AST/Expr.cpp
+++ b/lib/AST/Expr.cpp
@@ -1446,7 +1446,7 @@ UnaryExprOrTypeTraitExpr::UnaryExprOrTypeTraitExpr(
 
   // Check to see if we are in the situation where alignof(decl) should be
   // dependent because decl's alignment is dependent.
-  if (ExprKind == UETT_AlignOf) {
+  if (ExprKind == UETT_AlignOf || ExprKind == UETT_PreferredAlignOf) {
     if (!isValueDependent() || !isInstantiationDependent()) {
       E = E->IgnoreParens();
 
diff --git a/lib/AST/ExprConstant.cpp b/lib/AST/ExprConstant.cpp
index d2258cc212..4ed8198e60 100644
--- a/lib/AST/ExprConstant.cpp
+++ b/lib/AST/ExprConstant.cpp
@@ -5946,21 +5946,33 @@ bool PointerExprEvaluator::VisitCastExpr(const
CastExpr *E) {
   return ExprEvaluatorBaseTy::VisitCastExpr(E);
 }
 
-static CharUnits GetAlignOfType(EvalInfo &Info, QualType T) {
+static CharUnits GetAlignOfType(EvalInfo &Info, QualType T,
+                                bool PreferredAlignOf) {
   // C++ [expr.alignof]p3:
   //     When alignof is applied to a reference type, the result is the
   //     alignment of the referenced type.
   if (const ReferenceType *Ref = T->getAs<ReferenceType>())
     T = Ref->getPointeeType();
 
-  // __alignof is defined to return the preferred alignment.
   if (T.getQualifiers().hasUnaligned())
     return CharUnits::One();
-  return Info.Ctx.toCharUnitsFromBits(
-    Info.Ctx.getPreferredTypeAlign(T.getTypePtr()));
+
+  const bool alignOfReturnsPreferred =
+      Info.Ctx.getLangOpts().getClangABICompat() <=
LangOptions::ClangABI::Ver7;
+  // __alignof is defined to return the preferred alignment.
+  // before 8, clang returned the preferred alignment for alignof and
_Alignof
+  // as well
+  if (PreferredAlignOf || alignOfReturnsPreferred)
+    return Info.Ctx.toCharUnitsFromBits(
+        Info.Ctx.getPreferredTypeAlign(T.getTypePtr()));
+  // alignof (C++) and _Alignof (C11) are defined to return the ABI
alignment
+  // see https://bugs.llvm.org/show_bug.cgi?id=26547 for explanation
+  else
+    return Info.Ctx.getTypeAlignInChars(T.getTypePtr());
 }
 
-static CharUnits GetAlignOfExpr(EvalInfo &Info, const Expr *E) {
+static CharUnits GetAlignOfExpr(EvalInfo &Info, const Expr *E,
+                                bool PreferredAlignOf) {
   E = E->IgnoreParens();
 
   // The kinds of expressions that we have special-case logic here for
@@ -5977,7 +5989,7 @@ static CharUnits GetAlignOfExpr(EvalInfo &Info, const
Expr *E) {
     return Info.Ctx.getDeclAlign(ME->getMemberDecl(),
                                  /*RefAsPointee*/true);
 
-  return GetAlignOfType(Info, E->getType());
+  return GetAlignOfType(Info, E->getType(), PreferredAlignOf);
 }
 
 // To be clear: this happily visits unsupported builtins. Better name
welcomed.
@@ -6039,7 +6051,7 @@ bool PointerExprEvaluator::VisitBuiltinCallExpr(const
CallExpr *E,
         BaseAlignment = Info.Ctx.getDeclAlign(VD);
       } else {
         BaseAlignment =
-          GetAlignOfExpr(Info, OffsetResult.Base.get<const Expr*>());
+            GetAlignOfExpr(Info, OffsetResult.Base.get<const Expr *>(),
false);
       }
 
       if (BaseAlignment < Align) {
@@ -9357,12 +9369,18 @@ bool IntExprEvaluator::VisitBinaryOperator(const
BinaryOperator *E) {
 /// a result as the expression's type.
 bool IntExprEvaluator::VisitUnaryExprOrTypeTraitExpr(
                                     const UnaryExprOrTypeTraitExpr *E) {
-  switch(E->getKind()) {
+  auto const kind = E->getKind();
+  switch (kind) {
+  case UETT_PreferredAlignOf:
   case UETT_AlignOf: {
     if (E->isArgumentType())
-      return Success(GetAlignOfType(Info, E->getArgumentType()), E);
+      return Success(GetAlignOfType(Info, E->getArgumentType(),
+                                    kind == UETT_PreferredAlignOf),
+                     E);
     else
-      return Success(GetAlignOfExpr(Info, E->getArgumentExpr()), E);
+      return Success(GetAlignOfExpr(Info, E->getArgumentExpr(),
+                                    kind == UETT_PreferredAlignOf),
+                     E);
   }
 
   case UETT_VecStep: {
diff --git a/lib/AST/ItaniumMangle.cpp b/lib/AST/ItaniumMangle.cpp
index e99549850a..9caa2db3d7 100644
--- a/lib/AST/ItaniumMangle.cpp
+++ b/lib/AST/ItaniumMangle.cpp
@@ -3881,6 +3881,7 @@ recurse:
     case UETT_SizeOf:
       Out << 's';
       break;
+    case UETT_PreferredAlignOf:
     case UETT_AlignOf:
       Out << 'a';
       break;
diff --git a/lib/AST/StmtPrinter.cpp b/lib/AST/StmtPrinter.cpp
index e9d351e868..cf6240c07f 100644
--- a/lib/AST/StmtPrinter.cpp
+++ b/lib/AST/StmtPrinter.cpp
@@ -1686,6 +1686,9 @@ void
StmtPrinter::VisitUnaryExprOrTypeTraitExpr(UnaryExprOrTypeTraitExpr *Node){
     else
       OS << "__alignof";
     break;
+  case UETT_PreferredAlignOf:
+    OS << "__alignof";
+    break;
   case UETT_VecStep:
     OS << "vec_step";
     break;
diff --git a/lib/Parse/ParseExpr.cpp b/lib/Parse/ParseExpr.cpp
index b9028bffa1..39369ea9ef 100644
--- a/lib/Parse/ParseExpr.cpp
+++ b/lib/Parse/ParseExpr.cpp
@@ -2022,7 +2022,9 @@ ExprResult
Parser::ParseUnaryExprOrTypeTraitExpression() {
                                                           CastRange);
 
   UnaryExprOrTypeTrait ExprKind = UETT_SizeOf;
-  if (OpTok.isOneOf(tok::kw_alignof, tok::kw___alignof, tok::kw__Alignof))
+  if (OpTok.is(tok::kw___alignof))
+    ExprKind = UETT_PreferredAlignOf;
+  else if (OpTok.isOneOf(tok::kw_alignof, tok::kw__Alignof))
     ExprKind = UETT_AlignOf;
   else if (OpTok.is(tok::kw_vec_step))
     ExprKind = UETT_VecStep;
diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
index 113626374c..9e3a6df7c1 100644
--- a/lib/Sema/SemaChecking.cpp
+++ b/lib/Sema/SemaChecking.cpp
@@ -5695,7 +5695,8 @@ bool Sema::SemaBuiltinAllocaWithAlign(CallExpr
*TheCall) {
   if (!Arg->isTypeDependent() && !Arg->isValueDependent()) {
     if (const auto *UE =
             dyn_cast<UnaryExprOrTypeTraitExpr>(Arg->IgnoreParenImpCasts()))
-      if (UE->getKind() == UETT_AlignOf)
+      if (UE->getKind() == UETT_AlignOf ||
+          UE->getKind() == UETT_PreferredAlignOf)
         Diag(TheCall->getBeginLoc(), diag::warn_alloca_align_alignof)
             << Arg->getSourceRange();
 
@@ -10313,7 +10314,7 @@ static void AnalyzeAssignment(Sema &S,
BinaryOperator *E) {
   }
 
   AnalyzeImplicitConversions(S, E->getRHS(), E->getOperatorLoc());
-  
+
   // Diagnose implicitly sequentially-consistent atomic assignment.
   if (E->getLHS()->getType()->isAtomicType())
     S.Diag(E->getRHS()->getBeginLoc(), diag::warn_atomic_implicit_seq_cst);
diff --git a/lib/Sema/SemaExpr.cpp b/lib/Sema/SemaExpr.cpp
index 8144b3a779..801c52d5b1 100644
--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3596,7 +3596,8 @@ static bool CheckExtensionTraitOperandType(Sema &S,
QualType T,
 
   // C99 6.5.3.4p1:
   if (T->isFunctionType() &&
-      (TraitKind == UETT_SizeOf || TraitKind == UETT_AlignOf)) {
+      (TraitKind == UETT_SizeOf || TraitKind == UETT_AlignOf ||
+       TraitKind == UETT_PreferredAlignOf)) {
     // sizeof(function)/alignof(function) is allowed as an extension.
     S.Diag(Loc, diag::ext_sizeof_alignof_function_type)
       << TraitKind << ArgRange;
@@ -3674,7 +3675,7 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
   // the expression to be complete. 'sizeof' requires the expression's type
to
   // be complete (and will attempt to complete it if it's an array of
unknown
   // bound).
-  if (ExprKind == UETT_AlignOf) {
+  if (ExprKind == UETT_AlignOf || ExprKind == UETT_PreferredAlignOf) {
     if (RequireCompleteType(E->getExprLoc(),
                             Context.getBaseElementType(E->getType()),
                             diag::err_sizeof_alignof_incomplete_type,
ExprKind,
@@ -3698,7 +3699,8 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(Expr *E,
 
   // The operand for sizeof and alignof is in an unevaluated expression
context,
   // so side effects could result in unintended consequences.
-  if ((ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf) &&
+  if ((ExprKind == UETT_SizeOf || ExprKind == UETT_AlignOf ||
+       ExprKind == UETT_PreferredAlignOf) &&
       !inTemplateInstantiation() && E->HasSideEffects(Context, false))
     Diag(E->getExprLoc(), diag::warn_side_effects_unevaluated_context);
 
@@ -3767,7 +3769,8 @@ bool Sema::CheckUnaryExprOrTypeTraitOperand(QualType
ExprType,
   // C11 6.5.3.4/3, C++11 [expr.alignof]p3:
   //   When alignof or _Alignof is applied to an array type, the result
   //   is the alignment of the element type.
-  if (ExprKind == UETT_AlignOf || ExprKind == UETT_OpenMPRequiredSimdAlign)
+  if (ExprKind == UETT_AlignOf || ExprKind == UETT_PreferredAlignOf ||
+      ExprKind == UETT_OpenMPRequiredSimdAlign)
     ExprType = Context.getBaseElementType(ExprType);
 
   if (ExprKind == UETT_VecStep)
@@ -4046,14 +4049,14 @@ Sema::CreateUnaryExprOrTypeTraitExpr(Expr *E,
SourceLocation OpLoc,
   bool isInvalid = false;
   if (E->isTypeDependent()) {
     // Delay type-checking for type-dependent expressions.
-  } else if (ExprKind == UETT_AlignOf) {
+  } else if (ExprKind == UETT_AlignOf || ExprKind == UETT_PreferredAlignOf)
{
     isInvalid = CheckAlignOfExpr(*this, E);
   } else if (ExprKind == UETT_VecStep) {
     isInvalid = CheckVecStepExpr(E);
   } else if (ExprKind == UETT_OpenMPRequiredSimdAlign) {
-      Diag(E->getExprLoc(), diag::err_openmp_default_simd_align_expr);
-      isInvalid = true;
-  } else if (E->refersToBitField()) {  // C99 6.5.3.4p1.
+    Diag(E->getExprLoc(), diag::err_openmp_default_simd_align_expr);
+    isInvalid = true;
+  } else if (E->refersToBitField()) { // C99 6.5.3.4p1.
     Diag(E->getExprLoc(), diag::err_sizeof_alignof_typeof_bitfield) << 0;
     isInvalid = true;
   } else {



More information about the cfe-commits mailing list