[0.2.x] fixed: issue #12591, issue #12592. ~typeBuilder: alignment-requirement implemented

git-svn-id: https://pykd.svn.codeplex.com/svn@86871 9b283d60-5439-405e-af05-b73fd8c4d996

pykd/customtypes.cpp

@@ -7,7 +7,6 @@
 
 namespace pykd {
 
-
 ////////////////////////////////////////////////////////////////////////////////
 
 TypeInfoPtr TypeBuilder::createStruct(  const std::string &name, ULONG align )
@@ -24,19 +23,30 @@ TypeInfoPtr TypeBuilder::createUnion( const std::string &name )
 
 ////////////////////////////////////////////////////////////////////////////////
 
+ULONG CustomBase::getSize()
+{
+    return alignUp( m_size, getAlignReq() );
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+ULONG CustomBase::alignUp(ULONG val, ULONG align)
+{
+    return val + (val % align ? align - (val % align) : 0);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
 void CustomBase::throwIfFiledExist(const std::string &fieldName)
 {
-    bool fieldExist = false;
     try
     {
         lookupField(fieldName);
-        fieldExist = true;
     }
     catch (const TypeException&)
     {
+        return;
     }
-
-    if (fieldExist)
     throw TypeException(getName(), "duplicate field name: " + fieldName);
 }
 
@@ -50,46 +60,35 @@ void CustomBase::throwIfTypeRecursive(TypeInfoPtr type)
     if ( !type->isUserDefined() )
         return;
 
-    try
-    {
     const ULONG fields = type->getFieldCount();
     for (ULONG i = 0; i < fields; ++i)
-        {
+        throwIfTypeRecursive(type->getFieldByIndex(i));
-            TypeInfoPtr fileldType = type->getFieldByIndex(i);
-            if (fileldType->is(this))
-                throw TypeException(getName(), "wrong type of field");
-
-            throwIfTypeRecursive(fileldType);
-        }
-    }
-    catch (const TypeException &except)
-    {
-        DBG_UNREFERENCED_PARAMETER(except);
-    }
 }
 
-////////////////////////////////////////////////////////////////////////////////
+void CustomBase::checkAppendField(const std::string &fieldName, TypeInfoPtr &fieldType)
-
-void CustomStruct::appendField(const std::string &fieldName, TypeInfoPtr &fieldType )
 {
     if ( !fieldType )
          throw DbgException( "typeInfo can not be None" );
 
     throwIfFiledExist(fieldName);
     throwIfTypeRecursive(fieldType);
+}
+
+////////////////////////////////////////////////////////////////////////////////
+
+void CustomStruct::appendField(const std::string &fieldName, TypeInfoPtr &fieldType)
+{
+    checkAppendField(fieldName, fieldType);
 
     CustomUdtField  *field = new CustomUdtField( fieldType, fieldName );
 
-    ULONG fieldSize = fieldType->getSize();
+    const ULONG fieldAlignReq = fieldType->getAlignReq();
-    ULONG offset = m_size;
+    const ULONG align = fieldAlignReq < m_align ? fieldAlignReq : m_align;
-    ULONG align = fieldSize < m_align ? fieldSize : m_align;
+    const ULONG fieldOffset = alignUp(m_size, align);
 
-    if (align)
+    field->setOffset( fieldOffset );
-        offset += offset % align > 0 ? align - offset % align : 0;
 
-    field->setOffset( offset );
+    m_size = fieldOffset + fieldType->getSize();
-
-    m_size = offset + fieldSize;
 
     m_fields.push_back( UdtFieldPtr( field ) );
 }
@@ -98,8 +97,7 @@ void CustomStruct::appendField(const std::string &fieldName, TypeInfoPtr &fieldT
 
 void CustomUnion::appendField(const std::string &fieldName, TypeInfoPtr &fieldType)
 {
-    throwIfFiledExist(fieldName);
+    checkAppendField(fieldName, fieldType);
-    throwIfTypeRecursive(fieldType);
 
     CustomUdtField  *field = new CustomUdtField( fieldType, fieldName );
 
@@ -112,153 +110,6 @@ void CustomUnion::appendField(const std::string &fieldName, TypeInfoPtr &fieldTy
 
 ////////////////////////////////////////////////////////////////////////////////
 
-
-
-
-
-
-
-
-
-
-//////////////////////////////////////////////////////////////////////////////////
-//
-//CustomTypeBase::CustomTypeBase(const std::string &name, ULONG pointerSize) 
-//    : UdtFieldColl(name)
-//{
-//    m_ptrSize = pointerSize ? pointerSize : ptrSize();
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//void CustomTypeBase::throwIfFiledExist(const std::string &fieldName)
-//{
-//    bool fieldExist = false;
-//    try
-//    {
-//        lookupField(fieldName);
-//        fieldExist = true;
-//    }
-//    catch (const TypeException &except)
-//    {
-//        DBG_UNREFERENCED_PARAMETER(except);
-//    }
-//    if (fieldExist)
-//        throw TypeException(getName(), "duplicate field name: " + fieldName);
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//void CustomTypeBase::throwIfTypeRecursive(TypeInfoPtr type)
-//{
-//    if (type->is(this))
-//        throw TypeException(getName(), "wrong type of field");
-//
-//    return throwIfTypeRecursiveImpl(type);
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//void CustomTypeBase::throwIfTypeRecursiveImpl(TypeInfoPtr type)
-//{
-//    if (type->isEnum())
-//        return;
-//
-//    try
-//    {
-//        const ULONG fields = type->getFieldCount();
-//        for (ULONG i = 0; i < fields; ++i)
-//        {
-//            TypeInfoPtr fileldType = type->getFieldByIndex(i);
-//            if (fileldType->is(this))
-//                throw TypeException(getName(), "wrong type of field");
-//
-//            throwIfTypeRecursiveImpl(fileldType);
-//        }
-//    }
-//    catch (const TypeException &except)
-//    {
-//        DBG_UNREFERENCED_PARAMETER(except);
-//    }
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//TypeInfoPtr CustomStruct::create(
-//    const std::string &name,
-//    ULONG alignReq /*= 0*/,
-//    ULONG pointerSize /*= 0*/)
-//{
-//    return TypeInfoPtr( new CustomStruct(name, alignReq, pointerSize) );
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//ULONG CustomStruct::getSize()
-//{
-//    if (Base::empty())
-//        return 0;
-//
-//    UdtUtils::Field &field = Base::last();
-//    return field.m_offset + field.m_type->getSize();
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//void CustomStruct::appendField(const std::string &fieldName, TypeInfoPtr fieldType)
-//{
-//    Base::throwIfFiledExist(fieldName);
-//    Base::throwIfTypeRecursive(fieldType);
-//
-//    ULONG offset = getSize();
-//    offset += offset % (m_align ? m_align : fieldType->getAlignReq());
-//    Base::push_back(
-//        UdtUtils::Field(offset, fieldName, fieldType)
-//    );
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//TypeInfoPtr CustomUnion::create(const std::string &name, ULONG pointerSize /*= 0*/)
-//{
-//    return TypeInfoPtr( new CustomUnion(name, pointerSize) );
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//ULONG CustomUnion::getSize()
-//{
-//    ULONG size = 0;
-//    for (ULONG i = 0; i < getFieldCount(); ++i)
-//    {
-//        ULONG fieldSize = lookupField(i).m_type->getSize();
-//        if (fieldSize > size)
-//            size = fieldSize;
-//    }
-//    return size;
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//void CustomUnion::appendField(const std::string &fieldName, TypeInfoPtr fieldType)
-//{
-//    Base::throwIfFiledExist(fieldName);
-//    Base::throwIfTypeRecursive(fieldType);
-//
-//    Base::push_back(
-//        UdtUtils::Field(0, fieldName, fieldType)
-//    );
-//}
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//TypeInfoPtr PtrToVoid()
-//{
-//    return TypeInfoPtr( new PointerTypeInfo(ptrSize()) );
-//}
-
-////////////////////////////////////////////////////////////////////////////////
-
 }   // namespace pykd
 
 ////////////////////////////////////////////////////////////////////////////////

pykd/customtypes.h

@@ -53,6 +53,9 @@ private:
 
 class CustomBase : public UdtTypeInfoBase 
 {
+public:
+
+    virtual ULONG getSize();
 
 protected:
 
@@ -64,14 +67,12 @@ protected:
             m_size = 0;
         }
 
+    static ULONG alignUp(ULONG val, ULONG align);
+
     void throwIfFiledExist(const std::string &fieldName);
     void throwIfTypeRecursive(TypeInfoPtr type);
 
-protected:
+    void checkAppendField(const std::string &fieldName, TypeInfoPtr &fieldType);
-
-    virtual ULONG getSize() {
-        return m_size;
-    }
 
 protected:
 
@@ -90,10 +91,7 @@ public:
         CustomBase( name, ptrSize, align )
         {}
 
-private:
-
     virtual void appendField(const std::string &fieldName, TypeInfoPtr &fieldType);
-
 };
 
 ////////////////////////////////////////////////////////////////////////////////
@@ -106,110 +104,9 @@ public:
         CustomBase( name, ptrSize, align )
         {}
 
-private:
-
     virtual  void appendField(const std::string &fieldName, TypeInfoPtr &fieldType);
-
 };
 
-////////////////////////////////////////////////////////////////////////////////
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//class CustomTypeBase : public UdtFieldColl
-//{
-//    typedef UdtFieldColl Base;
-//protected:
-//    CustomTypeBase(const std::string &name, ULONG pointerSize);
-//
-//    void throwIfFiledExist(const std::string &fieldName);
-//    void throwIfTypeRecursive(TypeInfoPtr type);
-//
-//private:
-//    void throwIfTypeRecursiveImpl(TypeInfoPtr type);
-//};
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//class CustomStruct : public CustomTypeBase
-//{
-//    typedef CustomTypeBase Base;
-//public:
-//    static TypeInfoPtr create(const std::string &name, ULONG align = 0, ULONG pointerSize = 0);
-//
-//protected:
-//    CustomStruct(const std::string &name, ULONG align, ULONG pointerSize)
-//        : Base(name, pointerSize), m_name(name), m_align(align) 
-//    {
-//    }
-//
-//    virtual std::string getName() override {
-//        return m_name;
-//    }
-//
-//    virtual ULONG getSize() override;
-//
-//    virtual void appendField(const std::string &fieldName, TypeInfoPtr fieldType) override;
-//
-//    virtual std::string getTypeString() const override {
-//        return "custom struct";
-//    }
-//
-//private:
-//    std::string m_name;
-//    ULONG m_align;
-//};
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//class CustomUnion : public CustomTypeBase
-//{
-//    typedef CustomTypeBase Base;
-//public:
-//    static TypeInfoPtr create(const std::string &name, ULONG pointerSize = 0);
-//
-//protected:
-//    CustomUnion(const std::string &name, ULONG pointerSize) 
-//        : Base(name, pointerSize)
-//    {
-//    }
-//
-//    virtual ULONG getSize() override;
-//
-//    virtual void appendField(const std::string &fieldName, TypeInfoPtr fieldType) override;
-//
-//    virtual std::string getTypeString() const override {
-//        return "custom union";
-//    }
-//
-//private:
-//    std::string m_name;
-//};
-//
-//////////////////////////////////////////////////////////////////////////////////
-//
-//TypeInfoPtr PtrToVoid();
-//
 //////////////////////////////////////////////////////////////////////////////////
 
 }   // namespace pykd

pykd/typeinfo.cpp

@@ -721,20 +721,6 @@ std::string UdtTypeInfoBase::print()
 
 /////////////////////////////////////////////////////////////////////////////////////
 
-ULONG UdtTypeInfoBase::getAlignReq()
-{
-    //ULONG alignReq = 1;
-    //const ULONG fieldCount = getFieldCount();
-    //for ( ULONG i = 0; i < fieldCount; ++i )
-    //    alignReq = max(alignReq, lookupField(i).m_type->getAlignReq());
-
-    //return alignReq;
-
-    throw ImplementException( __FILE__, __LINE__, "TODO" );
-}
-
-/////////////////////////////////////////////////////////////////////////////////////
-
 void UdtTypeInfoBase::getVirtualDisplacement( const std::string& fieldName, ULONG &virtualBasePtr, ULONG &virtualDispIndex, ULONG &virtualDispSize )
 {
     const UdtFieldPtr &field = lookupField(fieldName);
@@ -759,6 +745,19 @@ void  UdtTypeInfoBase::getVirtualDisplacementByIndex( ULONG index, ULONG &virtua
 
 /////////////////////////////////////////////////////////////////////////////////////
 
+ULONG UdtTypeInfoBase::getAlignReq()
+{
+    ULONG alignReq = 1;
+    for (ULONG i = 0; i < getFieldCount(); ++i)
+    {
+        const ULONG fieldAlignReq = getFieldByIndex(i)->getAlignReq();
+        alignReq = (fieldAlignReq > alignReq) ? fieldAlignReq : alignReq;
+    }
+    return alignReq;
+}
+
+/////////////////////////////////////////////////////////////////////////////////////
+
 void UdtTypeInfo::getFields( 
         SymbolPtr &rootSym, 
         SymbolPtr &baseVirtualSym,

pykd/typeinfo.h

@@ -215,10 +215,6 @@ public:
 
     TypeInfoPtr arrayOf( ULONG count );
 
-    virtual ULONG getAlignReq() {
-        return 1;
-    }
-
     void setConstant( const BaseTypeVariant& var )
     {
         m_constant = true;
@@ -237,6 +233,10 @@ public:
         return m_ptrSize;
     }
 
+    // http://msdn.microsoft.com/en-us/library/hx1b6kkd.aspx
+    // "Padding and Alignment of Structure Members"
+    virtual ULONG getAlignReq() = 0;
+
 protected:
 
     std::string getComplexName();
@@ -266,7 +266,11 @@ public:
         m_ptrSize = pointerSize;
     }
 
-private:
+    virtual ULONG getAlignReq() {
+        return getSize();
+    }
+
+protected:
 
     virtual std::string getName() {
         return m_name;
@@ -285,6 +289,8 @@ private:
         return t < 0;
     }
 
+private:
+
     std::string     m_name;
 };
 
@@ -321,14 +327,14 @@ public:
         return m_bitWidth;
     }
 
-    virtual ULONG getAlignReq() override {
-        return m_size;
-    }
-
     virtual bool isSigned() {
         return m_signed;
     }
 
+    virtual ULONG getAlignReq() {
+        return getSize();
+    }
+
 private:
 
     bool            m_signed;
@@ -410,8 +416,6 @@ protected:
         return true;
     }
 
