[Xcb-commit] xcb/proto: 5 commits - src xcbgen

Christian Linhart clinhart at kemper.freedesktop.org
Tue Jan 5 17:00:21 PST 2016


 src/glx.xml        |    4 
 src/present.xml    |    4 
 src/xinput.xml     |   10 
 src/xkb.xml        |   25 +-
 xcbgen/Makefile.am |    2 
 xcbgen/align.py    |  177 ++++++++++++++++
 xcbgen/expr.py     |   26 ++
 xcbgen/xtypes.py   |  575 ++++++++++++++++++++++++++++++++++++++++++++++++++---
 8 files changed, 786 insertions(+), 37 deletions(-)

New commits:
commit fe8ae2426d0064972a2f612d5d520879ffbd9278
Author: Christian Linhart <chris at demorecorder.com>
Date:   Sun Nov 1 19:44:29 2015 +0100

    make xkb pass the alignment checker
    
    These are just minimal adjustments to get xkb through
    the checks of the alignment checker.
    
    It is not the big fixup which I have already posted an RFC patch
    a while ago.
    
    V2 of this patch:
      make indentation consistent with the file
      (tabs vs spaces)
    
    Signed-off-by: Christian Linhart <chris at demorecorder.com>

diff --git a/src/xkb.xml b/src/xkb.xml
index ad52ea2..06488e7 100644
--- a/src/xkb.xml
+++ b/src/xkb.xml
@@ -745,6 +745,7 @@ authorization from the authors.
 		<list name="string" type="STRING8">
 			<fieldref>length</fieldref>
 		</list>
+		<pad align="2" />
 	</struct>
 
 	<struct name="DeviceLedInfo">
@@ -1443,6 +1444,7 @@ authorization from the authors.
 				<list name="actionsCount" type="CARD8">
 					<fieldref>nKeyActions</fieldref>
 				</list>
+				<pad align="4" />
 				<list name="actions" type="Action">
 					<fieldref>totalActions</fieldref>
 				</list>
@@ -1458,6 +1460,7 @@ authorization from the authors.
 				<list name="vmods" type="CARD8">
 					<popcount><fieldref>virtualMods</fieldref></popcount>
 				</list>
+				<pad align="4" />
 			</bitcase>
 			<bitcase>
 				<enumref ref="MapPart">ExplicitComponents</enumref>
@@ -1667,20 +1670,7 @@ authorization from the authors.
 					       <fieldref>nKTLevels</fieldref> -->
 					        <fieldref>nTypes</fieldref>
 					</list>
-					<list type="CARD8" name="alignment_pad">
-					    <op op="-">
-						<op op="&">
-						    <op op="+">
-							<fieldref>nTypes</fieldref>
-							<value>3</value>
-						    </op>
-						    <unop op="~">
-							<value>3</value>
-						    </unop>
-						</op>
-						<fieldref>nTypes</fieldref>
-					    </op>
-					</list>
+					<pad align="4" />
 					<list name="ktLevelNames" type="ATOM">
 						<sumof ref="nLevelsPerType" />
 					</list>
@@ -1784,6 +1774,7 @@ authorization from the authors.
 				<list name="nLevelsPerType" type="CARD8">
 					<fieldref>nTypes</fieldref>
 				</list>
+				<pad align="4"/>
 				<list name="ktLevelNames" type="ATOM">
 					<sumof ref="nLevelsPerType" />
 				</list>
@@ -2092,6 +2083,7 @@ authorization from the authors.
 							<list name="acts_rtrn_count" type="CARD8">
 								<fieldref>nKeyActions</fieldref>
 							</list>
+							<pad align="4" />
 							<list name="acts_rtrn_acts" type="Action">
 								<fieldref>totalActions</fieldref>
 							</list>
@@ -2107,18 +2099,21 @@ authorization from the authors.
 							<list name="vmods_rtrn" type="CARD8" mask="ModMask">
 								<popcount><fieldref>virtualMods</fieldref></popcount>
 							</list>
+							<pad align="4" />
 						</bitcase>
 						<bitcase>
 							<enumref ref="MapPart">ExplicitComponents</enumref>
 							<list name="explicit_rtrn" type="SetExplicit">
 								<fieldref>totalKeyExplicit</fieldref>
 							</list>
+							<pad align="4" />
 						</bitcase>
 						<bitcase>
 							<enumref ref="MapPart">ModifierMap</enumref>
 							<list name="modmap_rtrn" type="KeyModMap">
 								<fieldref>totalModMapKeys</fieldref>
 							</list>
+							<pad align="4" />
 						</bitcase>
 						<bitcase>
 							<enumref ref="MapPart">VirtualModMap</enumref>
@@ -2226,6 +2221,7 @@ authorization from the authors.
 							<list name="nLevelsPerType" type="CARD8">
 								<fieldref>nTypes</fieldref>
 							</list>
+							<pad align="4" />
 							<list name="ktLevelNames" type="ATOM">
 								<sumof ref="nLevelsPerType" />
 							</list>
@@ -2352,6 +2348,7 @@ authorization from the authors.
 			<list name="name" type="STRING8">
 				<fieldref>nameLen</fieldref>
 			</list>
+			<pad align="4" />
 			<list name="btnActions" type="Action">
 				<fieldref>nBtnsRtrn</fieldref>
 			</list>
commit c499401bdac3b87bd4f9cd4bc64cfd1781ab447f
Author: Christian Linhart <chris at demorecorder.com>
Date:   Sun Nov 1 18:26:33 2015 +0100

    automatic alignment checker / manual offsets
    
    The verification algorithm basically traverses the protocol-description
    of a protocol entity by recursively processing all fields and their
    types.
    
    A start-align consists of two numbers:
    * the alignment: This is a power of 2, and guarantees that the
      address (or protocol position) minus the offset can be divided
      by this number.
    
    * the offset: how many bytes the current position is after a
      position that can be divided by the alignment.
    
    The algorithm starts with the start-alignment and computes the alignment
    of each field from the start-align of the previous field as follows:
    * if the previous field is a primitive type then the offset is increased
      by the size of the primitive type module the alignment.
      If the alignment or offset are incompatible with the primitive type,
      then an error is reported.
    * if the previous field is a complex type, then it is processed recursively.
    * if the previous field is an alignment-pad, then the alignment is
      adjusted accordingly, as to be expected by the alignment-pad.
    
    Start-aligns can also be set manually with the xml-element
    "required_start_align" which has two attributes: "align" and "offset"
    If "offset" is omitted, it is assumed to 0.
    
    All toplevel protocol entities default to 4-byte start-alignment with offset 0.
    
    All non-toplevel complex entities, such as structs, switch, case, ...
    do not have a default alignment.
    If no alignment is explicitly specified for them, their alignment
    is computed by their usage in other entities.
    In that case, if they are used with aligments that violate the
    alignment requirements of some of their fields, an error is issued.
    
    If they are used with an alignment with non-zero offset,
    a warning is issued, which recommends to specify the required_start_align
    explicitly. (Reason: I don't want non-zero offsets to be silently
    computed automatically. These non-zero offsets have to be reviewed
    by a human and specified explicitely to record that this was reviewed
    by a human)
    
    If the required_start_align is explicitly specified for an entity
    then an error will be issued if it is used with an aligment that's
    incompatible with the explicitly specified alignment.
    
    If an entity is used in different contexts with different start-aligns
    then those start-aligns are combined to an align which is compatible
    with all aligns.
    E.g. (align 4, offset 0) and (align 4, offset 2) are combined
    to (align 2, offset 0).
    
    Error reports include the relevant context for a misalignment.
    For non-toplevel entities this includes the entity-usage stack.
    There is some complexity in the algorithm for reducing the size
    of the error-reports to include only the relevant info.
    
    This alignment verifier is also a prerequisite for getting
    rid of implicit alignment altogether.
    (This will then simplify the generated code and make it faster.)
    
    Signed-off-by: Christian Linhart <chris at demorecorder.com>

diff --git a/xcbgen/Makefile.am b/xcbgen/Makefile.am
index 110a992..d6c7adc 100644
--- a/xcbgen/Makefile.am
+++ b/xcbgen/Makefile.am
@@ -1,3 +1,3 @@
 pkgpythondir = $(pythondir)/xcbgen
 
-pkgpython_PYTHON = __init__.py error.py expr.py matcher.py state.py xtypes.py
+pkgpython_PYTHON = __init__.py error.py expr.py align.py matcher.py state.py xtypes.py
diff --git a/xcbgen/align.py b/xcbgen/align.py
new file mode 100644
index 0000000..5e31838
--- /dev/null
+++ b/xcbgen/align.py
@@ -0,0 +1,177 @@
+'''
+This module contains helper classes for alignment arithmetic and checks
+'''
+
+from fractions import gcd
+
+class Alignment(object):
+
+    def __init__(self, align=4, offset=0):
+        self.align = align
+        # normalize the offset (just in case)
+        self.offset = offset % align
+
+
+    def __eq__(self, other):
+        return self.align == other.align and self.offset == other.offset
+
+    def __str__(self):
+	return "(align=%d, offset=%d)" % (self.align, self.offset)
+
+    @staticmethod
+    def for_primitive_type(size):
+	# compute the required start_alignment based on the size of the type
+	if size % 8 == 0:
+            # do 8-byte primitives require 8-byte alignment in X11?
+            return Alignment(8,0)
+        elif size % 4 == 0:
+            return Alignment(4,0)
+        elif size % 2 == 0:
+            return Alignment(2,0)
+        else:
+            return Alignment(1,0)
+
+
+    def align_after_fixed_size(self, size):
+	new_offset = (self.offset + size) % self.align
+        return Alignment(self.align, new_offset)
+
+
+    def is_guaranteed_at(self, external_align):
+        '''
+        Assuming the given external_align, checks whether
+        self is fulfilled for all cases.
+	Returns True if yes, False otherwise.
+        '''
+        if self.align == 1 and self.offset == 0:
+            # alignment 1 with offset 0 is always fulfilled
+            return True
+
+        if external_align is None:
+            # there is no external align -> fail
+            return False
+
+        if external_align.align < self.align:
+            # the external align guarantees less alignment -> not guaranteed
+            return False
+
+	if external_align.align % self.align != 0:
+            # the external align cannot be divided by our align
+	    # -> not guaranteed
+            # (this can only happen if there are alignments that are not
+            # a power of 2, which is highly discouraged. But better be
+            # safe and check for it)
+            return False
+
+        if external_align.offset % self.align != self.offset:
+            # offsets do not match
+            return False
+
+        return True
+
+
+    def combine_with(self, other):
+        # returns the alignment that is guaranteed when
+	# both, self or other, can happen
+        new_align = gcd(self.align, other.align)
+        new_offset_candidate1 = self.offset % new_align
+        new_offset_candidate2 = other.offset % new_align
+        if new_offset_candidate1 == new_offset_candidate2:
+            new_offset = new_offset_candidate1
+        else:
+            offset_diff = abs(new_offset_candidate2 - new_offset_candidate1)
+            new_align = gcd(new_align, offset_diff)
+            new_offset_candidate1 = self.offset % new_align
+            new_offset_candidate2 = other.offset % new_align
+	    assert new_offset_candidate1 == new_offset_candidate2
+	    new_offset = new_offset_candidate1
+        # return the result
+        return Alignment(new_align, new_offset)
+
+
+class AlignmentLog(object):
+
+    def __init__(self):
+	self.ok_list = []
+	self.fail_list = []
+	self.verbosity = 1
+
+    def __str__(self):
+	result = ""
+
+	# output the OK-list
+	for (align_before, field_name, type_obj, callstack, align_after) in self.ok_list:
+	    stacksize = len(callstack)
+            indent = '  ' * stacksize
+	    if self.ok_callstack_is_relevant(callstack):
+                if field_name is None or field_name == "":
+	            result += ("    %sok: %s:\n\t%sbefore: %s, after: %s\n"
+		        % (indent, str(type_obj), indent, str(align_before), str(align_after)))
+	        else:
+		    result += ("    %sok: field \"%s\" in %s:\n\t%sbefore: %s, after: %s\n"
+		        % (indent, str(field_name), str(type_obj),
+		           indent, str(align_before), str(align_after)))
+                if self.verbosity >= 1:
+		    result += self.callstack_to_str(indent, callstack)
+
+	# output the fail-list
+	for (align_before, field_name, type_obj, callstack, reason) in self.fail_list:
+	    stacksize = len(callstack)
+            indent = '  ' * stacksize
+	    if field_name is None or field_name == "":
+	        result += ("    %sfail: align %s is incompatible with\n\t%s%s\n\t%sReason: %s\n"
+		    % (indent, str(align_before), indent, str(type_obj), indent, reason))
+	    else:
+		result += ("    %sfail: align %s is incompatible with\n\t%sfield \"%s\" in %s\n\t%sReason: %s\n"
+		    % (indent, str(align_before), indent, str(field_name), str(type_obj), indent, reason))
+
+            if self.verbosity >= 1:
+	        result += self.callstack_to_str(indent, callstack)
+
+
+	return result
+
+
+    def callstack_to_str(self, indent, callstack):
+        result = "\t%scallstack: [\n" % indent
+        for stack_elem in callstack:
+            result += "\t  %s%s\n" % (indent, str(stack_elem))
+        result += "\t%s]\n" % indent
+	return result
+
+
+    def ok_callstack_is_relevant(self, ok_callstack):
+        # determine whether an ok callstack is relevant for logging
+	if self.verbosity >= 2:
+	    return True
+
+        # empty callstacks are always relevant
+	if len(ok_callstack) == 0:
+            return True
+
+	# check whether the ok_callstack is a subset or equal to a fail_callstack
+        for (align_before, field_name, type_obj, fail_callstack, reason) in self.fail_list:
+            if len(ok_callstack) <= len(fail_callstack):
+                zipped = zip(ok_callstack, fail_callstack[:len(ok_callstack)])
+		is_subset = all([i == j for i, j in zipped])
+		if is_subset:
+                    return True
+
+        return False
+
+
+    def ok(self, align_before, field_name, type_obj, callstack, align_after):
+	self.ok_list.append((align_before, field_name, type_obj, callstack, align_after))
+
+    def fail(self, align_before, field_name, type_obj, callstack, reason):
+	self.fail_list.append((align_before, field_name, type_obj, callstack, reason))
+
+    def append(self, other):
+	self.ok_list.extend(other.ok_list)
+	self.fail_list.extend(other.fail_list)
+
+    def ok_count(self):
+	return len(self.ok_list)
+
+
+
diff --git a/xcbgen/expr.py b/xcbgen/expr.py
index e4ee8c6..a716d34 100644
--- a/xcbgen/expr.py
+++ b/xcbgen/expr.py
@@ -24,6 +24,17 @@ class Field(object):
         self.isfd = isfd
         self.parent = None
 
