Hello ports!

In OpenBSD macppc, clang and gcc use incompatible ABIs to return small
structs: gcc defaults to -msvr4-struct-return, but clang always acts
like gcc -maix-struct-return.  This causes crashes at runtime: for
example, clang code can't call libxcb (in Xenocara built by gcc).

This diff for ports-clang adds clang -msvr4-struct-return, and uses it
by default for OpenBSD powerpc.  I proposed the diff upstream at
https://reviews.llvm.org/D73290 (where Mark Millard reported that the
diff seems to work so far on FreeBSD).

My iMac G3 (1x 400 MHz PowerPC 750, 512 M RAM) with OpenBSD macppc is
trying to build devel/llvm with this diff, but after 45 hours, has built
less than half of 4470 targets.  (If I had not broken my PowerBook G4,
it might have completed the build by now.)

My amd64 desktop built devel/llvm with this diff days ago.  I can now
cross-compile, where I run 'clang -c' on amd64, scp the *.o to my iMac,
then use base-gcc to link.

The attachment "sret-examples.tar.gz" contains my tiny example programs
and a Makefile set to use ports-clang as CLANG and base-gcc as GCC.  Run
'make' to build smain-[cg][cg] and xcbtest.  The 4 smain-* try different
combinations of clang and gcc; they should show 12 ok tests.  The
xcbtest requires an X server and should show terminal output like,

$ ./xcbtest                                                          
conn = 0xb16f3033000
cookie.sequence = 1
reply = 0xb173fd873e0

where conn and reply should not be zero.  (This output is from amd64;
I can't run xcbtest on my iMac because X11 isn't working.)

On my iMac, 'make CLANG=/usr/bin/clang' uses a base-clang without this
diff: smain-cg (clang calling gcc) fails most tests and smain-gc (gcc
calling clang) crashes.

To cross-compile the clang parts from amd64, I do
$ make clang CLANG='/usr/local/bin/clang -target ppc-openbsd'
and scp the *.o files to my iMac.  This fixes smain-cg and smain-gc.

If you want to build devel/llvm on macppc, I suggest a G4 or G5 with at
least 1024M of RAM.  (My iMac with 512M RAM took over 600M swap to
compile ASTDumper; the build seemed to freeze for a few hours.)  If you
build it, you might try 'make COMPILER=ports-clang' on some ports.  If
a port so built does crash at runtime, it might be hard to know whether
a struct return or some other clang bug caused the crash.

I'm still waiting for my iMac to build this.  Later, I might try to
apply this diff to base-clang.  I won't commit this diff to ports-clang
before I know what to do with base-clang.

--George

Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/llvm/Makefile,v
retrieving revision 1.240
diff -u -p -r1.240 Makefile
--- Makefile    7 Jan 2020 22:59:43 -0000       1.240
+++ Makefile    24 Jan 2020 17:37:47 -0000
@@ -18,7 +18,7 @@ PKGSPEC-main =        llvm-=${LLVM_V}
 PKGNAME-main = llvm-${LLVM_V}
 PKGNAME-python =       py-llvm-${LLVM_V}
 PKGNAME-lldb = lldb-${LLVM_V}
-REVISION-main =        5
+REVISION-main =        6
 REVISION-lldb= 0
 
 CATEGORIES =   devel
Index: patches/patch-tools_clang_include_clang_Driver_Options_td
===================================================================
RCS file: 
/cvs/ports/devel/llvm/patches/patch-tools_clang_include_clang_Driver_Options_td,v
retrieving revision 1.19
diff -u -p -r1.19 patch-tools_clang_include_clang_Driver_Options_td
--- patches/patch-tools_clang_include_clang_Driver_Options_td   6 Jul 2019 
15:06:36 -0000       1.19
+++ patches/patch-tools_clang_include_clang_Driver_Options_td   24 Jan 2020 
17:37:47 -0000
@@ -2,6 +2,7 @@ $OpenBSD: patch-tools_clang_include_clan
 
 - Add ret protctor options as no-ops.
 - Improve the X86FixupGadgets pass
