Compile time asserts to catch things like: [Bug 23403] compiler padding causes reply parsing to use incorrect offsets

Submitted by Uli Schlachter on Oct. 31, 2017, 11:42 a.m.

Details

Message ID feba9ce9-01a9-d5ef-bd49-7ff01bc4db6c@znc.in
State New
Series "Compile time asserts to catch things like: [Bug 23403] compiler padding causes reply parsing to use incorrect offsets"
Headers show

Commit Message

Uli Schlachter Oct. 31, 2017, 11:42 a.m.
Hi everyone,

due to the recent activity on [1], I wrote the attached patch. It adds
compile time asserts checking that sizeof() various things is as
expected. The full list of failures is also attached (but some of these
are still bugs with my patch, i.e. I doubt sizeof(xcb_xkb_get_map_map_t)
== 0 should hold).

I only took a quick look at the result of the patch.
xcb_sync_systemcounter_t (the subject of bug 23403) is spotted. The
first two GLX-related things are about structs with uint8_t fields at
their end. I didn't look closer than this at the results.

Should something like this be added to libxcb eventually? Does anyone
want to look at these? For example, the compiler-added padding at the
end of xcb_*_request_t either is correct or sending this request never
worked correctly in the first place, so should be possible to add to the
XML.

From the total 38 failures, there are 11 failures for requests. 2 for
errors (both in SYNC), 4 in replies, and 21 "remaining" failures.

Cheers,
Uli

P.S.: I don't know python and I don't know the python code around
xcb/proto. The attached patch was written by trial and error, and by
hacking around until xproto.c compiled. I bet/hope someone with an
actual clue about the code can come up with a nicer patch.

[1]: https://bugs.freedesktop.org/show_bug.cgi?id=23403

Patch hide | download patch | download mbox

diff --git a/src/c_client.py b/src/c_client.py
index 2213a31..d897998 100644
--- a/src/c_client.py
+++ b/src/c_client.py
@@ -289,8 +289,13 @@  def c_open(self):
 
     _c('')
     _c('#define ALIGNOF(type) offsetof(struct { char dummy; type member; }, member)')
+    _c('#define PASTE_TOKENS(a, b, c) a##b##c')
+    _c('#define COMPILE_TIME_ASSERT_IMPL(expr, comment, line) \\')
+    _c('    typedef char PASTE_TOKENS(assertion_failed_,line,comment)[2*!!(expr)-1];')
+    _c('#define COMPILE_TIME_ASSERT(expr, comment) COMPILE_TIME_ASSERT_IMPL(expr, comment, __LINE__)')
 
     if _ns.is_ext:
+        _c('')
         for (n, h) in self.direct_imports:
             _hc('#include "%s.h"', h)
 
@@ -2089,11 +2094,22 @@  def _c_complex(self, force_packed = False):
 
     struct_fields = []
     maxtypelen = 0
+    size = 0
 
     for field in self.fields:
         if field.wire and (field.type.fixed_size() or self.is_switch or self.is_union):
             struct_fields.append(field)
 
+    if self.is_union:
+        for field in struct_fields:
+            size = max(size, field.type.nmemb * field.type.size)
+    else:
+        for field in struct_fields:
+            if field.type.fixed_size():
+                size += field.type.nmemb * field.type.size
+
+    _c('COMPILE_TIME_ASSERT(sizeof(%s) == %d, sizeof_%s);', self.c_type, size, self.c_type)
+
     for field in struct_fields:
         length = len(field.c_field_type)
         # account for '*' pointer_spec