+    def __str__(self):
+        field_string = "Field"
+        if self.field_name is None:
+            if self.field_type is not None:
+                field_string += " with type " + str(self.type)
+        else:
+            field_string += " \"" + self.field_name + "\""
+        if self.parent is not None:
+            field_string += " in " + str(self.parent)
+
+        return field_string
 
 class Expression(object):
     '''
@@ -122,6 +133,21 @@ class Expression(object):
     def fixed_size(self):
         return self.nmemb != None
 
+    def get_value(self):
+        return self.nmemb
+
+    # if the value of the expression is a guaranteed multiple of a number
+    # return this number, else return 1 (which is trivially guaranteed for integers)
+    def get_multiple(self):
+        multiple = 1
+        if self.op == '*':
+            if self.lhs.fixed_size():
+                multiple *= self.lhs.get_value()
+            if self.rhs.fixed_size():
+                multiple *= self.rhs.get_value()
+
+        return multiple
+
     def recursive_resolve_tasks(self, module, parents):
         for subexpr in (self.lhs, self.rhs):
             if subexpr != None:
diff --git a/xcbgen/xtypes.py b/xcbgen/xtypes.py
index 4d6bbc0..f5302be 100644
--- a/xcbgen/xtypes.py
+++ b/xcbgen/xtypes.py
@@ -2,8 +2,11 @@
 This module contains the classes which represent XCB data types.
 '''
 from xcbgen.expr import Field, Expression
+from xcbgen.align import Alignment, AlignmentLog
 import __main__
 
+verbose_align_log = False
+
 class Type(object):
     '''
     Abstract base class for all XCB data types.
@@ -36,6 +39,10 @@ class Type(object):
         self.is_case_or_bitcase = False
         self.is_bitcase = False
         self.is_case = False
+        self.required_start_align = Alignment()
+
+        # the biggest align value of an align-pad contained in this type
+        self.max_align_pad = 1
 
     def resolve(self, module):
         '''
@@ -91,7 +98,91 @@ class Type(object):
 
         complex_type.fields.append(new_fd)
 