+- Add -msvr4-struct-return for powerpc.
 - Alias the command line parameter -p to -pg.
 - implement -msave-args in clang/llvm, like the sun did for gcc
 
@@ -23,7 +24,20 @@ Index: tools/clang/include/clang/Driver/
  def fstandalone_debug : Flag<["-"], "fstandalone-debug">, Group<f_Group>, 
Flags<[CoreOption]>,
    HelpText<"Emit full debug info for all types used by the program">;
  def fno_standalone_debug : Flag<["-"], "fno-standalone-debug">, 
Group<f_Group>, Flags<[CoreOption]>,
-@@ -2500,7 +2508,7 @@ def pthreads : Flag<["-"], "pthreads">;
+@@ -2230,6 +2238,12 @@ def mlongcall: Flag<["-"], "mlongcall">,
+     Group<m_ppc_Features_Group>;
+ def mno_longcall : Flag<["-"], "mno-longcall">,
+     Group<m_ppc_Features_Group>;
++def maix_struct_return : Flag<["-"], "maix-struct-return">,
++  Group<m_Group>, Flags<[CC1Option]>,
++  HelpText<"Return all structs in memory (PPC32 only)">;
++def msvr4_struct_return : Flag<["-"], "msvr4-struct-return">,
++  Group<m_Group>, Flags<[CC1Option]>,
++  HelpText<"Return small structs in registers (PPC32 only)">;
+ 
+ def mvx : Flag<["-"], "mvx">, Group<m_Group>;
+ def mno_vx : Flag<["-"], "mno-vx">, Group<m_Group>;
+@@ -2500,7 +2514,7 @@ def pthreads : Flag<["-"], "pthreads">;
  def pthread : Flag<["-"], "pthread">, Flags<[CC1Option]>,
    HelpText<"Support POSIX threads in generated code">;
  def no_pthread : Flag<["-"], "no-pthread">, Flags<[CC1Option]>;
@@ -32,7 +46,7 @@ Index: tools/clang/include/clang/Driver/
  def pie : Flag<["-"], "pie">;
  def read__only__relocs : Separate<["-"], "read_only_relocs">;
  def remap : Flag<["-"], "remap">;
-@@ -2949,6 +2957,8 @@ def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Featur
+@@ -2949,6 +2963,8 @@ def mshstk : Flag<["-"], "mshstk">, Group<m_x86_Featur
  def mno_shstk : Flag<["-"], "mno-shstk">, Group<m_x86_Features_Group>;
  def mretpoline_external_thunk : Flag<["-"], "mretpoline-external-thunk">, 
Group<m_x86_Features_Group>;
  def mno_retpoline_external_thunk : Flag<["-"], 
