Fix allocation/initialization bug in messages with varsize arrays
authorPhil Miller <mille121@illinois.edu>
Fri, 7 Jan 2011 20:43:36 +0000 (14:43 -0600)
committerPhil Miller <mille121@illinois.edu>
Fri, 28 Jan 2011 23:19:30 +0000 (17:19 -0600)
Store the offsets computed during message allocation and assign the
pointers to later in the buffer during the constructor of the
message's generated base class. This saves us from cases where the
compiler overwrites the buffer's contents between allocation and
construction.

This patch does not address the cases in which
 - The compiler generates code in the user's constructor that
   overwrites these pointers
 - Alignment is screwed up by the compiler padding the allocation at
   the beginning rather than the end.
We don't currently know of instances in which either of these errors
occurs.

src/ck-core/charm++.h
src/ck-core/init.C
src/ck-core/msgalloc.C
src/xlat-i/xi-symbol.C
tests/charm++/megatest/Make.depends
tests/charm++/megatest/Makefile
tests/charm++/megatest/varsizetest2.C [new file with mode: 0644]
tests/charm++/megatest/varsizetest2.ci [new file with mode: 0644]
tests/charm++/megatest/varsizetest2.h [new file with mode: 0644]

index 071cf5ccf5d2754af6eb83bbe01abada308eaf58..00fcb50677b9ddb22d6a1c47b386eed9adfa8747 100644 (file)
@@ -62,6 +62,8 @@ public:
        static int __idx;
 };
 