-class SimpleType(Type):
+
+    def get_total_size(self):
+        '''
+        get the total size of this type if it is fixed-size, otherwise None
+        '''
+        if self.fixed_size():
+            if self.nmemb is None:
+                return self.size
+            else:
+                return self.size * self.nmemb
+        else:
+            return None
+
+    def get_align_offset(self):
+        if self.required_start_align is None:
+            return 0
+        else:
+            return self.required_start_align.offset
+
+    def is_acceptable_start_align(self, start_align, callstack, log):
+        return self.get_alignment_after(start_align, callstack, log) is not None
+
+    def get_alignment_after(self, start_align, callstack, log):
+        '''
+        get the alignment after this type based on the given start_align.
+        the start_align is checked for compatibility with the
+        internal start align. If it is not compatible, then None is returned
+        '''
+        if self.required_start_align is None or self.required_start_align.is_guaranteed_at(start_align):
+            return self.unchecked_get_alignment_after(start_align, callstack, log)
+        else:
+            if log is not None:
+                log.fail(start_align, "", self, callstack + [self],
+                    "start_align is incompatible with required_start_align %s"
+                    % (str(self.required_start_align)))
+            return None
+
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        '''
+        Abstract method for geting the alignment after this type
+        when the alignment at the start is given, and when this type
+        has variable size.
+        '''
+        raise Exception('abstract unchecked_get_alignment_after method not overridden!')
+
+
+    @staticmethod
+    def type_name_to_str(type_name):
+        if isinstance(type_name, str):
+            #already a string
+            return type_name
+        else:
+            return ".".join(type_name)
+
+
+    def __str__(self):
+        return type(self).__name__ + " \"" + Type.type_name_to_str(self.name) + "\""
+
+class PrimitiveType(Type):
+
+    def __init__(self, name, size):
+        Type.__init__(self, name)
+        self.size = size
+        self.nmemb = 1
+
+        # compute the required start_alignment based on the size of the type
+        self.required_start_align = Alignment.for_primitive_type(self.size)
+
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        my_callstack = callstack + [self];
+        after_align = start_align.align_after_fixed_size(self.size)
+
+        if log is not None:
+            if after_align is None:
+                log.fail(start_align, "", self, my_callstack,
+                "align after fixed size %d failed" % self.size)
+            else:
+                log.ok(start_align, "", self, my_callstack, after_align)
+
+        return after_align
+
+    def fixed_size(self):
+        return True
+
+class SimpleType(PrimitiveType):
     '''
     Derived class which represents a cardinal type like CARD32 or char.
     Any type which is typedef'ed to cardinal will be one of these.
@@ -100,17 +191,13 @@ class SimpleType(Type):
     none
     '''
     def __init__(self, name, size):
-        Type.__init__(self, name)
+        PrimitiveType.__init__(self, name, size)
         self.is_simple = True
-        self.size = size
-        self.nmemb = 1
+
 
     def resolve(self, module):
         self.resolved = True
 
-    def fixed_size(self):
-        return True
-
     out = __main__.output['simple']
 
 
@@ -189,6 +276,8 @@ class ListType(Type):
         self.size = member.size if member.fixed_size() else None
         self.nmemb = self.expr.nmemb if self.expr.fixed_size() else None
 
+        self.required_start_align = self.member.required_start_align
+
     def make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum=None):
         if not self.fixed_size():
             # We need a length field.
@@ -219,6 +308,8 @@ class ListType(Type):
         self.member.resolve(module)
         self.expr.resolve(module, self.parents)
 
+        self.required_start_align = self.member.required_start_align
+
         # Find my length field again.  We need the actual Field object in the expr.
         # This is needed because we might have added it ourself above.
         if not self.fixed_size():
@@ -233,7 +324,70 @@ class ListType(Type):
     def fixed_size(self):
         return self.member.fixed_size() and self.expr.fixed_size()
 
-class ExprType(Type):
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        my_callstack = callstack[:]
+        my_callstack.append(self)
+        if start_align is None:
+            log.fail(start_align, "", self, my_callstack, "start_align is None")
+            return None
+        if self.expr.fixed_size():
+            # fixed number of elements
+            num_elements = self.nmemb
+            prev_alignment = None
+            alignment = start_align
+            while num_elements > 0:
+                if alignment is None:
+                    if log is not None:
+                        log.fail(start_align, "", self, my_callstack,
+                            ("fixed size list with size %d after %d iterations"
+                            + ", at transition from alignment \"%s\"")
+                            % (self.nmemb,
+                               (self.nmemb - num_elements),
+                               str(prev_alignment)))
+                    return None
+                prev_alignment = alignment
+                alignment = self.member.get_alignment_after(prev_alignment, my_callstack, log)
+                num_elements -= 1
+            if log is not None:
+                log.ok(start_align, "", self, my_callstack, alignment)
+            return alignment
+        else:
+            # variable number of elements
+            # check whether the number of elements is a multiple
+            multiple = self.expr.get_multiple()
+            assert multiple > 0
+
+            # iterate until the combined alignment does not change anymore
+            alignment = start_align
+            while True:
+                prev_multiple_alignment = alignment
+                # apply "multiple" amount of changes sequentially
+                prev_alignment = alignment
+                for multiple_count in range(0, multiple):
+
+                    after_alignment = self.member.get_alignment_after(prev_alignment, my_callstack, log)
+                    if after_alignment is None:
+                        if log is not None:
+                            log.fail(start_align, "", self, my_callstack,
+                                ("variable size list "
+                                + "at transition from alignment \"%s\"")
+                                % (str(prev_alignment)))
+                        return None
+
+                    prev_alignment = after_alignment
+
+                # combine with the cumulatively combined alignment
+                # (to model the variable number of entries)
+                alignment = prev_multiple_alignment.combine_with(after_alignment)
+
+                if alignment == prev_multiple_alignment:
+                    # does not change anymore by adding more potential elements
+                    # -> finished
+                    if log is not None:
+                        log.ok(start_align, "", self, my_callstack, alignment)
+                    return alignment
+
+class ExprType(PrimitiveType):
     '''
     Derived class which represents an exprfield.  Fixed size.
 
@@ -241,24 +395,19 @@ class ExprType(Type):
     expr is an Expression object containing the value of the field.
     '''
     def __init__(self, elt, member, *parents):
-        Type.__init__(self, member.name)
+        PrimitiveType.__init__(self, member.name, member.size)
         self.is_expr = True
         self.member = member
         self.parents = parents
 
         self.expr = Expression(list(elt)[0], self)
 
-        self.size = member.size
-        self.nmemb = 1
-
     def resolve(self, module):
         if self.resolved:
             return
         self.member.resolve(module)
         self.resolved = True
 
