bug-hurd
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH mig] Make MIG work for pure 64 bit kernel and userland.


From: Flavio Cruz
Subject: [PATCH mig] Make MIG work for pure 64 bit kernel and userland.
Date: Sun, 12 Feb 2023 01:15:05 -0500

Made changes to MIG so that generated stubs can be compiled when using
a 64 bit userland. The most important change here is to ensure
that data is always 8-byte aligned. Not doing that will lead to
undefined behavior since on 64 bits many structures can contain 64 bit
scalars which require 8-byte alignment. Even if x86 is usually fine with
unaligned accesses, compilers are taking advantage of this UD which may
break us in the future
(http://pzemtsov.github.io/2016/11/06/bug-story-alignment-on-x86.html)

The downside is that messages get significantly larger since we have
to pad the initial mach_msg_type_t so that the data item can be 8-byte
aligned. In the future, I want to make MIG layout the data in a way
that non-complex data can be packed together (essentially, how XNU messages
work) and to force userland to create larger structures so that we don't
have to do any structure resizing while in the kernel. For now, this unblocks
future work on 64 bits and is quite simple to understand.

For the 64 bit / 32 bit configuration, we can pass --enable-user32 when
targeting x86_64, and MIG will generate stubs as before. I am unsure
whether there exists undefined behavior in that particular configuration
since most of the structures used by the kernel do not seem to use 8
byte scalars so potentially we should be OK there.

As a follow up, we will need to change gnumach to iterate over these
messages correctly (to follow the 8-byte alignment rules).

Tested this patch by building Hurd and comparing previous stubs which
are byte-by-byte identical.
---
 configure.ac |  7 +++++
 cpu.sym      |  4 +--
 global.c     |  5 ++--
 global.h     |  5 ++--
 parser.y     | 14 +++++++---
 routine.c    |  5 +++-
 type.c       | 75 +++++++++++++++++++++++++---------------------------
 utils.c      | 12 +++++++--
 utils.h      |  2 ++
 9 files changed, 77 insertions(+), 52 deletions(-)

diff --git a/configure.ac b/configure.ac
index e4645bd..8d52d5f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -55,6 +55,13 @@ AC_SUBST([MIGCOM])
 [MIG=`echo mig | sed "$program_transform_name"`]
 AC_SUBST([MIG])
 
+AC_ARG_ENABLE(user32,
+              AS_HELP_STRING([--enable-user32],
+                             [Build mig for 64 bit kernel that works with 32 
bit userland]))
+AS_IF([test "x$enable_user32" = "xyes"], [
+       AC_DEFINE([USER32], [1], ["Build mig for 64 bit kernel that works with 
32 bit userland"])
+])
+
 AC_OUTPUT([Makefile mig tests/Makefile \
            tests/good/Makefile \
            tests/generate-only/Makefile \
diff --git a/cpu.sym b/cpu.sym
index 6bcb878..7858b47 100644
--- a/cpu.sym
+++ b/cpu.sym
@@ -95,8 +95,8 @@ expr MACH_MSG_TYPE_POLYMORPHIC
 
 
 /* Types used in interfaces */
-expr sizeof(integer_t)                 word_size
-expr (sizeof(integer_t)*8)             word_size_in_bits
+/* The minimum alignment required for this architecture */
+expr sizeof(uintptr_t)                 desired_machine_alignment
 expr sizeof(void*)                     sizeof_pointer
 expr sizeof(char)                      sizeof_char
 expr sizeof(short)                     sizeof_short
diff --git a/global.c b/global.c
index bf3d10d..ff10e58 100644
--- a/global.c
+++ b/global.c
@@ -66,8 +66,9 @@ string_t InternalHeaderFileName = strNULL;
 string_t UserFileName = strNULL;
 string_t ServerFileName = strNULL;
 
-int port_size = port_name_size;
-int port_size_in_bits = port_name_size_in_bits;
+size_t port_size = port_name_size;
+size_t port_size_in_bits = port_name_size_in_bits;
+size_t word_size = desired_machine_alignment;
 
 void
 more_global(void)
diff --git a/global.h b/global.h
index 8e57df2..8a53b9e 100644
--- a/global.h
+++ b/global.h
@@ -67,8 +67,9 @@ extern string_t InternalHeaderFileName;
 extern string_t UserFileName;
 extern string_t ServerFileName;
 
-extern int port_size;
-extern int port_size_in_bits;
+extern size_t port_size;
+extern size_t port_size_in_bits;
+extern size_t word_size;
 
 extern void more_global(void);
 
diff --git a/parser.y b/parser.y
index 03c5ec8..1ebeae5 100644
--- a/parser.y
+++ b/parser.y
@@ -212,6 +212,16 @@ Subsystem          :       SubsystemStart SubsystemMods
               IsKernelUser ? ", KernelUser" : "",
               IsKernelServer ? ", KernelServer" : "");
     }
+    if (IsKernelUser || IsKernelServer) {
+        port_size = vm_offset_size;
+        port_size_in_bits = vm_offset_size_in_bits;
+#ifdef USER32
+        /* Since we are using MIG to generate stubs for a 64 bit kernel that 
operates
+           with a 32 bit userland, then we need to consider that `word_size` 
is 4 byte aligned.
+         */
+        word_size = sizeof(uint32_t);
+#endif
+    }
     init_type();
 }
                        ;
@@ -237,16 +247,12 @@ SubsystemMod              :       syKernelUser
     if (IsKernelUser)
        warn("duplicate KernelUser keyword");
     IsKernelUser = true;
-    port_size = vm_offset_size;
-    port_size_in_bits = vm_offset_size_in_bits;
 }
                        |       syKernelServer
 {
     if (IsKernelServer)
        warn("duplicate KernelServer keyword");
     IsKernelServer = true;
-    port_size = vm_offset_size;
-    port_size_in_bits = vm_offset_size_in_bits;
 }
                        ;
 
diff --git a/routine.c b/routine.c
index c0a016c..363d6be 100644
--- a/routine.c
+++ b/routine.c
@@ -47,6 +47,7 @@
 #include "routine.h"
 #include "message.h"
 #include "cpu.h"
+#include "utils.h"
 
 u_int rtNumber = 0;
 
@@ -316,19 +317,21 @@ rtFindSize(const argument_t *args, u_int mask)
     const argument_t *arg;
     u_int size = sizeof_mach_msg_header_t;
 
+    size = ALIGN(size, word_size);
     for (arg = args; arg != argNULL; arg = arg->argNext)
        if (akCheck(arg->argKind, mask))
        {
            ipc_type_t *it = arg->argType;
 
            /* might need proper alignment on demanding 64bit archies */
-           size = (size + word_size-1) & ~(word_size-1);
            if (arg->argLongForm) {
                size += sizeof_mach_msg_type_long_t;
            } else {
                size += sizeof_mach_msg_type_t;
            }
+           size = ALIGN(size, word_size);
 
+           /* Note itMinTypeSize is already aligned to word_size. */
            size += it->itMinTypeSize;
        }
 
diff --git a/type.c b/type.c
index b104c66..05ee201 100644
--- a/type.c
+++ b/type.c
@@ -35,18 +35,6 @@
 #include "cpu.h"
 #include "utils.h"
 
-#if word_size_in_bits == 32
-#define        word_size_name MACH_MSG_TYPE_INTEGER_32
-#define        word_size_name_string "MACH_MSG_TYPE_INTEGER_32"
-#else
-#if word_size_in_bits == 64
-#define        word_size_name MACH_MSG_TYPE_INTEGER_64
-#define        word_size_name_string "MACH_MSG_TYPE_INTEGER_64"
-#else
-#error Unsupported word size!
-#endif
-#endif
-
 ipc_type_t *itByteType;         /* used for defining struct types */
 ipc_type_t *itRetCodeType;     /* used for return codes */
 ipc_type_t *itDummyType;       /* used for camelot dummy args */
@@ -169,6 +157,7 @@ itNameToString(u_int name)
 static void
 itCalculateSizeInfo(ipc_type_t *it)
 {
+    assert(it->itAlignment > 0);
     if (it->itInLine)
     {
        u_int bytes = (it->itNumber * it->itSize + 7) / 8;
@@ -189,7 +178,7 @@ itCalculateSizeInfo(ipc_type_t *it)
        it->itTypeSize = bytes;
        it->itPadSize = 0;
        it->itMinTypeSize = bytes;
-       it->itAlignment = bytes;
+       assert(it->itAlignment == bytes);
     }
 
     /* Unfortunately, these warning messages can't give a type name;
@@ -682,6 +671,7 @@ itPtrDecl(ipc_type_t *it)
     it->itInLine = false;
     it->itStruct = true;
     it->itString = false;
+    it->itAlignment = sizeof_pointer;
 
     itCalculateSizeInfo(it);
     return it;
@@ -806,7 +796,6 @@ itCIntTypeDecl(const_string_t ctype, const size_t size)
           exit(EXIT_FAILURE);
     }
     it->itName = ctype;
-    it->itAlignment = size;
     itCalculateNameInfo(it);
     return it;
 }
@@ -817,11 +806,12 @@ itMakeCountType(void)
     ipc_type_t *it = itAlloc();
 
     it->itName = "mach_msg_type_number_t";
-    it->itInName = word_size_name;
-    it->itInNameStr = word_size_name_string;
-    it->itOutName = word_size_name;
-    it->itOutNameStr = word_size_name_string;
-    it->itSize = word_size_in_bits;
+    it->itInName = MACH_MSG_TYPE_INTEGER_32;
+    it->itInNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itOutName = MACH_MSG_TYPE_INTEGER_32;
+    it->itOutNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itSize = sizeof_int * 8;
+    it->itAlignment = sizeof_int;
 
     itCalculateSizeInfo(it);
     itCalculateNameInfo(it);
@@ -834,11 +824,12 @@ itMakeNaturalType(const char *name)
     ipc_type_t *it = itAlloc();
 
     it->itName = name;
-    it->itInName = word_size_name;
-    it->itInNameStr = word_size_name_string;
-    it->itOutName = word_size_name;
-    it->itOutNameStr = word_size_name_string;
-    it->itSize = word_size_in_bits;
+    it->itInName = MACH_MSG_TYPE_INTEGER_32;
+    it->itInNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itOutName = MACH_MSG_TYPE_INTEGER_32;
+    it->itOutNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itSize = sizeof_int * 8;
+    it->itAlignment = sizeof_int;
 
     itCalculateSizeInfo(it);
     itCalculateNameInfo(it);
@@ -851,11 +842,12 @@ itMakePolyType(void)
     ipc_type_t *it = itAlloc();
 
     it->itName = "mach_msg_type_name_t";
-    it->itInName = word_size_name;
-    it->itInNameStr = word_size_name_string;
-    it->itOutName = word_size_name;
-    it->itOutNameStr = word_size_name_string;
-    it->itSize = word_size_in_bits;
+    it->itInName = MACH_MSG_TYPE_INTEGER_32;
+    it->itInNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itOutName = MACH_MSG_TYPE_INTEGER_32;
+    it->itOutNameStr = "MACH_MSG_TYPE_INTEGER_32";
+    it->itSize = sizeof_int * 8;
+    it->itAlignment = sizeof_int;
 
     itCalculateSizeInfo(it);
     itCalculateNameInfo(it);
@@ -873,6 +865,7 @@ itMakeDeallocType(void)
     it->itOutName = MACH_MSG_TYPE_BOOLEAN;
     it->itOutNameStr = "MACH_MSG_TYPE_BOOLEAN";
     it->itSize = 32;
+    it->itAlignment = sizeof_int;
 
     itCalculateSizeInfo(it);
     itCalculateNameInfo(it);
@@ -899,7 +892,8 @@ init_type(void)
     itByteType->itInNameStr = "MACH_MSG_TYPE_BYTE";
     itByteType->itOutName = MACH_MSG_TYPE_BYTE;
     itByteType->itOutNameStr = "MACH_MSG_TYPE_BYTE";
-    itByteType->itSize = 8;
+    itByteType->itSize = sizeof_char * 8;
+    itByteType->itAlignment = sizeof_char;
     itCalculateSizeInfo(itByteType);
     itCalculateNameInfo(itByteType);
 
@@ -909,7 +903,8 @@ init_type(void)
     itRetCodeType->itInNameStr = "MACH_MSG_TYPE_INTEGER_32";
     itRetCodeType->itOutName = MACH_MSG_TYPE_INTEGER_32;
     itRetCodeType->itOutNameStr = "MACH_MSG_TYPE_INTEGER_32";
-    itRetCodeType->itSize = 32;
+    itRetCodeType->itSize = sizeof_int * 8;
+    itRetCodeType->itAlignment = sizeof_int;
     itCalculateSizeInfo(itRetCodeType);
     itCalculateNameInfo(itRetCodeType);
 
@@ -919,7 +914,8 @@ init_type(void)
     itDummyType->itInNameStr = "MACH_MSG_TYPE_UNSTRUCTURED";
     itDummyType->itOutName = MACH_MSG_TYPE_UNSTRUCTURED;
     itDummyType->itOutNameStr = "MACH_MSG_TYPE_UNSTRUCTURED";
-    itDummyType->itSize = word_size_in_bits;
+    itDummyType->itSize = word_size * 8;
+    itDummyType->itAlignment = word_size;
     itCalculateSizeInfo(itDummyType);
     itCalculateNameInfo(itDummyType);
 
@@ -930,6 +926,7 @@ init_type(void)
     itRequestPortType->itOutName = MACH_MSG_TYPE_PORT_SEND;
     itRequestPortType->itOutNameStr = "MACH_MSG_TYPE_PORT_SEND";
     itRequestPortType->itSize = port_size_in_bits;
+    itRequestPortType->itAlignment = port_size;
     itCalculateSizeInfo(itRequestPortType);
     itCalculateNameInfo(itRequestPortType);
 
@@ -940,6 +937,7 @@ init_type(void)
     itZeroReplyPortType->itOutName = 0;
     itZeroReplyPortType->itOutNameStr = "0";
     itZeroReplyPortType->itSize = port_size_in_bits;
+    itZeroReplyPortType->itAlignment = port_size;
     itCalculateSizeInfo(itZeroReplyPortType);
     itCalculateNameInfo(itZeroReplyPortType);
 
@@ -950,6 +948,7 @@ init_type(void)
     itRealReplyPortType->itOutName = MACH_MSG_TYPE_PORT_SEND_ONCE;
     itRealReplyPortType->itOutNameStr = "MACH_MSG_TYPE_PORT_SEND_ONCE";
     itRealReplyPortType->itSize = port_size_in_bits;
+    itRealReplyPortType->itAlignment = port_size;
     itCalculateSizeInfo(itRealReplyPortType);
     itCalculateNameInfo(itRealReplyPortType);
 
@@ -1041,17 +1040,15 @@ itCheckIntType(identifier_t name, const ipc_type_t *it)
        error("argument %s isn't a proper integer", name);
 }
 void
-itCheckNaturalType(name, it)
-    identifier_t name;
-    ipc_type_t *it;
+itCheckNaturalType(identifier_t name, ipc_type_t *it)
 {
-    if ((it->itInName != word_size_name) ||
-       (it->itOutName != word_size_name) ||
+    if ((it->itInName != MACH_MSG_TYPE_INTEGER_32) ||
+       (it->itOutName != MACH_MSG_TYPE_INTEGER_32) ||
        (it->itNumber != 1) ||
-       (it->itSize != word_size_in_bits) ||
+       (it->itSize != sizeof_int * 8) ||
        !it->itInLine ||
        it->itDeallocate != d_NO ||
        !it->itStruct ||
        it->itVarArray)
-       error("argument %s should have been a %s", name, word_size_name_string);
+       error("argument %s should have been a MACH_MSG_TYPE_INTEGER_32", name);
 }
diff --git a/utils.c b/utils.c
index 2fb8c2e..e8aec9a 100644
--- a/utils.c
+++ b/utils.c
@@ -304,6 +304,10 @@ WriteFieldDeclPrim(FILE *file, const argument_t *arg,
 
     fprintf(file, "\t\tmach_msg_type_%st %s;\n",
            arg->argLongForm ? "long_" : "", arg->argTTName);
+    if (!arg->argLongForm && word_size > sizeof_mach_msg_type_t) {
+        /* Pad mach_msg_type_t in case we need alignment by more than its 
size. */
+       fprintf(file, "\t\tchar %s_pad[%d];\n", arg->argTTName, word_size - 
sizeof_mach_msg_type_t);
+    }
 
     if (it->itInLine && it->itVarArray)
     {
@@ -338,12 +342,16 @@ void
 WriteStructDecl(FILE *file, const argument_t *args, write_list_fn_t *func,
                u_int mask, const char *name)
 {
-    fprintf(file, "#pragma pack(push,%d)\n", word_size);
+    if (word_size == 4) {
+        fprintf(file, "#pragma pack(push,%d)\n", word_size);
+    }
     fprintf(file, "\ttypedef struct {\n");
     fprintf(file, "\t\tmach_msg_header_t Head;\n");
     WriteList(file, args, func, mask, "\n", "\n");
     fprintf(file, "\t} %s;\n", name);
-    fprintf(file, "#pragma pack(pop)\n");
+    if (word_size == 4) {
+        fprintf(file, "#pragma pack(pop)\n");
+    }
     fprintf(file, "\n");
 }
 
diff --git a/utils.h b/utils.h
index 4be4f4c..477ea74 100644
--- a/utils.h
+++ b/utils.h
@@ -39,6 +39,8 @@
   ({ __typeof__ (a) _a = (a); \
       __typeof__ (b) _b = (b); \
     _a > _b ? _b : _a; })
+#define ALIGN(x, alignment) \
+    ((x) + (alignment)-1) & ~((alignment)-1)
 
 typedef void   write_list_fn_t(FILE *file, const argument_t *arg);
 
-- 
2.39.1




reply via email to

[Prev in Thread] Current Thread [Next in Thread]