Skip to content

Fix dereference behavior on mixed subscript and arrow / dot operators - #182

Merged
jserv merged 1 commit into
sysprog21:masterfrom
ChAoSUnItY:fix/deref
Mar 24, 2025
Merged

jserv merged 1 commit into
sysprog21:masterfrom
ChAoSUnItY:fix/deref

Conversation

@ChAoSUnItY

@ChAoSUnItY ChAoSUnItY commented Mar 20, 2025 •

Copy link
Copy Markdown
Collaborator

Summary by Bito

This pull request fixes a double free error in the globals file and enhances type safety in the parser by changing variable types from int to bool. It also introduces new tests for dynamic data structures, improving the robustness of the codebase and addressing issues #165, #181, and #164.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

@sysprog21 sysprog21 deleted a comment from bito-code-review Bot Mar 20, 2025
@ChAoSUnItY
ChAoSUnItY marked this pull request as ready for review March 20, 2025 11:28
Comment thread tests/driver.sh Outdated
Comment thread src/parser.c
@ChAoSUnItY ChAoSUnItY changed the title Fix deference behavior on mixed subscription and arrow operator Fix deference behavior on mixed subscript and arrow operators Mar 20, 2025
@ChAoSUnItY
ChAoSUnItY requested a review from DrXiao March 20, 2025 16:25
Comment thread tests/driver.sh Outdated
Comment thread src/parser.c Outdated
Comment thread src/parser.c Outdated
@ChAoSUnItY
ChAoSUnItY force-pushed the fix/deref branch 2 times, most recently from a939ffb to 4b1627c Compare March 20, 2025 17:56
@ChAoSUnItY ChAoSUnItY changed the title Fix deference behavior on mixed subscript and arrow operators Fix deference behavior on mixed subscript and arrow / dot operators Mar 20, 2025

@jserv jserv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Clarify the use of "deference" and "dereference" in the wording.

Comment thread src/parser.c Outdated

@DrXiao DrXiao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I notice that the title of this pull request still uses "deference" in the wording. Fixing it to "dereference" is better and prevents misunderstandings.

Additionally, the git commit title is Fix incorrect deref behavior, and I believe it could be improved with a clearer summary.

Comment thread tests/driver.sh Outdated
Comment thread src/parser.c
@ChAoSUnItY ChAoSUnItY changed the title Fix deference behavior on mixed subscript and arrow / dot operators Fix dereference behavior on mixed subscript and arrow / dot operators Mar 21, 2025

@DrXiao DrXiao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Except for the typo in the comment that needs to be fixed, please adjust the git commit message.

After fixing the typo and refining the git commit message, I think the proposed changes look good.

Comment thread src/parser.c Outdated

@DrXiao DrXiao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The proposed changes look good to me.

@jserv jserv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Append the Close #? where ? stands for the issue you are managing to close at the end of git commit messages.

Previously, using subscript operator (literally "[]") with arrow
operator (literally "->") or dot operator (literally ".") would cause
incorrect dereference result due to inappropriate delayed dereference
strategy. For example, considering "data->raw[0]", this would evaluate
to "data[0]", same for "data.raw[0]". In this patch, by adding
dereference instruction when encountered subscript operator after
parsed either arrow or dot operator, in other word, after accessed
struct's member, this corrects the final evaluated address.

Close sysprog21#164, close sysprog21#165, close sysprog21#181.
@jserv
jserv merged commit 24bffd4 into sysprog21:master Mar 24, 2025
@jserv

jserv commented Mar 24, 2025

Copy link
Copy Markdown
Collaborator

Thank @ChAoSUnItY for contributing!

@ChAoSUnItY
ChAoSUnItY deleted the fix/deref branch April 28, 2025 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incorrect double free behavior Invalid char pointer deference behavior On-heap struct pointer assignment corrupts memory

3 participants