-    def fixed_size(self):
-        return True
 
 class PadType(Type):
     '''
@@ -274,12 +423,47 @@ class PadType(Type):
             self.nmemb = int(elt.get('bytes', "1"), 0)
             self.align = int(elt.get('align', "1"), 0)
 
+        # pads don't require any alignment at their start
+        self.required_start_align = Alignment(1,0)
+
     def resolve(self, module):
         self.resolved = True
 
     def fixed_size(self):
         return self.align <= 1
 
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        if self.align <= 1:
+            # fixed size pad
+            after_align = start_align.align_after_fixed_size(self.get_total_size())
+            if log is not None:
+                if after_align is None:
+                    log.fail(start_align, "", self, callstack,
+                    "align after fixed size pad of size %d failed" % self.size)
+                else:
+                    log.ok(start_align, "", self, callstack, after_align)
+
+            return after_align
+
+        # align-pad
+        assert self.align > 1
+        assert self.size == 1
+        assert self.nmemb == 1
+        if (start_align.offset == 0
+           and self.align <= start_align.align
+           and start_align.align % self.align == 0):
+            # the alignment pad is size 0 because the start_align
+            # is already sufficiently aligned -> return the start_align
+            after_align = start_align
+        else:
+            # the alignment pad has nonzero size -> return the alignment
+            # that is guaranteed by it, independently of the start_align
+            after_align = Alignment(self.align, 0)
+
+        if log is not None:
+            log.ok(start_align, "", self, callstack, after_align)
+
+        return after_align
     
 class ComplexType(Type):
     '''
@@ -298,6 +482,18 @@ class ComplexType(Type):
         self.lenfield_parent = [self]
         self.fds = []
 
+        # get required_start_alignment
+        required_start_align_element = elt.find("required_start_align")
+        if required_start_align_element is None:
+            # unknown -> mark for autocompute
+            self.required_start_align = None
+        else:
+            self.required_start_align = Alignment(
+                int(required_start_align_element.get('align', "4"), 0),
+                int(required_start_align_element.get('offset', "0"), 0))
+            if verbose_align_log:
+                print "Explicit start-align for %s: %s\n" % (self, self.required_start_align)
+
     def resolve(self, module):
         if self.resolved:
             return
@@ -352,7 +548,15 @@ class ComplexType(Type):
             # Recursively resolve the type (could be another structure, list)
             type.resolve(module)
 
+            # Compute the size of the maximally contain align-pad
+            if type.max_align_pad > self.max_align_pad:
+                self.max_align_pad = type.max_align_pad
+
+        self.check_implicit_fixed_size_part_aligns();
+
         self.calc_size() # Figure out how big we are
+        self.calc_or_check_required_start_align()
+
         self.resolved = True
 
     def calc_size(self):
@@ -361,17 +565,155 @@ class ComplexType(Type):
             if not m.wire:
                 continue
             if m.type.fixed_size():
-                self.size = self.size + (m.type.size * m.type.nmemb)
+                self.size = self.size + m.type.get_total_size()
             else:
                 self.size = None
                 break
 
+    def calc_or_check_required_start_align(self):
+        if self.required_start_align is None:
+            # no required-start-align configured -> calculate it
+            log = AlignmentLog()
+            callstack = []
+            self.required_start_align = self.calc_minimally_required_start_align(callstack, log)
+            if self.required_start_align is None:
+                print ("ERROR: could not calc required_start_align of %s\nDetails:\n%s"
+                    % (str(self), str(log)))
+            else:
+                if verbose_align_log:
+                    print ("calc_required_start_align: %s has start-align %s"
+                        % (str(self), str(self.required_start_align)))
+                    print "Details:\n" + str(log)
+                if self.required_start_align.offset != 0:
+                    print (("WARNING: %s\n\thas start-align with non-zero offset: %s"
+                        + "\n\tsuggest to add explicit definition with:"
+                        + "\n\t\t<required_start_align align=\"%d\" offset=\"%d\" />"
+                        + "\n\tor to fix the xml so that zero offset is ok\n")
+                        % (str(self), self.required_start_align,
+                           self.required_start_align.align,
+                           self.required_start_align.offset))
+        else:
+            # required-start-align configured -> check it
+            log = AlignmentLog()
+            callstack = []
+            if not self.is_possible_start_align(self.required_start_align, callstack, log):
+                print ("ERROR: required_start_align %s of %s causes problems\nDetails:\n%s"
+                    % (str(self.required_start_align), str(self), str(log)))
+
+
+    def calc_minimally_required_start_align(self, callstack, log):
+        # calculate the minimally required start_align that causes no
+        # align errors
+        best_log = None
+        best_failed_align = None
+        for align in [1,2,4,8]:
+            for offset in range(0,align):
+                align_candidate = Alignment(align, offset)
+                if verbose_align_log:
+                    print "trying %s for %s" % (str(align_candidate), str(self))
+                my_log = AlignmentLog()
+                if self.is_possible_start_align(align_candidate, callstack, my_log):
+                    log.append(my_log)
+                    if verbose_align_log:
+                        print "found start-align %s for %s" % (str(align_candidate), str(self))
+                    return align_candidate
+                else:
+                    my_ok_count = my_log.ok_count()
+                    if (best_log is None
+                       or my_ok_count > best_log.ok_count()
+                       or (my_ok_count == best_log.ok_count()
+                          and align_candidate.align > best_failed_align.align)
+                          and align_candidate.align != 8):
+                        best_log = my_log
+                        best_failed_align = align_candidate
+
+
+
+        # none of the candidates applies
+        # this type has illegal internal aligns for all possible start_aligns
+        if verbose_align_log:
+            print "didn't find start-align for %s" % str(self)
+        log.append(best_log)
+        return None
+
+    def is_possible_start_align(self, align, callstack, log):
+        if align is None:
+            return False
+        if (self.max_align_pad > align.align
+           or align.align % self.max_align_pad != 0):
+            # our align pad implementation depends on known alignment
+            # at the start of our type
+            return False
+
+        return self.get_alignment_after(align, callstack, log) is not None
+
     def fixed_size(self):
         for m in self.fields:
             if not m.type.fixed_size():
                 return False
         return True
 
