[PATCH] D50144: Add Windows support for the GNUstep Objective-C ABI V2.

John McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 9 22:33:29 PDT 2018


rjmccall accepted this revision.
rjmccall added a comment.

LGTM.



================
Comment at: lib/CodeGen/CGObjCGNU.cpp:915
+    return name;
+  }
   /// The GCC ABI superclass message lookup function.  Takes a pointer to a
----------------
theraven wrote:
> rjmccall wrote:
> > theraven wrote:
> > > rjmccall wrote:
> > > > Can this section-names cleanup also just be in a separate patch?
> > > This is difficult to extract, because it's mostly needed for the COFF part where we need to modify the section names.  For ELF, it was fine to keep them as separate `const char*`s
> > I don't mind if the extracted patch seems unmotivated on its own, but I do think it should be a separate patch.
> None of the changes to do this are in a separate commit, and they touch enough of the code that it's very difficult to separate them without ending up with conflicts in this branch (which touches the code surrounding the changes in multiple places).  I cannot extract the code in such a way that this patch would cleanly rebase on top, because various things (e.g. the null section code) needed restructuring to work with Windows and touch this logic.
Alright, I won't insist.


================
Comment at: lib/CodeGen/CGObjCGNU.cpp:3817
+  if (isRethrow && CGF.CGM.getTarget().getTriple().isWindowsMSVCEnvironment()) {
+    CGF.EmitRuntimeCallOrInvoke(ExceptionReThrowFn).setDoesNotReturn();
+  }
----------------
theraven wrote:
> rjmccall wrote:
> > theraven wrote:
> > > rjmccall wrote:
> > > > You're sure here that the static information aligns with the dynamic?
> > > I'm not sure I understand this question.
> > Your new code path no longer uses `ExceptionAsObject`, which is our static notion of the current exception value.  Instead, you call a runtime function which presumably relies on a dynamically-stored current exception value.  I'm just asking if, in this lexical position, you're definitely rethrowing the right exception.
> Ah, I see.  This code path is hit only as a `@throw;`, not a `@throw obj` (in the latter case, there is a non-null return from `S.getThrowExpr()`).  In this case, the value of ExceptionAsObject may not be useful. In the case of a catchall, this is a load from a stack location with no corresponding store.  The Windows unwind logic keeps the object on the stack during funclet execution and then continues the unwind at the end, without ever providing this frame's funclets with a pointer to it.  That's probably not very obvious, so I've added an explanatory comment.
Thanks.


Repository:
  rC Clang

https://reviews.llvm.org/D50144





More information about the cfe-commits mailing list