Visual Studio packs bit-fields differently

Author: Daniel Kroening

appveyor.yml

@@ -70,7 +70,6 @@ test_script:
     rmdir /s /q ansi-c\arch_flags_mthumb_good
     rmdir /s /q ansi-c\Forward_Declaration2
     rmdir /s /q ansi-c\Incomplete_Type1
-    rmdir /s /q ansi-c\Union_Padding1
     rmdir /s /q ansi-c\Universal_characters1
     rmdir /s /q ansi-c\function_return1
     rmdir /s /q ansi-c\gcc_attributes7

buildspec-windows.yml

@@ -34,7 +34,6 @@ phases:
         Remove-Item ansi-c\arch_flags_mthumb_good -Force -Recurse
         Remove-Item ansi-c\Forward_Declaration2 -Force -Recurse
         Remove-Item ansi-c\Incomplete_Type1 -Force -Recurse
-        Remove-Item ansi-c\Union_Padding1 -Force -Recurse
         Remove-Item ansi-c\Universal_characters1 -Force -Recurse
         Remove-Item ansi-c\function_return1 -Force -Recurse
         Remove-Item ansi-c\gcc_attributes7 -Force -Recurse

regression/ansi-c/Struct_Padding5/main.c

@@ -28,6 +28,46 @@ STATIC_ASSERT(sizeof(struct flowi)==8);
 STATIC_ASSERT(__builtin_offsetof(struct flowi, flexible)==1);
 #endif
 
+// bit-fields are very evil
+#pragma pack(push, 1)
+struct bit_field0
+{
+  int i:23;
+};
+
+struct bit_field1
+{
+  int i:1;
+  unsigned int j:1;
+  // in MSC, it matters that the underlying type changes!
+  short c:1;
+};
+
+struct bit_field2
+{
+  int i:23;
+  char ch;
+};
+#pragma pack(pop)
+
+struct bit_field3
+{
+  int i:23;
+  char ch;
+};
+
+#ifdef _MSC_VER
+STATIC_ASSERT(sizeof(struct bit_field0)==4);
+STATIC_ASSERT(sizeof(struct bit_field1)==6);
+STATIC_ASSERT(sizeof(struct bit_field2)==5);
+STATIC_ASSERT(sizeof(struct bit_field3)==8);
+#else
+STATIC_ASSERT(sizeof(struct bit_field0)==3);
+STATIC_ASSERT(sizeof(struct bit_field1)==1);
+STATIC_ASSERT(sizeof(struct bit_field2)==4);
+STATIC_ASSERT(sizeof(struct bit_field3)==4);
+#endif
+
 int main()
 {
 }

regression/ansi-c/Union_Padding1/main.c

@@ -41,14 +41,16 @@ STATIC_ASSERT(sizeof(union some_union3)==sizeof(int));
 #ifdef _MSC_VER
 
 // bit-fields are evil
-#pragma pack(1)
+#pragma pack(push,1)
 union some_union4
 {
   int i:23;
 };
+#pragma pack(pop)
 
 // Visual Studio ignores the 'packed'
 STATIC_ASSERT(sizeof(union some_union4)==sizeof(int));
+STATIC_ASSERT(__alignof(union some_union4)==1);
 
 #else
 
@@ -59,34 +61,39 @@ union some_union4
 } __attribute__((__packed__));
 
 STATIC_ASSERT(sizeof(union some_union4)==3);
+STATIC_ASSERT(_Alignof(union some_union4)==1);
 
 #endif
 
-#ifdef _MSC_VER
-
 union some_union5
 {
   int i;
 };
 
-STATIC_ASSERT(__alignof(union some_union5)==1);
-
+#ifdef _MSC_VER
+STATIC_ASSERT(__alignof(union some_union5)==4);
 #else
+STATIC_ASSERT(_Alignof(union some_union5)==4);
+#endif
 
-union some_union5
+#ifdef _MSC_VER
+#pragma pack(push,1)
+union some_union6
 {
   int i;
 };
+#pragma pack(pop)
 
+// Packing may affect alignment
+STATIC_ASSERT(__alignof(union some_union6)==1);
+#else
 union some_union6
 {
   int i;
 } __attribute__((__packed__));
 
 // Packing may affect alignment
-STATIC_ASSERT(_Alignof(union some_union5)==4);
 STATIC_ASSERT(_Alignof(union some_union6)==1);
-
 #endif
 
 int main()

src/ansi-c/padding.cpp

@@ -100,15 +100,93 @@ mp_integer alignment(const typet &type, const namespacet &ns)
   return result;
 }
 