+
+    # default impls of polymorphic methods which assume sequential layout of fields
+    # (like Struct or CaseOrBitcaseType)
+    def check_implicit_fixed_size_part_aligns(self):
+        # find places where the implementation of the C-binding would
+        # create code that makes the compiler add implicit alignment.
+        # make these places explicit, so we have
+        # consistent behaviour for all bindings
+        size = 0
+        for field in self.fields:
+            if not field.wire:
+                continue
+            if not field.type.fixed_size():
+                # end of fixed-size part
+                break
+            required_field_align = field.type.required_start_align
+            if required_field_align is None:
+                raise Exception(
+                    "field \"%s\" in \"%s\" has not required_start_align"
+                    % (field.field_name, self.name)
+                )
+            mis_align = (size + required_field_align.offset) % required_field_align.align
+            if mis_align != 0:
+                # implicit align pad is required
+                padsize = required_field_align.align - mis_align
+                raise Exception(
+                    "C-compiler would insert implicit alignpad of size %d before field \"%s\" in \"%s\""
+                    % (padsize, field.field_name, self.name)
+                )
+
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        # default impl assumes sequential layout of fields
+        # (like Struct or CaseOrBitcaseType)
+        my_align = start_align
+        if my_align is None:
+            return None
+
+        for field in self.fields:
+            if not field.wire:
+                continue
+            my_callstack = callstack[:]
+            my_callstack.extend([self, field])
+
+            prev_align = my_align
+            my_align = field.type.get_alignment_after(my_align, my_callstack, log)
+            if my_align is None:
+                if log is not None:
+                    log.fail(prev_align, field.field_name, self, my_callstack,
+                        "alignment is incompatible with this field")
+                return None
+            else:
+                if log is not None:
+                    log.ok(prev_align, field.field_name, self, my_callstack, my_align)
+
+        if log is not None:
+            my_callstack = callstack[:]
+            my_callstack.append(self)
+            log.ok(start_align, "", self, my_callstack, my_align)
+        return my_align
+
+
 class SwitchType(ComplexType):
     '''
     Derived class which represents a List of Items.  
@@ -439,6 +781,7 @@ class SwitchType(ComplexType):
                         self.fields.append(new_field)
 
         self.calc_size() # Figure out how big we are
+        self.calc_or_check_required_start_align()
         self.resolved = True
 
     def make_member_of(self, module, complex_type, field_type, field_name, visible, wire, auto, enum=None):
@@ -479,6 +822,123 @@ class SwitchType(ComplexType):
 #        return True
 
 
+
+    def check_implicit_fixed_size_part_aligns(self):
+        # this is done for the CaseType or BitCaseType
+        return
+
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        # we assume that BitCases can appear in any combination,
+        # and that at most one Case can appear
+        # (assuming that Cases are mutually exclusive)
+
+        # get all Cases (we assume that at least one case is selected if there are cases)
+        case_fields = []
+        for field in self.bitcases:
+            if field.type.is_case:
+                case_fields.append(field)
+
+        if not case_fields:
+            # there are no case-fields -> check without case-fields
+            case_fields = [None]
+
+        my_callstack = callstack[:]
+        my_callstack.append(self)
+        #
+        total_align = None
+        first = True
+        for case_field in case_fields:
+            my2_callstack = my_callstack[:]
+            if case_field is not None:
+                my2_callstack.append(case_field)
+
+            case_align = self.get_align_for_selected_case_field(
+                             case_field, start_align, my2_callstack, log)
+
+
+            if case_align is None:
+                if log is not None:
+                    if case_field is None:
+                        log.fail(start_align, "", self, my2_callstack,
+                            "alignment without cases (only bitcases) failed")
+                    else:
+                        log.fail(start_align, "", self, my2_callstack + [case_field],
+                            "alignment for selected case %s failed"
+                            % case_field.field_name)
+                return None
+            if first:
+                total_align = case_align
+            else:
+                total_align = total_align.combine_with(case_align)
+
+            if log is not None:
+                if case_field is None:
+                    log.ok(
+                        start_align,
+                        "without cases (only arbitrary bitcases)",
+                        self, my2_callstack, case_align)
+                else:
+                    log.ok(
+                        start_align,
+                        "case %s and arbitrary bitcases" % case_field.field_name,
+                        self, my2_callstack, case_align)
+
+
+        if log is not None:
+            log.ok(start_align, "", self, my_callstack, total_align)
+        return total_align
+
+    # aux function for unchecked_get_alignment_after
+    def get_align_for_selected_case_field(self, case_field, start_align, callstack, log):
+        if verbose_align_log:
+            print "get_align_for_selected_case_field: %s, case_field = %s" % (str(self), str(case_field))
+        total_align = start_align
+        for field in self.bitcases:
+            my_callstack = callstack[:]
+            my_callstack.append(field)
+
+            if not field.wire:
+                continue
+            if field is case_field:
+                # assume that this field is active -> no combine_with to emulate optional
+                after_field_align = field.type.get_alignment_after(total_align, my_callstack, log)
+
+                if log is not None:
+                    if after_field_align is None:
+                        log.fail(total_align, field.field_name, field.type, my_callstack,
+                            "invalid aligment for this case branch")
+                    else:
+                        log.ok(total_align, field.field_name, field.type, my_callstack,
+                            after_field_align)
+
+                total_align = after_field_align
+            elif field.type.is_bitcase:
+                after_field_align = field.type.get_alignment_after(total_align, my_callstack, log)
+                # we assume that this field is optional, therefore combine
+                # alignment after the field with the alignment before the field.
+                if after_field_align is None:
+                    if log is not None:
+                        log.fail(total_align, field.field_name, field.type, my_callstack,
+                            "invalid aligment for this bitcase branch")
+                    total_align = None
+                else:
+                    if log is not None:
+                        log.ok(total_align, field.field_name, field.type, my_callstack,
+                            after_field_align)
+
+                    # combine with the align before the field because
+                    # the field is optional
+                    total_align = total_align.combine_with(after_field_align)
+            else:
+                # ignore other fields as they are irrelevant for alignment
+                continue
+
+            if total_align is None:
+                break
+
+        return total_align
+
+
 class Struct(ComplexType):
     '''
     Derived class representing a struct data type.
