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

Avoid overlapping .data / .rwtext #616

Merged
merged 2 commits into from
Jun 23, 2023
Merged

Avoid overlapping .data / .rwtext #616

merged 2 commits into from
Jun 23, 2023

Conversation

bjoernQ
Copy link
Contributor

@bjoernQ bjoernQ commented Jun 23, 2023 •
edited
Loading

This fixes a case where .data and .rwtext can overlap if .rwtext's length is not aligned (which might happen because not all instructions are 32bit)

See esp-rs/esp-mbedtls#8

What happened there

4037dacf:	f01d      	retw.n

3fc8dad0 A _data_start
3fc8dad0 d _ZN8esp_wifi4wifi10os_adapter14FAKE_SPIN_LOCK17h713b30e6a3223c01E

0x4037dacf is 0x1d
0x4037dad0 is 0xf0

0x4037dad0 and 3fc8dad0 map to the same location in SRAM - so we end up with an illegal instruction at 0x4037dacf

@bjoernQ bjoernQ mentioned this pull request Jun 23, 2023
Merged
@bjoernQ bjoernQ marked this pull request as ready for review June 23, 2023 12:26
@bjoernQ bjoernQ requested a review from MabezDev June 23, 2023 12:26
MabezDev
MabezDev approved these changes Jun 23, 2023
View reviewed changes
Copy link
Member

@MabezDev MabezDev left a comment

Choose a reason for hiding this comment

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

Another nice find! Thanks for taking care of this! LGTM!

@bjoernQ bjoernQ mentioned this pull request Jun 23, 2023
Closed
@bjoernQ bjoernQ merged commit 0d1d64d into esp-rs:main Jun 23, 2023
15 checks passed
@bjoernQ bjoernQ deleted the avoid-overlapping-data-rwtext branch June 23, 2023 12:33
@AnthonyGrondin AnthonyGrondin mentioned this pull request Jul 3, 2023
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MabezDev MabezDev MabezDev 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
@bjoernQ @MabezDev