-
Notifications
You must be signed in to change notification settings - Fork 10.6k
[Debug] Add pointer based stringForPrintObject #84742
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Debug] Add pointer based stringForPrintObject #84742
Conversation
Adds an overload of `stringForPrintObject` which takes a pointer and mangled typename as arguments. This will be used to improve performance and resilience of `po` in lldb. The pointer and mangled typename are used to construct an `Any` value, which is then passed into the primary implementation of `stringForPrintObject`. This allows calling `stringForPrintObject` without having to first construct a context that contains all necessary Swift modules. This will improve speed, and also resilience when modules cannot be loaded for whatever reason.
| return target | ||
| } | ||
|
|
||
| public static func stringForPrintObject(_ pointer: UnsafeRawPointer?, mangledTypeName: String) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intend to rework this signature, either making it throws, or changing the return type to Result or Optional.
| return "invalid type \(mangledTypeName)" | ||
| } | ||
|
|
||
| func loadPointer<T>(type: T.Type) -> Any { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what would happen here if T is noncopyable. Presumably it would be OK not to support noncopyable types as a first implementation, but we should make sure they at least do something vaguely sensible when failing. I don't think we can use reflection on them at all, but we could potentially check for noncopyable types and return the demangled type name and some message about not being able to show the contents.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
po today doesn't work at all for noncopyable types, so this won't by any worse.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
turns out execution doesn't make it this far (loadPointer). Currently, _getTypeByMangledNameInContext returns nil for non-copyable types. The output is:
"invalid type 4main19StructIsNonCopyableV"
| unsafe _getTypeByMangledNameInContext( | ||
| mangledTypeName, | ||
| UInt(mangledTypeName.count), | ||
| genericContext: nil, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're passing nil here, does it mean this doesn't work with generic types the moment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will work with generic types as long as the mangled name is fully bound, i.e. there aren't any "get the 3rd type parameter from the type I'm in" directives. It may not work for generic types that depend on the context (e.g. you have a [T] local variable), depending on how this mangled name is obtained. But as long as you can get a name with all the parts in it, it will work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as Mike says, this depends on lldb being able to construct a concrete mangled type name. @augusto2112 you've dealt the most with generic types within expression evaluation, are there known cases where this will be a problem?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, the generic expression evaluator can have unbound types, in fact that's essentially its whole purpose for existing, so we'd need to figure out how to pass those parameters in as well.
An easy test for you is to stop in a generic type with one generic parameter and po self:
struct A<T> {
func f() {
// stop here, po self
}
}
| return target | ||
| } | ||
|
|
||
| public static func stringForPrintObject(_ pointer: UnsafeRawPointer?, mangledTypeName: String) -> String { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technical note: we will need to declare this new entry point with availability: @available(SwiftStdlib 6.3, *).
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test |
|
@swift-ci test linux |
|
@swift-ci test linux |
|
@swift-ci test linux |
|
@swift-ci test linux |
|
@swift-ci test linux |
|
@swift-ci test linux |
|
@swift-ci test macOS |
|
@swift-ci test windows |
|
@swift-ci test linux |
|
@swift-ci test macOS |
|
@swift-ci test windows |
|
@swift-ci smoke test |
|
@swift-ci smoke test linux |
|
@swift-ci smoke test windows |
2 similar comments
|
@swift-ci smoke test windows |
|
@swift-ci smoke test windows |
| if type is AnyObject.Type { | ||
| unsafe unsafeBitCast(pointer, to: T.self) | ||
| } else { | ||
| unsafe pointer.load(as: T.self) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mikeash following on our conversation last week, what do you think of this approach for handling objects? This is instead of providing a pointer to the object (pointer to pointer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should work, but it seems a bit overcomplicated. If you unconditionally pointer.load(as: T.self) then everything is consistent. Pass a pointer to the value, and it works.
If there's some case where lldb naturally ends up with an object pointer rather than a pointer to a value of class type, maybe that suggests a second entrypoint specifically class types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm torn. I can see the argument of consistency, but it also seems more natural, to me, to pass an object to the pointer argument, since it is already a pointer, rather than passing a pointer to an object. I'm also uncertain whether it will always be possible to get a pointer to an object, for example an object stored register.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm thinking that can also happen with a non-class type. A sufficiently small struct may be stored into a register. Somewhat larger ones may be stored in multiple registers. So you'll want a way to materialize a value regardless.
However, working with object pointers directly would allow you to support objects even if you don't yet have such a generalized mechanism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to merge this as is, because it allows forward progress, and merging does not prevent either of the following changes:
- changing the semantics of the
pointerargument (ie passing objects using a pointer-to-pointer) - creating two entry points, one for objects, and one for values
This function is used only by lldb and we have the freedom to alter it as needed in the coming weeks, while development and testing happens on the lldb side.
I'll also be thinking about how materialize small structs stored in registers.
thanks Mike.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me.
|
@swift-ci smoke test windows |
1 similar comment
|
@swift-ci smoke test windows |
Adds an overload of `_DebuggerSupport.stringForPrintObject` which takes a pointer and mangled typename as arguments. This will be used to improve performance and resilience of `po` in lldb. The pointer and mangled typename are used to construct an `Any` value, which is then passed into the primary implementation of `stringForPrintObject`. This allows calling `stringForPrintObject` without having to first construct a context that contains all necessary Swift modules. This will improve speed, and also resilience when modules cannot be loaded for whatever reason. rdar://158968103 (cherry picked from commit b6b477d)
Adds an overload of
_DebuggerSupport.stringForPrintObjectwhich takes a pointer and mangled typename as arguments. This will be used to improve performance and resilience ofpoin lldb.The pointer and mangled typename are used to construct an
Anyvalue, which is then passed into the primary implementation ofstringForPrintObject.This allows calling
stringForPrintObjectwithout having to first construct a context that contains all necessary Swift modules. This will improve speed, and also resilience when modules cannot be loaded for whatever reason.rdar://158968103