@@ -497,6 +957,66 @@ class Union(ComplexType):
     out = __main__.output['union']
 
 
+    def calc_size(self):
+        self.size = 0
+        for m in self.fields:
+            if not m.wire:
+                continue
+            if m.type.fixed_size():
+                self.size = max(self.size, m.type.get_total_size())
+            else:
+                self.size = None
+                break
+
+
+    def check_implicit_fixed_size_part_aligns(self):
+        # a union does not have implicit aligns because all fields start
+        # at the start of the union
+        return
+
+
+    def unchecked_get_alignment_after(self, start_align, callstack, log):
+        my_callstack = callstack[:]
+        my_callstack.append(self)
+
+        after_align = None
+        if self.fixed_size():
+
+            #check proper alignment for all members
+            start_align_ok = all(
+                [field.type.is_acceptable_start_align(start_align, my_callstack + [field], log)
+                for field in self.fields])
+
+            if start_align_ok:
+                #compute the after align from the start_align
+                after_align = start_align.align_after_fixed_size(self.get_total_size())
+            else:
+                after_align = None
+
+            if log is not None and after_align is not None:
+                log.ok(start_align, "fixed sized union", self, my_callstack, after_align)
+
+        else:
+            if start_align is None:
+                if log is not None:
+                    log.fail(start_align, "", self, my_callstack,
+                        "missing start_align for union")
+                return None
+
+            after_align = reduce(
+                lambda x, y: None if x is None or y is None else x.combine_with(y),
+                [field.type.get_alignment_after(start_align, my_callstack + [field], log)
+                 for field in self.fields])
+
+            if log is not None and after_align is not None:
+                log.ok(start_align, "var sized union", self, my_callstack, after_align)
+
+
+        if after_align is None and log is not None:
+            log.fail(start_align, "", self, my_callstack, "start_align is not ok for all members")
+
+        return after_align
+
 class CaseOrBitcaseType(ComplexType):
     '''
     Derived class representing a case or bitcase.
@@ -504,13 +1024,11 @@ class CaseOrBitcaseType(ComplexType):
     def __init__(self, index, name, elt, *parent):
         elts = list(elt)
         self.expr = []
-        fields = []
-        for elt in elts:
-            if elt.tag == 'enumref':
-                self.expr.append(Expression(elt, self))
-            else:
-                fields.append(elt)
-        ComplexType.__init__(self, name, fields)
+        for sub_elt in elts:
+            if sub_elt.tag == 'enumref':
+                self.expr.append(Expression(sub_elt, self))
+                elt.remove(sub_elt)
+        ComplexType.__init__(self, name, elt)
         self.has_name = True
         self.index = 1
         self.lenfield_parent = list(parent) + [self]
@@ -545,6 +1063,9 @@ class CaseOrBitcaseType(ComplexType):
         # Resolve the bitcase expression
         ComplexType.resolve(self, module)
 
+        #calculate alignment
+        self.calc_or_check_required_start_align()
+
 
 class BitcaseType(CaseOrBitcaseType):
     '''
@@ -571,6 +1092,8 @@ class Reply(ComplexType):
         ComplexType.__init__(self, name, elt)
         self.is_reply = True
         self.doc = None
+        if self.required_start_align is None:
+            self.required_start_align = Alignment(4,0)
 
         for child in list(elt):
             if child.tag == 'doc':
@@ -602,6 +1125,8 @@ class Request(ComplexType):
         self.reply = None
         self.doc = None
         self.opcode = elt.get('opcode')
+        if self.required_start_align is None:
+            self.required_start_align = Alignment(4,0)
 
         for child in list(elt):
             if child.tag == 'reply':
