Fix miscompiled 32bit binaries by mingw

[yvvan]

Summary

With that patch applied my 32-bit libclang.dll built with mingw does not crash anymore
unit tests are not affected by that change

Connected to
https://bugs.llvm.org/show_bug.cgi?id=32018

[hans]

From the bug, this is related to Richard's "r289413 - Add two new AST nodes to represent initialization of an array in terms of initialization of each array element", which broke MSVC builds due to under-alignment, which Reid addressed with "r289575 - Align EvalInfo in ExprConstant to avoid PointerUnion assertions".

Perhaps there's a problem with MinGW 4.9.2's alignas? Or is this https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78936 as Reid says in https://bugs.llvm.org/show_bug.cgi?id=32018#c8 ?

[yvvan]

The same alignment in other places works fine. i don't know the source of that issue. Should be somewhere in gcc(mingw) but it's not my focus.
So this is a workaround that we can make (probably only for mingw builds)

[rnk]

This doesn't seem like an acceptable workaround, surely this regresses functionality for arrays with more than UINT_MAX elements.

[yvvan]

Provide a workaround without regression

[rnk]

lib/AST/ExprConstant.cpp

588

Surely this will fault on SPARC or ARM or other ISAs that care about alignment?

[t.p.northover]

lib/AST/ExprConstant.cpp

588

As well as being horribly undefined behaviour.

[yvvan]

Ok, i will make safer solution...

[yvvan]

Make safe solution without regression

[yvvan]

ping. Do you think this is ok now?

[martell]

@rnk isn't this related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78936
Assuming this is in fact a gcc bug this will most likely cause a regression with the standalone clang mingw toolchains we are on the verge of having.
This is of course assuming clang does not have the same regression gcc does.

[rnk]

I looked at the blame, and I added this alignment thing in rL289575 to deal with some PointerIntPair assertions. Those probably started in @gbiv's rL270781, which introduced a PointerIntPair<EvalInfo*,1,bool> field. I think we can just fix this problem by not being so clever. We can unpack the boolean and the pointer, and everything will work fine.

In D34873#831698, @martell wrote:

@rnk isn't this related to https://gcc.gnu.org/bugzilla/show_bug.cgi?id=78936
Assuming this is in fact a gcc bug this will most likely cause a regression with the standalone clang mingw toolchains we are on the verge of having.
This is of course assuming clang does not have the same regression gcc does.

It's possible, but nobody has debugged any of these crashes and shown the same ret $imm inconsistencies that I was pointing out in that bug. This seems unrelated at first.

[rnk]

This shouldn't be necessary after rL310905.

[yvvan]

In D34873#841624, @rnk wrote:

This shouldn't be necessary after rL310905.

lib/AST/ExprConstant.cpp

namespace {
   /// "integer constant expression" or constant expression.  If not, this struct
   /// captures information about how and why not.
   ///
   /// One bit of information passed *into* the request for constant folding
   /// indicates whether the subexpression is "evaluated" or not according to C
   /// rules.  For example, the RHS of (0 && foo()) is not evaluated.  We can
   /// evaluate the expression regardless of what the RHS is, but C only allows
   /// certain things in certain situations.
+#ifndef _WIN32
   struct LLVM_ALIGNAS(/*alignof(uint64_t)*/ 8) EvalInfo {
+#else
+  struct EvalInfo {
+#endif
     ASTContext &Ctx;
 
     /// EvalStatus - Contains information about the evaluation.
     Expr::EvalStatus &EvalStatus;
 
     /// CurrentCall - The top of the constexpr call stack.
     CallStackFrame *CurrentCall;
 
#endif
     APValue::LValueBase EvaluatingDecl;
 
     /// EvaluatingDeclValue - This is the value being constructed for the
     /// declaration whose initializer is being evaluated, if any.
     APValue *EvaluatingDeclValue;
 
     /// The current array initialization index, if we're performing array
     /// initialization.
+#ifndef _WIN32
     uint64_t ArrayInitIndex = -1;
+#else
+    /// uint64_t value is split into two uint32_t values as a workaround
+    /// to deal with mingw 32-bit miscompilation
+    uint32_t ArrayInitIndex[2] = {static_cast<uint32_t>(-1), static_cast<uint32_t>(-1)};
+    uint64_t GetArrayInitIndex() {
+      return (static_cast<uint64_t>(ArrayInitIndex[0]) << 32)
+        + static_cast<uint64_t>(ArrayInitIndex[1]);
+    }
+    void SetArrayInitIndex(uint64_t index) {
+        ArrayInitIndex[0] = static_cast<uint32_t>(index >> 32);
+        ArrayInitIndex[1] = static_cast<uint32_t>(index);
+    }
+#endif
 
     /// HasActiveDiagnostic - Was the previous diagnostic stored? If so, further
     /// notes attached to it will also be stored, otherwise they will not be.
     bool HasActiveDiagnostic;
 
     /// \brief Have we emitted a diagnostic explaining why we couldn't constant
     /// fold (not just why it's not strictly a constant expression)?
     bool HasFoldFailureDiagnostic;
LLVM_NODISCARD bool noteFailure() {
       return KeepGoing;
     }
 
     class ArrayInitLoopIndex {
       EvalInfo &Info;
       uint64_t OuterIndex;
 
     public:
+#ifndef _WIN32
       ArrayInitLoopIndex(EvalInfo &Info)
           : Info(Info), OuterIndex(Info.ArrayInitIndex) {
         Info.ArrayInitIndex = 0;
       }
       ~ArrayInitLoopIndex() { Info.ArrayInitIndex = OuterIndex; }
 
       operator uint64_t&() { return Info.ArrayInitIndex; }
+#else
+      ArrayInitLoopIndex(EvalInfo &Info)
+          : Info(Info), OuterIndex(Info.GetArrayInitIndex()) {
+        Info.SetArrayInitIndex(0);
+      }
+      ~ArrayInitLoopIndex() { Info.SetArrayInitIndex(OuterIndex); }
+
+      operator uint64_t() { return Info.GetArrayInitIndex(); }
+      ArrayInitLoopIndex& operator++() {
+          Info.SetArrayInitIndex(Info.GetArrayInitIndex() + 1);
+          return *this;
+      }
+#endif
     };
   };
 
   /// Object used to treat all foldable expressions as constant expressions.
   struct FoldConstant {
     EvalInfo &Info;
     bool Enabled;
     bool HadNoPriorDiags;
bool VisitCXXBoolLiteralExpr(const CXXBoolLiteralExpr *E) {
     return Success(E->getValue(), E);
   }
 
   bool VisitObjCBoolLiteralExpr(const ObjCBoolLiteralExpr *E) {
     return Success(E->getValue(), E);
   }
 
   bool VisitArrayInitIndexExpr(const ArrayInitIndexExpr *E) {
+#ifndef _WIN32
     if (Info.ArrayInitIndex == uint64_t(-1)) {
+#else
+    if (Info.GetArrayInitIndex() == uint64_t(-1)) {
+#endif
       // We were asked to evaluate this subexpression independent of the
       // enclosing ArrayInitLoopExpr. We can't do that.
       Info.FFDiag(E);
       return false;
     }
+#ifndef _WIN32
     return Success(Info.ArrayInitIndex, E);
+#else
+    return Success(Info.GetArrayInitIndex(), E);
+#endif
   }
     
   // Note, GNU defines __null as an integer, not a pointer.
   bool VisitGNUNullExpr(const GNUNullExpr *E) {
     return ZeroInitialization(E);
   }
 
   bool VisitTypeTraitExpr(const TypeTraitExpr *E) {