-    virtual ULONG getAlignReq() override;
-
     virtual bool isVirtualMember( const std::string& fieldName )
     {
         return lookupField(fieldName)->isVirtualMember();
@@ -425,6 +429,8 @@ protected:
     virtual void getVirtualDisplacement( const std::string& fieldName, ULONG &virtualBasePtr, ULONG &virtualDispIndex, ULONG &virtualDispSize );
     virtual void getVirtualDisplacementByIndex( ULONG index, ULONG &virtualBasePtr, ULONG &virtualDispIndex, ULONG &virtualDispSize );
 
+    virtual ULONG getAlignReq();
+
 protected:
 
     UdtTypeInfoBase(const std::string &typeName) : 
@@ -512,6 +518,10 @@ public:
       m_dia( symbol )
       {}
 
+    virtual ULONG getAlignReq() {
+        return getSize();
+    }
+
 protected:
 
     virtual std::string getName() {
@@ -538,10 +548,6 @@ protected:
 
     virtual std::string print();
 
-    virtual ULONG getAlignReq() override {
-        return getSize();
-    }
-
     virtual ULONG getElementCount() {
         return getFieldCount();
     }
@@ -583,10 +589,6 @@ public:
         return getDerefType();
     }
 
-    virtual ULONG getAlignReq() override {
-        return m_size;
-    }
-
     TypeInfoPtr getDerefType() {
         if (!m_derefType)
             throw TypeException("<ptr>", "this pointer can not be dereferenced");
@@ -603,6 +605,10 @@ public:
         return m_derefName;
     }
 
+    virtual ULONG getAlignReq() {
+        return getSize();
+    }
+
 private:
     static std::string VoidTypeName;
 
@@ -652,14 +658,14 @@ public:
         return m_derefType;
     }
 