@@ -639,6 +1164,10 @@ class Event(ComplexType):
     '''
     def __init__(self, name, elt):
         ComplexType.__init__(self, name, elt)
+
+        if self.required_start_align is None:
+            self.required_start_align = Alignment(4,0)
+
         self.opcodes = {}
 
         self.has_seq = not bool(elt.get('no-sequence-number'))
@@ -693,6 +1222,8 @@ class Error(ComplexType):
     def __init__(self, name, elt):
         ComplexType.__init__(self, name, elt)
         self.opcodes = {}
+        if self.required_start_align is None:
+            self.required_start_align = Alignment(4,0)
 
     def add_opcode(self, opcode, name, main):
         self.opcodes[name] = opcode
commit ace537d050a3efa6c6370fecd3d1ebe82b2df4f3
Author: Christian Linhart <chris at demorecorder.com>
Date:   Sun Nov 1 18:26:32 2015 +0100

    xinput: add non-default start-aligns for switches and cases
    
    Especially, these start-aligns have non-zero offsets.
    
    Signed-off-by: Christian Linhart <chris at demorecorder.com>

diff --git a/src/xinput.xml b/src/xinput.xml
index 6564f4c..94855ba 100644
--- a/src/xinput.xml
+++ b/src/xinput.xml
@@ -173,6 +173,7 @@ This affects the following:
         <field type="CARD8" name="len" />
         <switch name="info">
             <fieldref>class_id</fieldref>
+            <required_start_align align="4" offset="2" />
             <case name="key">
                 <enumref ref="InputClass">Key</enumref>
                 <field type="KeyCode" name="min_keycode" />
@@ -186,6 +187,7 @@ This affects the following:
             </case>
             <case name="valuator">
                 <enumref ref="InputClass">Valuator</enumref>
+                <required_start_align align="4" offset="2" />
                 <field type="CARD8"   name="axes_len" />
                 <field type="CARD8"   name="mode" enum="ValuatorMode" />
                 <field type="CARD32"  name="motion_size" />
@@ -941,8 +943,10 @@ This affects the following:
         <field type="CARD8" name="len" />
         <switch name="data">
             <fieldref>class_id</fieldref>
+            <required_start_align align="4" offset="2" />
             <case name="key">
                 <enumref ref="InputClass">Key</enumref>
+                <required_start_align align="4" offset="2" />
                 <field type="CARD8" name="num_keys" />
                 <pad bytes="1" />
                 <list type="CARD8" name="keys">
@@ -959,6 +963,7 @@ This affects the following:
             </case>
             <case name="valuator">
                 <enumref ref="InputClass">Valuator</enumref>
+                <required_start_align align="4" offset="2" />
                 <field type="CARD8" name="num_valuators" />
                 <field type="CARD8" name="mode" mask="ValuatorStateModeMask" />
                 <list type="INT32" name="valuators">
@@ -1726,8 +1731,10 @@ This affects the following:
         <field type="DeviceId" name="sourceid" />
 	<switch name="data">
 	    <fieldref>type</fieldref>
+	    <required_start_align align="4" offset="2" />
 	    <case name="key">
 		<enumref ref="DeviceClassType">Key</enumref>
+		<required_start_align align="4" offset="2" />
 		<field type="CARD16"   name="num_keys" />
 		<list type="CARD32" name="keys">
 		    <fieldref>num_keys</fieldref>
@@ -1735,6 +1742,7 @@ This affects the following:
 	    </case>
 	    <case name="button">
 		<enumref ref="DeviceClassType">Button</enumref>
+		<required_start_align align="4" offset="2" />
 		<field type="CARD16"   name="num_buttons" />
 		<list type="CARD32"    name="state">
 		    <op op="/">
@@ -1751,6 +1759,7 @@ This affects the following:
 	    </case>
 	    <case name="valuator">
 		<enumref ref="DeviceClassType">Valuator</enumref>
+		<required_start_align align="4" offset="2" />
 		<field type="CARD16"   name="number" />
 		<field type="ATOM"     name="label" />
 		<field type="FP3232"   name="min" />
@@ -1762,6 +1771,7 @@ This affects the following:
 	    </case>
 	    <case name="scroll">
 		<enumref ref="DeviceClassType">Scroll</enumref>
+                <required_start_align align="4" offset="2" />
 		<field type="CARD16"   name="number" />
 		<field type="CARD16"   name="scroll_type" enum="ScrollType" />
 		<pad bytes="2" />
commit 49a3541ed2b13438b305afbb70fe34ce1bbe3d0b
Author: Christian Linhart <chris at demorecorder.com>
Date:   Sun Nov 1 18:26:31 2015 +0100

    present: add non-default start-aligns for requests and events
    
    Signed-off-by: Christian Linhart <chris at demorecorder.com>

diff --git a/src/present.xml b/src/present.xml
index 513388c..a648ad7 100644
--- a/src/present.xml
+++ b/src/present.xml
@@ -88,6 +88,7 @@ OF THIS SOFTWARE.
   </request>
 
   <request name="Pixmap" opcode="1">
+    <required_start_align align="8" />
     <field type="WINDOW" name="window" />
     <field type="PIXMAP" name="pixmap" />
     <field type="CARD32" name="serial" />
@@ -107,6 +108,7 @@ OF THIS SOFTWARE.
   </request>
 
   <request name="NotifyMSC" opcode="2">
+    <required_start_align align="8" />
     <field type="WINDOW" name="window" />
     <field type="CARD32" name="serial" />
     <pad bytes="4"/>
@@ -157,6 +159,7 @@ OF THIS SOFTWARE.
   </event>
 
   <event name="CompleteNotify" number="1" xge="true">
+    <required_start_align align="8" />
     <field type="CARD8" name="kind" enum="CompleteKind" />
     <field type="CARD8" name="mode" enum="CompleteMode" />
     <field type="EVENT" name="event" />
@@ -176,6 +179,7 @@ OF THIS SOFTWARE.
   </event>
 
   <event name="RedirectNotify" number="3" xge="true">
+    <required_start_align align="8" />
     <field type="BOOL" name="update_window"/>
     <pad bytes="1"/>
     <field type="EVENT" name="event" />
commit 0e564a0df5a7be32c2e25561e6eb863262fb4b8e
Author: Christian Linhart <chris at demorecorder.com>
Date:   Sun Nov 1 18:26:30 2015 +0100

    glx: add non-default start-aligns for replies
    
    Signed-off-by: Christian Linhart <chris at demorecorder.com>

diff --git a/src/glx.xml b/src/glx.xml
index e0870f4..2e50aea 100644
--- a/src/glx.xml
+++ b/src/glx.xml
@@ -665,6 +665,7 @@ The patch that fixed this server bug in X.org CVS is here:
 		<field type="CONTEXT_TAG" name="context_tag" />
 		<field type="INT32" name="plane" />
 		<reply>
+			<required_start_align align="8" />
 			<pad bytes="1" />
 			<pad bytes="24" />
 			<list type="FLOAT64" name="data">
@@ -680,6 +681,7 @@ The patch that fixed this server bug in X.org CVS is here:
 		<field type="CONTEXT_TAG" name="context_tag" />
 		<field type="CARD32" name="pname" />
 		<reply>
+			<required_start_align align="8" />
 			<pad bytes="1" />
 			<pad bytes="4" />
 			<field type="CARD32" name="n" />
@@ -766,6 +768,7 @@ The patch that fixed this server bug in X.org CVS is here:
 		<field type="CARD32" name="target" />
 		<field type="CARD32" name="query" />
 		<reply>
+			<required_start_align align="8" />
 			<pad bytes="1" />
 			<pad bytes="4" />
 			<field type="CARD32" name="n" />
@@ -953,6 +956,7 @@ The patch that fixed this server bug in X.org CVS is here:
 		<field type="CARD32" name="coord" />
 		<field type="CARD32" name="pname" />
 		<reply>
+			<required_start_align align="8" />
 			<pad bytes="1" />
 			<pad bytes="4" />
 			<field type="CARD32" name="n" />


More information about the xcb-commit mailing list