Add error handling to GetChildCompilerTypeAtIndex() #8793
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This change is a general improvement of the internal API. My motivation is to use this in the Swift typesystem plugin. (cherry picked from commit ac1dc05) Conflicts: lldb/include/lldb/Symbol/TypeSystem.h
|
@swift-ci test |
This was prompted by a crash report due to an out-of-bounds access of "fields" in
return get_from_field_info(fields[idx], tuple, true);
which is now wrapped in an error. I don't know how to trigger this
situation from a testcase.
rdar://128284599
|
@swift-ci test |
JDevlieghere
approved these changes
May 23, 2024
augusto2112
reviewed
May 23, 2024
| child_byte_offset, child_bitfield_bit_size, child_bitfield_bit_offset, | ||
| child_is_base_class, child_is_deref_of_parent, this, language_flags); | ||
| CompilerType child_compiler_type; | ||
| if (!child_compiler_type_or_err) |
There was a problem hiding this comment.
Extreme bikeshed, but I'd switch the success case to be first
| uint64_t language_flags = 0; | ||
|
|
||
| ExecutionContext exe_ctx(GetExecutionContextRef()); | ||
|
|
||
| child_compiler_type = compiler_type.GetChildCompilerTypeAtIndex( | ||
| CompilerType child_compiler_type; |
There was a problem hiding this comment.
Also very inconsequential, but I'd but this closer to the "if" below
| return {}; | ||
| } | ||
| if (!num_children_or_err) | ||
| return num_children_or_err.takeError(); |
| @@ -362,7 +362,7 @@ class TypeSystem : public PluginInterface, | |||
| GetVirtualBaseClassAtIndex(lldb::opaque_compiler_type_t type, size_t idx, | |||
| uint32_t *bit_offset_ptr) = 0; | |||
|
|
|||
| virtual CompilerType GetChildCompilerTypeAtIndex( | |||
| virtual llvm::Expected<CompilerType> GetChildCompilerTypeAtIndex( | |||
There was a problem hiding this comment.
Are you upstreaming these changes to the generic code?
| @@ -53,6 +53,14 @@ using namespace lldb_private; | |||
|
|
|||
| namespace lldb_private { | |||
|
|
|||
| std::string toString(const swift::reflection::TypeRef *tr) { | |||
| @@ -1220,6 +1227,11 @@ CompilerType SwiftLanguageRuntimeImpl::GetChildCompilerTypeAtIndex( | |||
| language_flags = 0; | |||
| return ts->GetRawPointerType(); | |||
| } | |||
| if (idx - 3 >= fields.size()) | |||
| return llvm::createStringError(llvm::inconvertibleErrorCode(), | |||
| llvm::Twine("index") + llvm::Twine(idx) + | |||
There was a problem hiding this comment.
I think you need spaces here? Will it print "indexnis out of bounds"?
| @@ -1387,7 +1414,7 @@ CompilerType SwiftLanguageRuntimeImpl::GetChildCompilerTypeAtIndex( | |||
| // field in the base class. | |||
| if (!type_ref) { | |||
| child_name = "<base class>"; | |||
| return {}; | |||
| return CompilerType(); | |||
There was a problem hiding this comment.
Are these "return CompilerType()" different from errors?
drodriguez
reviewed
May 24, 2024
| @@ -2425,9 +2447,11 @@ constexpr ExecutionContextScope *g_no_exe_ctx = nullptr; | |||
| } while (0) | |||
|
|
|||
| #define VALIDATE_AND_RETURN_EXPECTED(IMPL, REFERENCE, TYPE, EXE_CTX, ARGS, \ | |||
| FALLBACK_ARGS, DEFAULT) \ | |||
| FALLBACK_ARGS) \ | |||
There was a problem hiding this comment.
We need the equivalent of #8394 to remove the DEFAULT from the #else branch in new line 2500.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was prompted by a crash report due to an out-of-bounds access of "fields" in
which is now wrapped in an error. I don't know how to trigger this
situation from a testcase.
rdar://128284599