Re: [committed mig] Do not generate code dereferencing type-punned pointers
Hi David :)
thanks for cleaning up after me ;) (again and again...)
Quoting David Michael (2015-02-18 05:39:46)
On Sun, Feb 15, 2015 at 11:09 AM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
* utils.c (WriteFieldDeclPrim): Generate a union with an additional
pointer field for variable-length arrays.
This makes GDB's awk script go haywire because it doesn't know how to
deal with unions. The following is a workaround to get it building
again, but I'm not sure of its correctness. Can someone more
knowledgeable than me check on this?
Not that I'm claiming to know why gdb does what it does, but Samuel
asked me to look into this. Here goes:
For some reason (proxying?), gdb uses mig to create server-side stubs
for the *reply*-half of some protocols. It then uses an awk-script to
insert the following code into the server-stubs:
if (In0P-Head.msgh_size == sizeof (Reply)
! (In0P-Head.msgh_bits MACH_MSGH_BITS_COMPLEX)
! BAD_TYPECHECK(In0P-return_codeType, return_codeCheck)
In0P-return_code != 0)
/* Error return, only the error code argument is passed. */
{
kern_return_t (*sfun)(mach_port_t, kern_return_t, int, data_t,
mach_msg_type_number_t, data_t, mach_msg_type_number_t) =
S_proc_getprocinfo_reply;
OutP-RetCode = (*(kern_return_t (*)(mach_port_t, kern_return_t))sfun)
(In0P-Head.msgh_request_port, In0P-return_code);
return;
}
This is inserted *before* any type checks. The awk script has this to
say:
# The first args type checking statement; we need to insert our chunk of
# code that bypasses all the type checks if this is an error return, after
# which we're done until we get to the next function. Handily, the size
# of mig's Reply structure is also the size of the alternate Request
# structure that we want to check for.
[...]
# Force the function into a type that only takes the first two args, via
# the temp variable SFUN (is there another way to correctly do this cast?).
# This is possibly bogus, but easier than supplying bogus values for all
# the other args (we can't just pass 0 for them, as they might not be scalar).
I don't see why it bothers with SFUN in the first place. It could just do
OutP-RetCode = (*(kern_return_t (*)(mach_port_t, kern_return_t))
S_proc_getprocinfo_reply) (In0P-Head.msgh_request_port, In0P-return_code);
right? If so, the main reason for parsing the struct is gone, and we
could simplify the script a lot and thus making it less likely to
break in the future. Getting rid of it entirely would be nice though.
A word about the patch. It seems to cheat by assuming `data_t' as the
type as soon at it sees a union. That shouldn't matter since the type
is only used for the stupid cast. But the better way would be to just
get rid of the parsing in the first place, or the whole script when
we're at it.
Justus
Re: [committed mig] Do not generate code dereferencing type-punned pointers
On Mon, Mar 2, 2015 at 11:03 AM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
Hi David :)
thanks for cleaning up after me ;) (again and again...)
Quoting David Michael (2015-02-18 05:39:46)
On Sun, Feb 15, 2015 at 11:09 AM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
* utils.c (WriteFieldDeclPrim): Generate a union with an additional
pointer field for variable-length arrays.
This makes GDB's awk script go haywire because it doesn't know how to
deal with unions. The following is a workaround to get it building
again, but I'm not sure of its correctness. Can someone more
knowledgeable than me check on this?
Not that I'm claiming to know why gdb does what it does, but Samuel
asked me to look into this.
Thanks for looking into it.
For some reason (proxying?), gdb uses mig to create server-side stubs
for the *reply*-half of some protocols. It then uses an awk-script to
insert the following code into the server-stubs:
if (In0P-Head.msgh_size == sizeof (Reply)
! (In0P-Head.msgh_bits MACH_MSGH_BITS_COMPLEX)
! BAD_TYPECHECK(In0P-return_codeType, return_codeCheck)
In0P-return_code != 0)
/* Error return, only the error code argument is passed. */
{
kern_return_t (*sfun)(mach_port_t, kern_return_t, int, data_t,
mach_msg_type_number_t, data_t, mach_msg_type_number_t) =
S_proc_getprocinfo_reply;
OutP-RetCode = (*(kern_return_t (*)(mach_port_t, kern_return_t))sfun)
(In0P-Head.msgh_request_port, In0P-return_code);
return;
}
This is inserted *before* any type checks. The awk script has this to
say:
# The first args type checking statement; we need to insert our chunk of
# code that bypasses all the type checks if this is an error return, after
# which we're done until we get to the next function. Handily, the size
# of mig's Reply structure is also the size of the alternate Request
# structure that we want to check for.
[...]
# Force the function into a type that only takes the first two args, via
# the temp variable SFUN (is there another way to correctly do this cast?).
# This is possibly bogus, but easier than supplying bogus values for all
# the other args (we can't just pass 0 for them, as they might not be scalar).
I don't see why it bothers with SFUN in the first place. It could just do
OutP-RetCode = (*(kern_return_t (*)(mach_port_t, kern_return_t))
S_proc_getprocinfo_reply) (In0P-Head.msgh_request_port, In0P-return_code);
That was my first thought as well, but the comment is there another
way to correctly do this cast? made me think I was missing some
subtlety. Thomas took care of updating this last time mig was
modified; maybe he is familiar with its design choices?
right? If so, the main reason for parsing the struct is gone, and we
could simplify the script a lot and thus making it less likely to
break in the future. Getting rid of it entirely would be nice though.
A word about the patch. It seems to cheat by assuming `data_t' as the
type as soon at it sees a union. That shouldn't matter since the type
is only used for the stupid cast. But the better way would be to just
get rid of the parsing in the first place, or the whole script when
we're at it.
Yes, I'd be in favor of anything that makes the build process less
dependent on the exact mig output. I don't know enough about what gdb
is doing to remove this script entirely, but I'll see if it still
works when using a direct cast instead of all the argument parsing.
Thanks.
David
Re: [committed mig] Do not generate code dereferencing type-punned pointers
On Sun, Feb 15, 2015 at 11:09 AM, Justus Winter
4win...@informatik.uni-hamburg.de wrote:
* utils.c (WriteFieldDeclPrim): Generate a union with an additional
pointer field for variable-length arrays.
This makes GDB's awk script go haywire because it doesn't know how to
deal with unions. The following is a workaround to get it building
again, but I'm not sure of its correctness. Can someone more
knowledgeable than me check on this?
Thanks.
David
--- gdb/reply_mig_hack.awk
+++ gdb/reply_mig_hack.awk
@@ -68,6 +68,11 @@
print; next;
}
+parse_phase == 4 /^[ \t]*union {/ {
+ parse_phase = 4.5;
+ print; next;
+}
+
parse_phase == 4 {
# The value field for an argument.
arg_name[num_args] = $2;
@@ -78,6 +83,22 @@
print; next;
}
+parse_phase == 4.5 {
+ arg_name[num_args] = $2;
+ sub (/[][0-9;]*$/, , arg_name[num_args]);
+ arg_type[num_args] = data_t;
+ arg_name[num_args + 1] = arg_name[num_args] Cnt;
+ arg_type[num_args + 1] = mach_msg_type_number_t;
+ num_args += 2;
+ parse_phase = 4.6;
+ print; next;
+}
+
+parse_phase == 4.6 /}/ {
+ parse_phase = 3;
+ print; next;
+}
+
parse_phase == 5 /^[ \t]*(auto |static |)const mach_msg_type_t/ {
# The type check structure for an argument.
arg_check_name[num_checks] = $(NF - 2);
