Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[tx] check for various issues in dumpCstr and dumpINDEX #956

Merged
merged 3 commits into from
Sep 5, 2019
Merged

[tx] check for various issues in dumpCstr and dumpINDEX #956

merged 3 commits into from
Sep 5, 2019

Conversation

cjchapman
Copy link
Contributor

@cjchapman cjchapman commented Sep 5, 2019 •
edited
Loading

  • check for empty varRegionInfo in dumpCstr (like we already do in dcf_setNumRegions)
  • check for invalid offSize in dumpINDEX
  • check for stack underflow wth blend and vsindex in dumpCstr

@cjchapman
[tx] check for empty varRegionInfo in dumpCstr
c9133a2 
(like we already do in dcf_setNumRegions)
blueshade7
blueshade7 requested changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix itself looks good, but I noticed we are missing a check against an empty stack before accessing h->stack.array[0]. Maybe we need CHKUFLOW(1); as we have it in callsubr().
The same can be said for t2_blend too.

@cjchapman cjchapman changed the title [tx] check for empty varRegionInfo in dumpCstr [tx] check for various issues in dumpCstr and dumpINDEX Sep 5, 2019
blueshade7
blueshade7 approved these changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@blueshade7 blueshade7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@cjchapman cjchapman merged commit 8af25e4 into develop Sep 5, 2019
@cjchapman cjchapman deleted the cjc-check-empty-varRegionInfo-in-dumpCstr branch September 5, 2019 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blueshade7 blueshade7 blueshade7 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cjchapman @blueshade7