-
Notifications
You must be signed in to change notification settings - Fork 353
[lldb] Adopt new pointer and mangled typename based stringForPrintObject #11916
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
base: swift/release/6.3
Are you sure you want to change the base?
Changes from 7 commits
38f5709
9cd7c44
3c969d6
caade22
defe915
ff4b066
714c3bb
efa060f
f6bf6dd
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -640,15 +640,22 @@ Status SwiftExpressionSourceCode::GetText( | |
| if (triple.isOSDarwin()) { | ||
| if (auto process_sp = exe_ctx.GetProcessSP()) { | ||
| os_vers << getAvailabilityName(triple) << " "; | ||
| auto platform = target->GetPlatform(); | ||
| bool is_simulator = platform->GetPluginName().ends_with("-simulator"); | ||
| if (is_simulator) { | ||
| // The simulators look like the host OS to Process, but Platform | ||
| // can the version out of an environment variable. | ||
| os_vers << platform->GetOSVersion(process_sp.get()).getAsString(); | ||
| if (options.GetDisableAvailability()) { | ||
| // Disable availability by setting the OS version to 9999. This | ||
| // placeholder OS version used for future OS versions when building | ||
| // the Swift standard library locally. | ||
| os_vers << "9999"; | ||
| } else { | ||
| llvm::VersionTuple version = process_sp->GetHostOSVersion(); | ||
| os_vers << version.getAsString(); | ||
| auto platform = target->GetPlatform(); | ||
| bool is_simulator = platform->GetPluginName().ends_with("-simulator"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we really not have a nicer way to tell if a Platform is a simulator?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's |
||
| if (is_simulator) { | ||
| // The simulators look like the host OS to Process, but Platform | ||
| // can the version out of an environment variable. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. "can the" -> "can get the" |
||
| os_vers << platform->GetOSVersion(process_sp.get()).getAsString(); | ||
| } else { | ||
| llvm::VersionTuple version = process_sp->GetHostOSVersion(); | ||
| os_vers << version.getAsString(); | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| #include "lldb/Utility/Log.h" | ||
| #include "lldb/Utility/OptionParsing.h" | ||
| #include "lldb/Utility/Status.h" | ||
| #include "lldb/Utility/StreamString.h" | ||
| #include "lldb/Utility/StructuredData.h" | ||
| #include "lldb/Utility/Timer.h" | ||
| #include "lldb/ValueObject/ValueObject.h" | ||
|
|
@@ -835,7 +836,7 @@ SwiftLanguageRuntime::GetObjectDescriptionExpr_Ref(ValueObject &object) { | |
| object.GetValueAsUnsigned(0)).str(); | ||
|
|
||
| if (log) | ||
| log->Printf("[GetObjectDescriptionExpr_Result] expression: %s", | ||
| log->Printf("[GetObjectDescriptionExpr_Ref] expression: %s", | ||
| expr_string.GetData()); | ||
| return expr_str; | ||
| } | ||
|
|
@@ -911,25 +912,27 @@ std::string SwiftLanguageRuntime::GetObjectDescriptionExpr_Copy( | |
| return expr_string; | ||
| } | ||
|
|
||
| llvm::Error SwiftLanguageRuntime::RunObjectDescriptionExpr( | ||
| ValueObject &object, std::string &expr_string, Stream &result) { | ||
| static llvm::Expected<ValueObjectSP> | ||
| RunObjectDescription(ValueObject &object, std::string &expr_string, | ||
| Process &process, bool disable_availability = false) { | ||
| Log *log(GetLog(LLDBLog::DataFormatters | LLDBLog::Expressions)); | ||
| ValueObjectSP result_sp; | ||
| EvaluateExpressionOptions eval_options; | ||
| eval_options.SetUnwindOnError(true); | ||
| eval_options.SetLanguage(lldb::eLanguageTypeSwift); | ||
| eval_options.SetSuppressPersistentResult(true); | ||
| eval_options.SetIgnoreBreakpoints(true); | ||
| eval_options.SetTimeout(GetProcess().GetUtilityExpressionTimeout()); | ||
| eval_options.SetTimeout(process.GetUtilityExpressionTimeout()); | ||
| if (disable_availability) | ||
| eval_options.SetDisableAvailability(); | ||
|
|
||
| StackFrameSP frame_sp = object.GetFrameSP(); | ||
| if (!frame_sp) | ||
| frame_sp = | ||
| GetProcess().GetThreadList().GetSelectedThread()->GetSelectedFrame( | ||
| DoNoSelectMostRelevantFrame); | ||
| frame_sp = process.GetThreadList().GetSelectedThread()->GetSelectedFrame( | ||
| DoNoSelectMostRelevantFrame); | ||
| if (!frame_sp) | ||
| return llvm::createStringError("no execution context to run expression in"); | ||
| auto eval_result = GetProcess().GetTarget().EvaluateExpression( | ||
| auto eval_result = process.GetTarget().EvaluateExpression( | ||
| expr_string, frame_sp.get(), result_sp, eval_options); | ||
|
|
||
| LLDB_LOG(log, "[RunObjectDescriptionExpr] {0}", toString(eval_result)); | ||
|
|
@@ -952,12 +955,17 @@ llvm::Error SwiftLanguageRuntime::RunObjectDescriptionExpr( | |
| return llvm::createStringError("expression produced invalid result type"); | ||
| } | ||
|
|
||
| return result_sp; | ||
| } | ||
|
|
||
| static llvm::Error DumpString(ValueObjectSP result_sp, Stream &strm) { | ||
| Log *log(GetLog(LLDBLog::DataFormatters | LLDBLog::Expressions)); | ||
| formatters::StringPrinter::ReadStringAndDumpToStreamOptions dump_options; | ||
| dump_options.SetEscapeNonPrintables(false); | ||
| dump_options.SetQuote('\0'); | ||
| dump_options.SetPrefixToken(nullptr); | ||
| if (formatters::swift::String_SummaryProvider( | ||
| *result_sp.get(), result, | ||
| *result_sp, strm, | ||
| TypeSummaryOptions() | ||
| .SetLanguage(lldb::eLanguageTypeSwift) | ||
| .SetCapping(eTypeSummaryUncapped), | ||
|
|
@@ -973,6 +981,15 @@ llvm::Error SwiftLanguageRuntime::RunObjectDescriptionExpr( | |
| return llvm::createStringError("expression produced unprintable string"); | ||
| } | ||
|
|
||
| llvm::Error SwiftLanguageRuntime::RunObjectDescriptionExpr( | ||
| ValueObject &object, std::string &expr_string, Stream &strm) { | ||
| auto result_or_err = RunObjectDescription(object, expr_string, GetProcess()); | ||
| if (!result_or_err) | ||
| return result_or_err.takeError(); | ||
|
|
||
| return DumpString(*result_or_err, strm); | ||
| } | ||
|
|
||
| static bool IsVariable(ValueObject &object) { | ||
| if (object.IsSynthetic()) | ||
| return IsVariable(*object.GetNonSyntheticValue()); | ||
|
|
@@ -1002,11 +1019,84 @@ static bool IsSwiftReferenceType(ValueObject &object) { | |
| return false; | ||
| } | ||
|
|
||
| static bool PrintObjectViaPointer(Stream &strm, ValueObject &object, | ||
| Process &process) { | ||
| Log *log = GetLog(LLDBLog::DataFormatters | LLDBLog::Expressions); | ||
|
|
||
| Flags flags(object.GetCompilerType().GetTypeInfo()); | ||
| addr_t addr = LLDB_INVALID_ADDRESS; | ||
| if (flags.Test(eTypeInstanceIsPointer)) { | ||
| // Objects are pointers. | ||
| addr = object.GetValueAsUnsigned(LLDB_INVALID_ADDRESS); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should use GetValueAsAddress.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm surprised we don't have a little wrapper for this in ValueObject, but you can do what GetValueAsAddress does, and call FixDataAddress of the addr you get back. @jasonmolenda will know for sure whether that's the right thing to do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah if it's guaranteed to be an address it's safe to run it through Process::FixAnyAddress() (the method to use when you don't know if it's an address of code or data). With MTE managed heap, if the value might get put into a jitted expression (so used in code running in the inferior, with MTE) you need to retain the MTE nibble in the top byte. It's an edge case but something to mention, @felipepiovezan dealt with a tricky case where we needed to maintain metadata bits through to the point where we looked it up/use it, just recently, for something that ended up in a jitted expression. |
||
| } else { | ||
| // Get the address of non-object values (structs, enums). | ||
| auto addr_and_type = object.GetAddressOf(false); | ||
| if (addr_and_type.type != eAddressTypeLoad) { | ||
| LLDB_LOG(log, "address-of value object failed, preventing use of " | ||
| "stringForPrintObject(_:mangledTypeName:)"); | ||
| return false; | ||
| } | ||
| addr = addr_and_type.address; | ||
| } | ||
|
|
||
| if (addr == 0 || addr == LLDB_INVALID_ADDRESS) | ||
| return false; | ||
|
|
||
| StringRef mangled_type_name = object.GetMangledTypeName(); | ||
| // Swift APIs that receive mangled names require the prefix removed. | ||
| mangled_type_name.consume_front("$s"); | ||
kastiglione marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| mangled_type_name.consume_front("$e"); // Embedded Swift prefix | ||
|
|
||
| std::string expr_string = | ||
| llvm::formatv( | ||
| "Swift._DebuggerSupport.stringForPrintObject(UnsafeRawPointer(" | ||
| "bitPattern: {0}), mangledTypeName: \"{1}\")", | ||
| addr, mangled_type_name) | ||
| .str(); | ||
|
|
||
| auto result_or_err = RunObjectDescription(object, expr_string, process, true); | ||
| if (!result_or_err) { | ||
| LLDB_LOG_ERROR(log, result_or_err.takeError(), | ||
| "stringForPrintObject(_:mangledTypeName:) failed: {0}"); | ||
kastiglione marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return false; | ||
| } | ||
|
|
||
| // A `(Bool, String)` tuple, where the bool indicates success/failure. | ||
| auto result_sp = *result_or_err; | ||
| auto success_sp = result_sp->GetChildAtIndex(0); | ||
| auto description_sp = result_sp->GetChildAtIndex(1); | ||
|
|
||
| StreamString dump_stream; | ||
| auto err = DumpString(description_sp, dump_stream); | ||
| if (err) { | ||
| LLDB_LOG_ERROR(log, std::move(err), | ||
| "decoding result of " | ||
| "stringForPrintObject(_:mangledTypeName:) failed: {0}"); | ||
| return false; | ||
| } | ||
|
|
||
| Status status; | ||
| if (!success_sp->IsLogicalTrue(status)) { | ||
| LLDB_LOGF(log, | ||
| "stringForPrintObject(_:mangledTypeName:) produced error: %s", | ||
| dump_stream.GetData()); | ||
| return false; | ||
| } | ||
|
|
||
| LLDB_LOG(log, "stringForPrintObject(_:mangledTypeName:) succeeded"); | ||
| strm.PutCString(dump_stream.GetString()); | ||
| return true; | ||
| } | ||
|
|
||
| llvm::Error SwiftLanguageRuntime::GetObjectDescription(Stream &str, | ||
| ValueObject &object) { | ||
| if (object.IsUninitializedReference()) | ||
| return llvm::createStringError("<uninitialized>"); | ||
|
|
||
| if (GetProcess().GetTarget().GetSwiftUseContextFreePrintObject()) | ||
| if (PrintObjectViaPointer(str, object, GetProcess())) | ||
| return llvm::Error::success(); | ||
|
|
||
| std::string expr_string; | ||
|
|
||
| if (::IsVariable(object) || ::IsSwiftResultVariable(object.GetName())) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| SWIFT_SOURCES := main.swift | ||
| SWIFTFLAGS_EXTRAS := -parse-as-library | ||
| include Makefile.rules |
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 is an odd set API. It only allows you to set it to true, which the name doesn't really imply?
We usually do:
SetWhatever(bool new_value)
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 doesn't? I'm failing to see how it would be interpreted otherwise…
There's no reason for code to call
SetDisableAvailability(False), as that is the default. Since there's no reason for it to be called like that, I made the function take no arguments, so that it can be called only one way. This may be different than other bool setters, but I feel it's better prevent it from being mis-called than make it match other setters.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.
Set means "give some value to this feature" - true or false to "Disabling Ability". If all this API does is "DisableAvailability" then it should be called
DisableAvailability(). The "Set" is just confusing and adds nothing.If there are never any situations where you Disable Availability, then you want to enable it again on the same object, then just having a DisableAvailability API is fine. If you would ever want to turn it back on, then you will need a Set API with a bool input.