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

RFC: Support non-standard extension (call via casted function pointer)



Hello gcc developers,

as discussed in https://ghc.haskell.org/trac/ghc/ticket/11395 (and
forwarded as PR c/69221), ghc generates non-compliant C code that is not
compiled as intended on m68k. This is because its internal Cmm (a C--
dialect) to C compiler needs to declare external functions (no varargs)
without fixing the type. It currently uses "unspecified-function-type*
function();" which is a quite good fit, because the argument list is
still left open, but it fixes the return type to some pointer type.
Before calling that function, the function address is put into a
function pointer of the correct type and invoked via that pointer. This
model currently works fine (possibly by accident) on all architectures
ghc supports except m68k.

I developed a gcc patch that does not change the code generation for
conforming programs but fixes this non-conforming use-case by always
taking the actual return type in the call expression into account, even
if the function declaration to be called is known. Please comment
whether such a patch has any chance to be applied to either gcc upstream
or gcc in Debian.

Regards,
  Michael Karcher
>From 50c0af571f829e54cb3274c05d7be3da41114162 Mon Sep 17 00:00:00 2001
From: Michael Karcher <debian@mkarcher.dialup.fu-berlin.de>
Date: Mon, 25 Jan 2016 22:07:37 +0100
Subject: [PATCH 1/1] Extend gcc on m68k to allow calling incorrectly declared
 externals through a function pointer of the correct type.

There are users of gcc (one of them is ghc) that expect to be able to declare
external functions with a dummy type (ghc currently uses
"ptr-to-function fn();") and call them through a function pointer of the
actual type of this function. I reported a ticket on ghc[1] which was
forwarded as PR c/69221 and subsequently closed as INVALID. While I agree
that the use case is way off the standard, and gcc emits the deserved
warnings about incompatible types, this extension seems useful, and at
least for m68k does no harm for conforming code.
---
 gcc/config/m68k/m68k.c                     | 40 ++++++++++++++++++++++--------
 gcc/testsuite/gcc.target/m68k/cast-fptr1.c | 11 ++++++++
 gcc/testsuite/gcc.target/m68k/cast-fptr2.c | 12 +++++++++
 3 files changed, 53 insertions(+), 10 deletions(-)
 create mode 100644 gcc/testsuite/gcc.target/m68k/cast-fptr1.c
 create mode 100644 gcc/testsuite/gcc.target/m68k/cast-fptr2.c

diff --git a/gcc/config/m68k/m68k.c b/gcc/config/m68k/m68k.c
index 03f474e..da7a5f0 100644
--- a/gcc/config/m68k/m68k.c
+++ b/gcc/config/m68k/m68k.c
@@ -5288,22 +5288,42 @@ m68k_function_value (const_tree valtype, const_tree func ATTRIBUTE_UNUSED)
 
   /* If the function returns a pointer, push that into %a0.  */
   if (func && POINTER_TYPE_P (TREE_TYPE (TREE_TYPE (func))))
+  {
     /* For compatibility with the large body of existing code which
        does not always properly declare external functions returning
        pointer types, the m68k/SVR4 convention is to copy the value
        returned for pointer functions from a0 to d0 in the function
        epilogue, so that callers that have neglected to properly
        declare the callee can still find the correct return value in
-       d0.  */
-    return gen_rtx_PARALLEL
-      (mode,
-       gen_rtvec (2,
-		  gen_rtx_EXPR_LIST (VOIDmode,
-				     gen_rtx_REG (mode, A0_REG),
-				     const0_rtx),
-		  gen_rtx_EXPR_LIST (VOIDmode,
-				     gen_rtx_REG (mode, D0_REG),
-				     const0_rtx)));
+       d0.
+       expand_call uses the first register in the PARALLEL rtx.  Order
+       the registers such that expand_call uses the same register as
+       if func was not given.  */
+    if (POINTER_TYPE_P (valtype))
+    {
+      return gen_rtx_PARALLEL
+        (mode,
+         gen_rtvec (2,
+                    gen_rtx_EXPR_LIST (VOIDmode,
+                                       gen_rtx_REG (mode, A0_REG),
+                                       const0_rtx),
+                    gen_rtx_EXPR_LIST (VOIDmode,
+                                       gen_rtx_REG (mode, D0_REG),
+                                       const0_rtx)));
+    }
+    else
+    {
+      return gen_rtx_PARALLEL
+        (mode,
+         gen_rtvec (2,
+                    gen_rtx_EXPR_LIST (VOIDmode,
+                                       gen_rtx_REG (mode, D0_REG),
+                                       const0_rtx),
+                    gen_rtx_EXPR_LIST (VOIDmode,
+                                       gen_rtx_REG (mode, A0_REG),
+                                       const0_rtx)));
+    }
+  }
   else if (POINTER_TYPE_P (valtype))
     return gen_rtx_REG (mode, A0_REG);
   else
diff --git a/gcc/testsuite/gcc.target/m68k/cast-fptr1.c b/gcc/testsuite/gcc.target/m68k/cast-fptr1.c
new file mode 100644
index 0000000..3116167
--- /dev/null
+++ b/gcc/testsuite/gcc.target/m68k/cast-fptr1.c
@@ -0,0 +1,11 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wno-incompatible-pointer-types" } */
+/* { dg-final { scan-assembler "cmp.l %d0,%d1" } } */
+
+void *extfn();
+
+int main(void)
+{
+  int(*f)(void) = extfn;
+  return f() == 42;
+}
diff --git a/gcc/testsuite/gcc.target/m68k/cast-fptr2.c b/gcc/testsuite/gcc.target/m68k/cast-fptr2.c
new file mode 100644
index 0000000..1aff7eb
--- /dev/null
+++ b/gcc/testsuite/gcc.target/m68k/cast-fptr2.c
@@ -0,0 +1,12 @@
+/* { dg-do compile } */
+/* { dg-options "-O -Wno-incompatible-pointer-types" } */
+/* { dg-final { scan-assembler "cmp.l #foo,%a0" } } */
+
+int extfn();
+extern char foo[];
+
+int main(void)
+{
+  void*(*f)(void) = extfn;
+  return f() == &foo;
+}
-- 
2.7.0.rc3


Reply to: