JDevlieghere accepted this revision.
JDevlieghere added a comment.
This revision is now accepted and ready to land.
Thanks Jim. This LGTM.
================
Comment at:
lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCTrampolineHandler.cpp:637-643
// Also we will use the version of the lookup code that doesn't rely on the
// stret version of the function.
- m_lookup_implementation_function_code =
- g_lookup_implementation_no_stret_function_code;
+ m_lookup_implementation_function_code.append(
+ g_lookup_implementation_no_stret_function_code);
} else {
- m_lookup_implementation_function_code =
- g_lookup_implementation_with_stret_function_code;
+ m_lookup_implementation_function_code.append(
+ g_lookup_implementation_with_stret_function_code);
----------------
jingham wrote:
> JDevlieghere wrote:
> > I don't understand why we need to have two versions of the code. IIUC, if
> > there's a "stret return lookup function", then if we pass `is_stret ==
> > true` we'll use it. Otherwise we'll unconditionally use the straight
> > lookup. Why do we need two variants of the code at all? Can't we pass false
> > to `is_stret` and achieve the same result with the
> > `g_lookup_implementation_with_stret_function_code` variant?
> On the systems that don't use stret returns,
> class_getMethodImplementation_stret doesn't exist. So we would either have
> to have two versions of that part of the code, or passing function pointers
> to the two functions (making them the same in the no _stret case) or do some
> dlsym type ugliness.
>
> This solution is straightforward, and reducing complexity in these Utility
> functions is a virtue. Having two copies of all the code was a bit bogus,
> but this solution is straightforward and easy to debug.
Thanks. I saw the `m_impl_stret_fn_addr` and figured we were passing in the
function pointer ourselves. Makes sense.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D123421/new/
https://reviews.llvm.org/D123421
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
