[llvm-commits] [dragonegg] r163499 - in /dragonegg/trunk: src/Convert.cpp test/validator/c/2012-09-10-MRVOverflow.c

Duncan Sands baldrick at free.fr
Mon Sep 10 03:27:21 PDT 2012


Author: baldrick
Date: Mon Sep 10 05:27:20 2012
New Revision: 163499

URL: http://llvm.org/viewvc/llvm-project?rev=163499&view=rev
Log:
Dragonegg's multiple return value (MRV) code returns this struct of three i32's
as a pair { i64, i64 }.  It was generating an i64 load from the address of the
third i32, i.e. reading beyond the end of source object.  Spotted by Chandler's
new SROA pass which replaced the load with undef.

Added:
    dragonegg/trunk/test/validator/c/2012-09-10-MRVOverflow.c
Modified:
    dragonegg/trunk/src/Convert.cpp

Modified: dragonegg/trunk/src/Convert.cpp
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/src/Convert.cpp?rev=163499&r1=163498&r2=163499&view=diff
==============================================================================
--- dragonegg/trunk/src/Convert.cpp (original)
+++ dragonegg/trunk/src/Convert.cpp Mon Sep 10 05:27:20 2012
@@ -1324,44 +1324,68 @@
   // If the function returns a value, get it into a register and return it now.
   if (!Fn->getReturnType()->isVoidTy()) {
     tree TreeRetVal = DECL_RESULT(FnDecl);
+    LValue ResultLV = EmitLV(TreeRetVal);
+    assert(!ResultLV.isBitfield() && "Bitfields not allowed here!");
+
     if (!isa<AGGREGATE_TYPE>(TREE_TYPE(TreeRetVal)) &&
         !isa<COMPLEX_TYPE>(TREE_TYPE(TreeRetVal))) {
       // If the DECL_RESULT is a scalar type, just load out the return value
       // and return it.
-      Value *RetVal = Builder.CreateLoad(DECL_LOCAL(TreeRetVal), "retval");
-      RetVal = Builder.CreateBitCast(RetVal, Fn->getReturnType());
-      RetVals.push_back(RetVal);
+      LoadInst *Load = Builder.CreateLoad(ResultLV.Ptr);
+      Load->setAlignment(ResultLV.getAlignment());
+      RetVals.push_back(Builder.CreateBitCast(Load, Fn->getReturnType()));
     } else {
-      Value *RetVal = DECL_LOCAL(TreeRetVal);
-      if (StructType *STy = dyn_cast<StructType>(Fn->getReturnType())) {
-        Value *R1 = Builder.CreateBitCast(RetVal, STy->getPointerTo());
-
-        llvm::Value *Idxs[2];
-        Idxs[0] = Builder.getInt32(0);
-        for (unsigned ri = 0; ri < STy->getNumElements(); ++ri) {
-          Idxs[1] = Builder.getInt32(ri);
-          Value *GEP = Builder.CreateGEP(R1, Idxs, "mrv_gep");
-          Value *E = Builder.CreateLoad(GEP, "mrv");
-          RetVals.push_back(E);
-        }
-        // If the return type specifies an empty struct then return one.
-        if (RetVals.empty())
-          RetVals.push_back(UndefValue::get(Fn->getReturnType()));
+      uint64_t ResultSize =
+        getTargetData().getTypeAllocSize(ConvertType(TREE_TYPE(TreeRetVal)));
+      uint64_t ReturnSize =
+        getTargetData().getTypeAllocSize(Fn->getReturnType());
+
+      // The load does not necessarily start at the beginning of the aggregate
+      // (x86-64).
+      if (ReturnOffset >= ResultSize) {
+        // Also catches the case of an empty return value.
+        RetVals.push_back(UndefValue::get(Fn->getReturnType()));
       } else {
-        // Otherwise, this aggregate result must be something that is returned
-        // in a scalar register for this target.  We must bit convert the
-        // aggregate to the specified scalar type, which we do by casting the
-        // pointer and loading.  The load does not necessarily start at the
-        // beginning of the aggregate (x86-64).
+        // Advance to the point we want to load from.
         if (ReturnOffset) {
-          RetVal = Builder.CreateBitCast(RetVal, Type::getInt8PtrTy(Context));
-          RetVal = Builder.CreateGEP(RetVal,
-                     ConstantInt::get(TD.getIntPtrType(Context), ReturnOffset));
+          ResultLV.Ptr =
+            Builder.CreateBitCast(ResultLV.Ptr, Type::getInt8PtrTy(Context));
+          ResultLV.Ptr =
+            Builder.CreateGEP(ResultLV.Ptr,
+                              ConstantInt::get(TD.getIntPtrType(Context),
+                                               ReturnOffset));
+          ResultLV.setAlignment(MinAlign(ResultLV.getAlignment(), ReturnOffset));
+          ResultSize -= ReturnOffset;
+        }
+
+        // A place to build up the function return value.
+        MemRef ReturnLoc = CreateTempLoc(Fn->getReturnType());
+
+        // Copy out DECL_RESULT while being careful to not overrun the source or
+        // destination buffers.
+        uint64_t OctetsToCopy = std::min(ResultSize, ReturnSize);
+        EmitMemCpy(ReturnLoc.Ptr, ResultLV.Ptr, Builder.getInt64(OctetsToCopy),
+                   std::min(ReturnLoc.getAlignment(), ResultLV.getAlignment()));
+
+        if (StructType *STy = dyn_cast<StructType>(Fn->getReturnType())) {
+          llvm::Value *Idxs[2];
+          Idxs[0] = Builder.getInt32(0);
+          for (unsigned ri = 0; ri < STy->getNumElements(); ++ri) {
+            Idxs[1] = Builder.getInt32(ri);
+            Value *GEP = Builder.CreateGEP(ReturnLoc.Ptr, Idxs, "mrv_gep");
+            Value *E = Builder.CreateLoad(GEP, "mrv");
+            RetVals.push_back(E);
+          }
+          // If the return type specifies an empty struct then return one.
+          if (RetVals.empty())
+            RetVals.push_back(UndefValue::get(Fn->getReturnType()));
+        } else {
+          // Otherwise, this aggregate result must be something that is returned
+          // in a scalar register for this target.  We must bit convert the
+          // aggregate to the specified scalar type, which we do by casting the
+          // pointer and loading.
+          RetVals.push_back(Builder.CreateLoad(ReturnLoc.Ptr, "retval"));
         }
-        RetVal = Builder.CreateBitCast(RetVal,
-                                       Fn->getReturnType()->getPointerTo());
-        RetVal = Builder.CreateLoad(RetVal, "retval");
-        RetVals.push_back(RetVal);
       }
     }
   }

Added: dragonegg/trunk/test/validator/c/2012-09-10-MRVOverflow.c
URL: http://llvm.org/viewvc/llvm-project/dragonegg/trunk/test/validator/c/2012-09-10-MRVOverflow.c?rev=163499&view=auto
==============================================================================
--- dragonegg/trunk/test/validator/c/2012-09-10-MRVOverflow.c (added)
+++ dragonegg/trunk/test/validator/c/2012-09-10-MRVOverflow.c Mon Sep 10 05:27:20 2012
@@ -0,0 +1,10 @@
+// RUN: %dragonegg -S %s -o - | FileCheck %s
+// Check that the MRV code doesn't load from beyond the end of an alloca.
+
+struct Three { int a; int b; int c; };
+
+struct Three foo(struct Three *p) {
+  return *p;
+// CHECK: call void @llvm.memcpy
+// CHECK-NOT: bitcast
+}





More information about the llvm-commits mailing list