"mno-retpoline-external-thunk">, Group<m_x86_Features_Group>;
Index: patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
===================================================================
RCS file: patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
diff -N patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp
--- /dev/null   1 Jan 1970 00:00:00 -0000
+++ patches/patch-tools_clang_lib_CodeGen_TargetInfo_cpp        24 Jan 2020 
17:37:47 -0000
@@ -0,0 +1,128 @@
+$OpenBSD$
+
+Add -msvr4-struct-return for powerpc.
+
+Index: tools/clang/lib/CodeGen/TargetInfo.cpp
+--- tools/clang/lib/CodeGen/TargetInfo.cpp.orig
++++ tools/clang/lib/CodeGen/TargetInfo.cpp
+@@ -4092,22 +4092,39 @@ namespace {
+ /// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ELF (SVR4) ABI information.
+ class PPC32_SVR4_ABIInfo : public DefaultABIInfo {
+   bool IsSoftFloatABI;
++  bool IsRetSmallStructInRegABI;
+ 
+   CharUnits getParamTypeAlignment(QualType Ty) const;
+ 
+ public:
+-  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI)
+-      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI) {}
++  PPC32_SVR4_ABIInfo(CodeGen::CodeGenTypes &CGT, bool SoftFloatABI,
++                     bool RetSmallStructInRegABI)
++      : DefaultABIInfo(CGT), IsSoftFloatABI(SoftFloatABI),
++        IsRetSmallStructInRegABI(RetSmallStructInRegABI) {}
+ 
++  ABIArgInfo classifyReturnType(QualType RetTy) const;
++
++  void computeInfo(CGFunctionInfo &FI) const override {
++    if (!getCXXABI().classifyReturnType(FI))
++      FI.getReturnInfo() = classifyReturnType(FI.getReturnType());
++    for (auto &I : FI.arguments())
++      I.info = classifyArgumentType(I.type);
++  }
++
+   Address EmitVAArg(CodeGenFunction &CGF, Address VAListAddr,
+                     QualType Ty) const override;
+ };
+ 
+ class PPC32TargetCodeGenInfo : public TargetCodeGenInfo {
+ public:
+-  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI)
+-      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI)) {}
++  PPC32TargetCodeGenInfo(CodeGenTypes &CGT, bool SoftFloatABI,
++                         bool RetSmallStructInRegABI)
++      : TargetCodeGenInfo(new PPC32_SVR4_ABIInfo(CGT, SoftFloatABI,
++                                                 RetSmallStructInRegABI)) {}
+ 
++  static bool isStructReturnInRegABI(const llvm::Triple &Triple,
++                                     const CodeGenOptions &Opts);
++
+   int getDwarfEHStackPointer(CodeGen::CodeGenModule &M) const override {
+     // This is recovered from gcc output.
+     return 1; // r1 is the dedicated stack pointer
+@@ -4142,6 +4159,34 @@ CharUnits PPC32_SVR4_ABIInfo::getParamTypeAlignment(Qu
+   return CharUnits::fromQuantity(4);
+ }
+ 
++ABIArgInfo PPC32_SVR4_ABIInfo::classifyReturnType(QualType RetTy) const {
++  uint64_t Size;
++
++  // -msvr4-struct-return puts small aggregates in GPR3 and GPR4.
++  if (isAggregateTypeForABI(RetTy) && IsRetSmallStructInRegABI &&
++      (Size = getContext().getTypeSize(RetTy)) <= 64) {
++    // System V ABI (1995), page 3-22, specified:
++    // > A structure or union whose size is less than or equal to 8 bytes
++    // > shall be returned in r3 and r4, as if it were first stored in the
++    // > 8-byte aligned memory area and then the low addressed word were
++    // > loaded into r3 and the high-addressed word into r4.  Bits beyond
++    // > the last member of the structure or union are not defined.
++    //
++    // GCC for big-endian PPC32 inserts the pad before the first member,
++    // not "beyond the last member" of the struct.  To stay compatible
++    // with GCC, we coerce the struct to an integer of the same size.
++    // LLVM will extend it and return i32 in r3, or i64 in r3:r4.
++    if (Size == 0)
++      return ABIArgInfo::getIgnore();
++    else {
++      llvm::Type *CoerceTy = llvm::Type::getIntNTy(getVMContext(), Size);
++      return ABIArgInfo::getDirect(CoerceTy);
++    }
++  }
++
++  return DefaultABIInfo::classifyReturnType(RetTy);
++}
++
+ // TODO: this implementation is now likely redundant with
+ // DefaultABIInfo::EmitVAArg.
+ Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction &CGF, Address VAList,
+@@ -4299,6 +4344,25 @@ Address PPC32_SVR4_ABIInfo::EmitVAArg(CodeGenFunction 
+   return Result;
+ }
+ 
++bool PPC32TargetCodeGenInfo::isStructReturnInRegABI(
++    const llvm::Triple &Triple, const CodeGenOptions &Opts) {
++  assert(Triple.getArch() == llvm::Triple::ppc);
++
++  switch (Opts.getStructReturnConvention()) {
++  case CodeGenOptions::SRCK_Default:
++    break;
++  case CodeGenOptions::SRCK_OnStack: // -maix-struct-return
++    return false;
++  case CodeGenOptions::SRCK_InRegs: // -msvr4-struct-return
++    return true;
++  }
++
++  if (Triple.isOSBinFormatELF() && !Triple.isOSLinux())
++    return true;
++
++  return false;
++}
++
+ bool
+ PPC32TargetCodeGenInfo::initDwarfEHRegSizeTable(CodeGen::CodeGenFunction &CGF,
+                                                 llvm::Value *Address) const {
+@@ -9330,9 +9394,13 @@ const TargetCodeGenInfo &CodeGenModule::getTargetCodeG
+     return SetCGInfo(new ARMTargetCodeGenInfo(Types, Kind));
+   }
+ 
+-  case llvm::Triple::ppc:
++  case llvm::Triple::ppc: {
++    bool RetSmallStructInRegABI =
++        PPC32TargetCodeGenInfo::isStructReturnInRegABI(Triple, CodeGenOpts);
+     return SetCGInfo(
+-        new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft"));
++        new PPC32TargetCodeGenInfo(Types, CodeGenOpts.FloatABI == "soft",
++                                   RetSmallStructInRegABI));
++  }
+   case llvm::Triple::ppc64:
+     if (Triple.isOSBinFormatELF()) {
+       PPC64_SVR4_ABIInfo::ABIKind Kind = PPC64_SVR4_ABIInfo::ELFv1;
Index: patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
===================================================================
RCS file: 
/cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v
retrieving revision 1.11
diff -u -p -r1.11 patch-tools_clang_lib_Driver_ToolChains_Clang_cpp
--- patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp   7 Jan 2020 
22:59:43 -0000       1.11
+++ patches/patch-tools_clang_lib_Driver_ToolChains_Clang_cpp   24 Jan 2020 
17:37:47 -0000
@@ -1,5 +1,6 @@
 $OpenBSD: patch-tools_clang_lib_Driver_ToolChains_Clang_cpp,v 1.11 2020/01/07 
22:59:43 jca Exp $
 
+- Add -msvr4-struct-return for powerpc.
 - Make LLVM create strict aligned code for OpenBSD/arm64.
 - Disable -fstrict-aliasing per default on OpenBSD.
 - Enable -fwrapv by default
@@ -33,7 +34,27 @@ $OpenBSD: patch-tools_clang_lib_Driver_T
 Index: tools/clang/lib/Driver/ToolChains/Clang.cpp
 --- tools/clang/lib/Driver/ToolChains/Clang.cpp.orig
 +++ tools/clang/lib/Driver/ToolChains/Clang.cpp
-@@ -3899,9 +3899,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -3870,6 +3870,19 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+     CmdArgs.push_back(A->getValue());
+   }
+ 
++  if (Arg *A = Args.getLastArg(options::OPT_maix_struct_return,
++                               options::OPT_msvr4_struct_return)) {
++    if (TC.getArch() != llvm::Triple::ppc) {
++      D.Diag(diag::err_drv_unsupported_opt_for_target)
++          << A->getSpelling() << RawTriple.str();
++    } else if (A->getOption().matches(options::OPT_maix_struct_return)) {
++      CmdArgs.push_back("-maix-struct-return");
++    } else {
++      assert(A->getOption().matches(options::OPT_msvr4_struct_return));
++      CmdArgs.push_back("-msvr4-struct-return");
++    }
++  }
++
+   if (Arg *A = Args.getLastArg(options::OPT_fpcc_struct_return,
+                                options::OPT_freg_struct_return)) {
+     if (TC.getArch() != llvm::Triple::x86) {
+@@ -3899,9 +3912,12 @@ void Clang::ConstructJob(Compilation &C, const JobActi
        OFastEnabled ? options::OPT_Ofast : options::OPT_fstrict_aliasing;
    // We turn strict aliasing off by default if we're in CL mode, since MSVC
    // doesn't do any TBAA.
@@ -48,7 +69,7 @@ Index: tools/clang/lib/Driver/ToolChains
      CmdArgs.push_back("-relaxed-aliasing");
    if (!Args.hasFlag(options::OPT_fstruct_path_tbaa,
                      options::OPT_fno_struct_path_tbaa))
-@@ -4527,7 +4530,8 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -4527,7 +4543,8 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                                        options::OPT_fno_strict_overflow)) {
      if (A->getOption().matches(options::OPT_fno_strict_overflow))
        CmdArgs.push_back("-fwrapv");
@@ -58,7 +79,7 @@ Index: tools/clang/lib/Driver/ToolChains
  
    if (Arg *A = Args.getLastArg(options::OPT_freroll_loops,
                                 options::OPT_fno_reroll_loops))
-@@ -4544,9 +4548,46 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -4544,9 +4561,46 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                     false))
      CmdArgs.push_back(Args.MakeArgString("-mspeculative-load-hardening"));
  
@@ -106,7 +127,7 @@ Index: tools/clang/lib/Driver/ToolChains
    // Translate -mstackrealign
    if (Args.hasFlag(options::OPT_mstackrealign, options::OPT_mno_stackrealign,
                     false))
-@@ -5029,6 +5070,18 @@ void Clang::ConstructJob(Compilation &C, const JobActi
+@@ -5029,6 +5083,18 @@ void Clang::ConstructJob(Compilation &C, const JobActi
                                       options::OPT_fno_rewrite_imports, false);
    if (RewriteImports)
      CmdArgs.push_back("-frewrite-imports");
Index: patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
===================================================================
RCS file: 
/cvs/ports/devel/llvm/patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp,v
retrieving revision 1.3
diff -u -p -r1.3 patch-tools_clang_lib_Frontend_CompilerInvocation_cpp
--- patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp       6 Jul 
2019 15:06:36 -0000       1.3
+++ patches/patch-tools_clang_lib_Frontend_CompilerInvocation_cpp       24 Jan 
2020 17:37:47 -0000
@@ -18,6 +18,8 @@ essentially gadget free, leaving only th
 jumping into the instruction stream partway through other instructions. Work to
 remove these gadgets will continue through other mechanisms.
 
+Add -msvr4-struct-return for powerpc.
+
 Index: tools/clang/lib/Frontend/CompilerInvocation.cpp
 --- tools/clang/lib/Frontend/CompilerInvocation.cpp.orig
 +++ tools/clang/lib/Frontend/CompilerInvocation.cpp
@@ -30,3 +32,25 @@ Index: tools/clang/lib/Frontend/Compiler
    if (Arg *A = Args.getLastArg(OPT_mstack_probe_size)) {
      StringRef Val = A->getValue();
      unsigned StackProbeSize = Opts.StackProbeSize;
+@@ -1197,11 +1199,18 @@ static bool ParseCodeGenArgs(CodeGenOptions &Opts, Arg
+       Diags.Report(diag::err_drv_invalid_value) << A->getAsString(Args) << 
Val;
+   }
+ 
+-  if (Arg *A = Args.getLastArg(OPT_fpcc_struct_return, 
OPT_freg_struct_return)) {
+-    if (A->getOption().matches(OPT_fpcc_struct_return)) {
++  // X86_32 has -fppc-struct-return and -freg-struct-return.
++  // PPC32 has -maix-struct-return and -msvr4-struct-return.
++  if (Arg *A =
++          Args.getLastArg(OPT_fpcc_struct_return, OPT_freg_struct_return,
++                          OPT_maix_struct_return, OPT_msvr4_struct_return)) {
++    const Option &O = A->getOption();
++    if (O.matches(OPT_fpcc_struct_return) ||
++        O.matches(OPT_maix_struct_return)) {
+       Opts.setStructReturnConvention(CodeGenOptions::SRCK_OnStack);
+     } else {
+-      assert(A->getOption().matches(OPT_freg_struct_return));
++      assert(O.matches(OPT_freg_struct_return) ||
++             O.matches(OPT_msvr4_struct_return));
+       Opts.setStructReturnConvention(CodeGenOptions::SRCK_InRegs);
+     }
+   }

Attachment: sret-examples.tar.gz
Description: Binary data

Reply via email to