+CkpvExtern(size_t *, _offsets);
+
 /// CkArgMsg is passed to the mainchare's constructor.
 class CkArgMsg : public CkMessage {
 public:
index d9503f9260080376d6fe4edc9b925fbc526e2d73..ba3d4bffd5e91a1314c386e3081cc0da794b8952 100644 (file)
@@ -929,6 +929,8 @@ void _initCharm(int unused_argc, char **argv)
 
        DEBUGF(("[%d,%.6lf ] _initCharm started\n",CmiMyPe(),CmiWallTimer()));
 
+       CkpvInitialize(size_t *, _offsets);
+       CkpvAccess(_offsets) = new size_t[32];
        CkpvInitialize(PtrQ*,_buffQ);
        CkpvInitialize(PtrVec*,_bocInitVec);
        CkpvInitialize(void*, _currentChare);
index c56848da3d4f96c84ff728c51aa76536296d767f..eca203b58236d500a84da0d86c3b787771e9f817 100644 (file)
@@ -8,6 +8,8 @@
 #include "ck.h"
 #include "queueing.h"
 
+CkpvDeclare(size_t *, _offsets);
+
 extern "C"
 void *CkAllocSysMsg(void)
 {
index bf57075373ab12b374d3db3191a9ccc402ec1e8f..76ea5ea0547adf3006f56ff106d9befd2ff2c55f 100644 (file)
@@ -1903,7 +1903,7 @@ Message::genAllocDecl(XStr &str)
   mtype << type;
   if(templat) templat->genVars(mtype);
   str << CIMsgClassAnsi;
-  str << "    CMessage_" << mtype << "() {};\n";
+  str << "    CMessage_" << mtype << "();\n";
   str << "    static void *pack(" << mtype << " *p);\n";
   str << "    static " << mtype << "* unpack(void* p);\n";
   num = numArrays();
@@ -2042,30 +2042,33 @@ Message::genDefs(XStr& str)
     // alloc(int, size_t, int*, priobits)
     str << tspec << "void* " << ptype;
     str << "::alloc(int msgnum, size_t sz, int *sizes, int pb) {\n";
-    str << "  size_t offsets[" << numArray+1 << "];\n";
-    str << "  offsets[0] = ALIGN8(sz);\n";
+    str << "  CkpvAccess(_offsets)[0] = ALIGN8(sz);\n";
     for(i=0, count=0, ml=mvlist; i<num; i++, ml=ml->next) {
       mv = ml->msg_var;
       if (mv->isArray()) {
         str << "  if(sizes==0)\n";
-        str << "    offsets[" << count+1 << "] = offsets[0];\n";
+        str << "    CkpvAccess(_offsets)[" << count+1 << "] = CkpvAccess(_offsets)[0];\n";
         str << "  else\n";
-        str << "    offsets[" << count+1 << "] = offsets[" << count << "] + ";
+        str << "    CkpvAccess(_offsets)[" << count+1 << "] = CkpvAccess(_offsets)[" << count << "] + ";
         str << "ALIGN8(sizeof(" << mv->type << ")*sizes[" << count << "]);\n";
         count ++;
       }
     }
-    str << "  " << mtype << " *newmsg = (" << mtype << " *) ";
-    str << "CkAllocMsg(msgnum, offsets[" << numArray << "], pb);\n";
+    str << "  return CkAllocMsg(msgnum, CkpvAccess(_offsets)[" << numArray << "], pb);\n";
+    str << "}\n";
+
+    str << tspec << ptype << "::" << proxyPrefix() << type << "() {\n";
+    str << mtype << " *newmsg = reinterpret_cast<" << mtype << " *>(this);\n";
     for(i=0, count=0, ml=mvlist; i<num; i++,ml=ml->next) {
       mv = ml->msg_var;
       if (mv->isArray()) {
         str << "  newmsg->" << mv->name << " = (" << mv->type << " *) ";
-        str << "((char *)newmsg + offsets[" << count << "]);\n";
+        str << "((char *)newmsg + CkpvAccess(_offsets)[" << count << "]);\n";
         count ++;
       }
     }
-    str << "  return (void *) newmsg;\n}\n";
+    str << "}\n";
+
     int numCond = numConditional();
     str << tspec << "void " << ptype << "::dealloc(void *p) {\n";
     if (numCond > 0) {
index e080a1840253d6b78250285d5388b9149de3ab40..bf4d85dd1ef1bd7ebd284d97d4ce48284e2f0ff7 100644 (file)
@@ -26,6 +26,13 @@ varsizetest.o: \
        varsizetest.decl.h \
        varsizetest.def.h
        $(CHARMC) -o varsizetest.o varsizetest.C
+varsizetest2.o: \
+       varsizetest2.C \
+       varsizetest2.h \
+       megatest.h \
+       varsizetest2.decl.h \
+       varsizetest2.def.h
+       $(CHARMC) -o varsizetest2.o varsizetest2.C
 varraystest.o: \
        varraystest.C \
        varraystest.h \
index ba7037dcec061f5bb0c2072841a58041616d56a9..c5eb6ae295264f3a30246184244eca2f8d75a5c4 100644 (file)
@@ -6,6 +6,7 @@ OBJS=megatest.o \
      groupring.o \
      nodering.o \
      varsizetest.o \
+     varsizetest2.o \
      varraystest.o \
      groupcast.o \
      groupmulti.o \
@@ -75,7 +76,7 @@ CIFILES =  \
       fib.def.h        megatest.def.h       priotest.def.h   tempotest.def.h  \
       groupcast.def.h  migration.def.h      queens.def.h     varraystest.def.h\
       groupring.def.h  nodecast.def.h       reduction.def.h  varsizetest.def.h\
-      groupsectiontest.def.h multisectiontest.def.h inlineem.def.h \
+      groupsectiontest.def.h multisectiontest.def.h inlineem.def.h varsizetest2.def.h\
       groupmulti.def.h #priolongtest.def.h
 
 .SUFFIXES:
diff --git a/tests/charm++/megatest/varsizetest2.C b/tests/charm++/megatest/varsizetest2.C
new file mode 100644 (file)
index 0000000..b4e3450
--- /dev/null
@@ -0,0 +1,65 @@
+#include "varsizetest2.h"
+
+static int nextseq = 0;
+
+void varsizetest2_init(void) 
+{
+    CProxy_varsizetest2_main::ckNew(0);
+}
+
+void varsizetest2_moduleinit(void) {}
+
+varsizetest2_main::varsizetest2_main(void)
+{
+  int sizes[2];
+  sizes[0] = 100; sizes[1] = 300;
+  varsizetest2_Msg *m = new (sizes,0) varsizetest2_Msg();
+  CkAssert(m->iarray && m->farray);
+  m->myMain = thishandle;
+  CProxy_varsizetest2_test::ckNew(m);
+}
+
+void varsizetest2_main::exit(varsizetest2_Msg *m)
+{
+  if(!m->check())
+    CkAbort("varsizetest2 failed!\n");
+  delete m;
+  delete this;
+  megatest_finish();
+}
+
+int varsizetest2_Msg::check(void)
+{
+  int i;
+  for(i=0; i<10; i++)
+    if(iarray[i] != 2*i*i*seqnum) {
+      return 1;
+    }
+  for(i=0; i<10; i++)
+    if(farray[i] != i*i*seqnum) {
+      return 1;
+    }
+  return 1;
+}
+
+varsizetest2_test::varsizetest2_test(varsizetest2_Msg *m)
+{
+  CkChareID mhandle = m->myMain;
+  int currentSeqnum = m->seqnum;
+  if(!m->check()) {
+    CkAbort("varsizetest2 failed!\n");
+    megatest_finish();
+    return;
+  }
+  delete m;
+  int sizes[2];
+  sizes[0] = 300;
+  sizes[1] = 100;
+  m = new (sizes,0) varsizetest2_Msg();
+  CProxy_varsizetest2_main mainproxy(mhandle);
+  mainproxy.exit(m); 
+  delete this;
+}
+
+MEGATEST_REGISTER_TEST(varsizetest2,"phil",1)
+#include "varsizetest2.def.h"
diff --git a/tests/charm++/megatest/varsizetest2.ci b/tests/charm++/megatest/varsizetest2.ci
new file mode 100644 (file)
index 0000000..91eda65
--- /dev/null
@@ -0,0 +1,15 @@
+module varsizetest2 {
+  message varsizetest2_Msg {
+    int iarray[];
+    float farray[];
+  };
+
+  chare varsizetest2_main {
+    entry varsizetest2_main(void);
+    entry void exit(varsizetest2_Msg *);
+  };
+
+  chare varsizetest2_test {
+    entry varsizetest2_test(varsizetest2_Msg *);
+  };
+};
diff --git a/tests/charm++/megatest/varsizetest2.h b/tests/charm++/megatest/varsizetest2.h
new file mode 100644 (file)
index 0000000..87f9ef8
--- /dev/null
@@ -0,0 +1,36 @@
+#ifndef _VARSIZETEST2_H
+#define _VARSIZETEST2_H
+
+#include "megatest.h"
+#include "varsizetest2.decl.h"
+
+
+class varsizetest2_Msg : public CMessage_varsizetest2_Msg {
+ public:
+  
+#if NOCRASH
+    varsizetest2_Msg() {}
+#endif
+  int check(void);
+  int isize;
+  int fsize;
+  int *iarray;
+  float *farray;
+  int seqnum;
+  CkChareID myMain;
+}; 
+
+class varsizetest2_main : public Chare {
+ public:
+  varsizetest2_main(void);
+  varsizetest2_main(CkMigrateMessage *m) {}
+  void exit(varsizetest2_Msg *m);
+};
+
+class varsizetest2_test : public Chare {
+ public:
+  varsizetest2_test(varsizetest2_Msg *m);
+  varsizetest2_test(CkMigrateMessage *m) {}
+};
+
+#endif