Update type.cpp#4
Conversation
Adding apply_callee_tinfo for apply to automatically recognise and change type if it is function call instruction
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b38e6b730c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d4bc33ec72
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (ida::instruction::is_call(ea)) { | ||
| if (!apply_callee_tinfo(ea, impl_->ti)) | ||
| return std::unexpected( | ||
| Error::sdk("apply_callee_tinfo failed", std::to_string(ea))); |
There was a problem hiding this comment.
TypeInfo::apply now sends every is_call(ea) address to apply_callee_tinfo, but Hex-Rays documents that apply_callee_tinfo requires the caller address to belong to a function; call instructions outside a defined function (or addresses misclassified as call-like) will therefore fail this branch and return an SDK error, whereas the previous implementation still attempted apply_tinfo at that address. This is a behavior regression for valid apply() use cases on not-yet-functionized code and should fall back to apply_tinfo when callee application is not applicable or fails (https://python.docs.hex-rays.com/ida_typeinf/index.html#ida_typeinf.apply_callee_tinfo).
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b6a96ff66d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
|
|
||
| NamedTypeIterator NamedTypeRange::begin() const { | ||
| NamedTypeIterator it; | ||
| NamedTypeAccess::get(it) = new NamedTypeIterator::Impl(impl_->root_til, impl_->flags); |
There was a problem hiding this comment.
NamedTypeRange is publicly default-constructible, leaving impl_ == nullptr, and the new unit tests exercise empty_range.begin()/end() for an empty/default range. This dereference turns that valid empty-range scenario into a crash instead of begin() == end(); return a default/end iterator when impl_ is null before reading impl_->root_til.
Useful? React with 👍 / 👎.
| return std::unexpected(Error::not_found("No type library available")); | ||
|
|
||
| NamedTypeRange range; | ||
| NamedTypeAccess::get(range) = new NamedTypeRange::Impl(til, flags); |
There was a problem hiding this comment.
NamedTypeRange::Impl initializes base_count to 0, and named_types() stores that unchanged here. After the root TIL is exhausted, operator++ only enters base libraries while base_index < base_count, so named_types()/named_types({}) never visits any loaded base TILs despite the API documenting iteration across all TILs. Initialize the range with til->nbases + 1 (or equivalent) when constructing it.
Useful? React with 👍 / 👎.
| til_t* current_til = (impl_->base_index == 0) | ||
| ? impl_->root_til | ||
| : impl_->root_til->base[impl_->base_index]; |
There was a problem hiding this comment.
When iteration advances past the root, base_index == 1 represents the first base TIL, but this indexes root_til->base[1], skipping base[0]; at the last valid base_index it reads one past base[nbases - 1]. This affects any named_types() iteration once base traversal is enabled, so the base case should use base_index - 1 as TilBaseIterator does.
Useful? React with 👍 / 👎.
All three fixes applied: type.cpp:986,1004 — base[impl_->base_index] → base[impl_->base_index - 1]. Matches TilBaseIterator convention: base_index == 1 means base[0]. type_impl.hpp:38,47 + type.cpp:1103,1064,1079 — Impl constructors now accept base_count parameter (default 0). named_types() passes til->nbases + 1, begin()/end() propagate it to the iterator, following TilBaseRange::Impl's existing pattern. type.cpp:1063,1078 — begin() returns end() when impl_ is null. end() returns a default-constructed iterator (null impl_) when impl_ is null. Both produce begin() == end() for default-constructed NamedTypeRange.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78847f113d
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
|
|
||
| TILTypeIterator::TILTypeIterator(const TILTypeIterator& other) { | ||
| auto* copy = new TILTypeIterator::Impl(*TILTypeAccess::get(other)); |
There was a problem hiding this comment.
When a TILTypeIterator is default-constructed, its public default constructor leaves impl_ null, so copying it dereferences a null pointer here. This makes ordinary operations such as TILTypeIterator b = a or post-increment on a default/empty iterator crash; the newly added TilBase* copy constructors use the same unchecked pattern, while NamedType* already guard null impls.
Useful? React with 👍 / 👎.
| const char* first = first_named_type(NamedTypeAccess::get(it)->root_til, NamedTypeAccess::get(it)->flags); | ||
| if (first) { | ||
| NamedTypeAccess::get(it)->current_name = first; | ||
| NamedTypeAccess::get(it)->current_library = get_library_name(NamedTypeAccess::get(it)->root_til); | ||
| return it; | ||
| } | ||
| } | ||
| return end(); |
There was a problem hiding this comment.
If the local/root TIL has no matching named types but loaded base TILs do, begin() returns end() immediately and the iterator never reaches the base-skipping logic in operator++. In that scenario named_types() reports an empty range despite the new API being documented as iterating across all TILs.
Useful? React with 👍 / 👎.
1 participant
Adding apply_callee_tinfo for apply to automatically recognise and change type if it is function call instruction