> On Jan 2, 2018, at 5:09 PM, Jim Ingham <[email protected]> wrote: > > > >> On Jan 2, 2018, at 3:11 PM, Joe Groff via swift-lldb-dev >> <[email protected]> wrote: >> >> >> >>> On Jan 2, 2018, at 2:50 PM, Jim Ingham <[email protected]> wrote: >>> >>> >>> >>>> On Jan 2, 2018, at 10:30 AM, Joe Groff via swift-lldb-dev >>>> <[email protected]> wrote: >>>> >>>> Hi lldb folks, I wanted to run this patch by you all before merging: >>>> >>>> https://github.com/apple/swift/pull/13585 >>>> >>>> I noticed that in code generation we were generating `swift_willThrow` >>>> calls, which are used by the "Swift Error" breakpoint feature, not only >>>> where the user wrote `throw` in their Swift source, but also along many >>>> (but not all) implicit error propagation edges. It's my recollection that >>>> we only wanted the breakpoint to fire when the user wrote `throw` in >>>> source code, so this seems like an oversight. Does this sound good to you >>>> all? I could see some utility in also catching errors when they propagate >>>> out of ObjC calls, or out of Swift code that's been built without >>>> willThrow hooks, but we could address those in more targeted ways without >>>> the code bloat and false positives from hooking every single propagation >>>> edge. >>> >>> I don't think the intention was that we would only have this breakpoint >>> fire when the user wrote an explicit throw. The point of the trap errors >>> feature is to help identify the source of a thrown error. So while >>> debugging, if I stop in a catch block I didn't expect to stop in, I can >>> turn on error catching, and run again, and this time I will stop when the >>> error gets thrown, and can presumably figure out why it was happening. If >>> this hook caused you to stop sometimes but not always, I think that would >>> be pretty confusing. It would appear as if the error came out of nowhere. >>> Anyway, so the use for this feature is not to catch user throws, but to >>> explain why the user ended up in their "catch" block somewhere. >>> >>> So I don't think it is as simple as only notating explicit user throws. I >>> don't know enough about the guts of the compiler's error propagation to >>> know if there's a class of places that aren't going to result in the user >>> ending up in their catch statement. I'm happy for you to leave the hook >>> out of any such bits of business. But if you leave it out of a place that >>> would then end the user up in their catch block, that's going against the >>> design. >> >> The main places where this would be an issue would be with calls into ObjC >> methods that get implicitly turned into throwing calls in Swift, and in >> places where we called external Swift code that for whatever reason had >> willThrow calls stripped. (It doesn't appear that we ever do that >> currently.) It would be straightforward to keep emitting swift_willThrow >> calls after ObjC calls. > > It would be nice if it wasn't possible to strip willThrow calls, otherwise > somebody is going to think that's a smart idea for their library w/o > realizing that it will make it very hard for clients to debug the library's > throw behavior. 'Course if it really is a smart idea, then we should more > think about making it not so (which was presumably the point of your ABI > changes.) > > Other than that, if you can keep the willThrow after ObjC calls, this sounds > fine.
Yeah, I agree. If we can change willThrow to use the error register, then the code size impact of calling the hook should be minimal, removing most of the motivation to strip the hook. -Joe > >> >>> >>>> >>>> I'd also like to experiment with customizing the calling convention for >>>> the hook so that it uses the error register feature of the Swift calling >>>> convention to reduce register shuffling around the call. This would make >>>> the error value still available to the hook, but it'd be in the error >>>> register instead of in the normal first argument register. Another thing >>>> I'd like to try is giving the hook the `preserveall` calling convention, >>>> so it doesn't clobber any registers (since it doesn't do anything >>>> normally), but this would require us to emit the hook as a local symbol in >>>> each image instead of in the dylib to avoid register clobbering by the >>>> lazy dynamic linker trampoline. Does lldb simply install a symbolic >>>> breakpoint on `swift_willThrow` anywhere, or only as a public symbol in >>>> the libswiftCore.dylib image? How disruptive would these changes be to >>>> lldb's Swift error handling support? >>>> >>> >>> I already have to know about the error register for other purposes, so I >>> don't mind using it rather than the first argument register to fetch the >>> error from the hook. >> >> Cool. >> >>> It looks like I neglected to make the original swift_willThrow breakpoint >>> module specific - though that was plainly an oversight. I'd rather not >>> have us look in every loaded module for this symbol. We've been doing that >>> so far, but then swift debugging isn't as speedy as we want it to be... So >>> that change won't make things slower than now. The breakpoint system >>> already handles multiple hits for one specification, so it there happened >>> to be more than one location to the error throw breakpoint, it won't cause >>> any problems. >> >> If you're OK with us saying we only ever emit willThrow when we see a >> `throw` or ObjC call in Swift code, then I'd say that pushes the calling >> convention change into pointless microoptimization territory. I'm happy to >> keep the single symbol in the runtime. > > That would make my life easier. I filed: > > <rdar://problem/36266718> Swift exception breakpoint for swift_willThrow is > not limited to libswiftCore.dylib > > to limit the search to libswiftCore, which I'll do if you are going to keep > this to one symbol. > > Jim > >> >> -Joe >> _______________________________________________ >> swift-lldb-dev mailing list >> [email protected] >> https://lists.swift.org/mailman/listinfo/swift-lldb-dev _______________________________________________ swift-lldb-dev mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-lldb-dev