-    virtual ULONG getAlignReq() override {
-        return m_derefType->getAlignReq();
-    }
-
     TypeInfoPtr getDerefType() {
         return m_derefType;
     }
 
+    virtual ULONG getAlignReq() {
+        return getDerefType()->getAlignReq();
+    }
+
 private:
 
     TypeInfoPtr     m_derefType;

test/scripts/customtypestest.py

@@ -6,7 +6,6 @@ import pykd
 
 class CustomTypesTest(unittest.TestCase):
     def testCommonStruct(self):
-
         tb = pykd.typeBuilder()
 
         mySubStruct =tb.createStruct("MySubCustomStruct")
@@ -42,10 +41,7 @@ class CustomTypesTest(unittest.TestCase):
         self.assertTrue( myType.fieldOffset("m_struct") < myType.fieldOffset("m_union") )
         self.assertTrue( myType.fieldOffset("m_struct") + myType.m_struct.size() <= myType.fieldOffset("m_union") )
 
-        # print myType
-
     def testCommonUnion(self):
-
         tb = pykd.typeBuilder()
 
         myType = tb.createUnion("MyCustomStruct")
@@ -77,7 +73,6 @@ class CustomTypesTest(unittest.TestCase):
         self.assertEqual( 0, myEmptyUnion1.size() )
 
     def testDupFieldName(self):
-    
         tb = pykd.typeBuilder()
 
         myType = tb.createStruct("MyCustomStruct")
@@ -146,3 +141,19 @@ class CustomTypesTest(unittest.TestCase):
         self.assertEqual( 16, struct.size() )
         struct.append( "m_field7", tb.UInt1B.arrayOf(5) )
         self.assertEqual( 20, struct.size() )
+
+    def testWi12591(self):
+        tb = pykd.typeBuilder()
+        struct = tb.createStruct(name ="MyAlignStruct", align=4)
+        struct.append( "m_field1", tb.UInt1B )
+        struct.append( "m_field2", tb.UInt1B.arrayOf(2) )
+        self.assertEqual( struct.size(), 3 )
+        self.assertEqual( struct.fieldOffset("m_field2"), 1 )
+
+    def testWi12592(self):
+        tb = pykd.typeBuilder()
+        struct = tb.createStruct(name ="MyAlignStruct", align=4)
+        struct.append( "field1", tb.UInt4B )
+        struct.append( "field2", tb.UInt1B )
+        self.assertEqual( struct.size(), 8 )
+        self.assertEqual( struct.fieldOffset("field2"), 4 )