-void add_padding(struct_typet &type, const namespacet &ns)
+struct_typet::componentst::iterator pad_bit_field(
+  struct_typet::componentst &components,
+  struct_typet::componentst::iterator where,
+  std::size_t pad_bits)
+{
+  c_bit_field_typet padding_type(
+    unsignedbv_typet(pad_bits), pad_bits);
+
+  struct_typet::componentt component;
+  component.type()=padding_type;
+  component.set_name(
+    "$bit_field_pad"+std::to_string(where-components.begin()));
+  component.set_is_padding(true);
+
+  auto it=components.insert(where, component);
+  it++;
+  return it;
+}
+
+void bit_field_padding(struct_typet &type, const namespacet &ns)
 {
   struct_typet::componentst &components=type.components();
 
-  // First do padding for bit-fields to make them
-  // appear on byte boundaries.
+  // bit-field padding depends greatly on compiler
+  if(config.ansi_c.mode == configt::ansi_ct::flavourt::VISUAL_STUDIO)
+  {
+    std::size_t bit_field_bits=0, underlying_bits=0;
+
+    for(struct_typet::componentst::iterator
+        it=components.begin();
+        it!=components.end();
+        it++)
+    {
+      if(it->type().id()==ID_c_bit_field &&
+         to_c_bit_field_type(it->type()).get_width()!=0)
+      {
+        const auto &bit_field_type=to_c_bit_field_type(it->type());
+
+        std::size_t new_underlying_bits=
+          bit_field_type.subtype().get_size_t(ID_width);
 
+        CHECK_RETURN(new_underlying_bits!=0);
+
+        if(underlying_bits==0)
+        {
+          underlying_bits=new_underlying_bits;
+          bit_field_bits=0;
+        }
+        else if(new_underlying_bits!=underlying_bits)
+        {
+          // new underlying width, we pad up
+          if((bit_field_bits%underlying_bits)!=0)
+          {
+            std::size_t pad=underlying_bits-(bit_field_bits%underlying_bits);
+            it=pad_bit_field(components, it, pad);
+          }
+
+          underlying_bits=new_underlying_bits;
+          bit_field_bits=0;
+        }
+
+        // count the bits
+        std::size_t width=bit_field_type.get_width();
+        bit_field_bits+=width;
+      }
+      else
+      {
+        // pad up
+        if(underlying_bits!=0 && (bit_field_bits%underlying_bits)!=0)
+        {
+          std::size_t pad=underlying_bits-(bit_field_bits%underlying_bits);
+          it=pad_bit_field(components, it, pad);
+          underlying_bits=bit_field_bits=0;
+        }
+      }
+    }
+
+    // Add padding at the end?
+    if(underlying_bits!=0 && (bit_field_bits%underlying_bits)!=0)
+    {
+      std::size_t pad=underlying_bits-(bit_field_bits%underlying_bits);
+      pad_bit_field(components, components.end(), pad);
+    }
+  }
+  else
   {
-    std::size_t padding_counter=0;
+    // Make bit-fields appear on byte boundaries
     std::size_t bit_field_bits=0;
 
     for(struct_typet::componentst::iterator
@@ -129,18 +207,7 @@ void add_padding(struct_typet &type, const namespacet &ns)
         if((bit_field_bits%8)!=0)
         {
           std::size_t pad=8-bit_field_bits%8;
-          c_bit_field_typet padding_type(unsignedbv_typet(pad), pad);
-
-          struct_typet::componentt component;
-          component.type()=padding_type;
-          component.set_name(
-            "$bit_field_pad"+std::to_string(padding_counter++));
-          component.set_is_padding(true);
-
-          it=components.insert(it, component);
-          it++; // skip over
-
-          bit_field_bits+=pad;
+          it=pad_bit_field(components, it, pad);
         }
 
         bit_field_bits=0;
@@ -151,16 +218,14 @@ void add_padding(struct_typet &type, const namespacet &ns)
     if((bit_field_bits%8)!=0)
     {
       std::size_t pad=8-bit_field_bits%8;
-      c_bit_field_typet padding_type(unsignedbv_typet(pad), pad);
-
-      struct_typet::componentt component;
-      component.type()=padding_type;
-      component.set_name("$bit_field_pad"+std::to_string(padding_counter++));
-      component.set_is_padding(true);
-
-      components.push_back(component);
+      pad_bit_field(components, components.end(), pad);
     }
   }
+}
+
+void add_padding(struct_typet &type, const namespacet &ns)
+{
+  bit_field_padding(type, ns);
 
   // Is the struct packed, without any alignment specification?
   if(type.get_bool(ID_C_packed) &&
@@ -172,6 +237,8 @@ void add_padding(struct_typet &type, const namespacet &ns)
   mp_integer max_alignment=0;
   std::size_t bit_field_bits=0;
 
+  struct_typet::componentst &components=type.components();
+
   for(struct_typet::componentst::iterator
       it=components.begin();
       it!=components.end();