Skip to content

Fully initialize ev.data. #325

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.

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

Merged
merged 1 commit into from
Dec 14, 2017
Merged

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Dec 4, 2017

On Linux systems, ev.data is an 8-byte field, but this code currently initializes only 4 bytes of the field, leaving the trailing 4 bytes unininitialized. This causes memory safety tools like Valgrind to complain, as noted in #321.

This patch no longer explicitly initializes .data, ensuring that C structure initialization will force the .data field to be zero-initialized. Manual checks of the compiled binary show that the compiler correctly initializes the 4 bytes for .u32 only once, so this change incurs minimal overhead (specifically, setting the next 4 bytes to 0).

On Linux systems, ev.data is an 8-byte field, but this code
currently initializes only 4 bytes of the field, leaving the
trailing 4 bytes unininitialized. This causes memory safety tools
like Valgrind to complain.

This patch no longer explicitly initializes .data, ensuring that C
structure initialization will force the .data field to be zero
initialized. Manual checks of the compiled binary show that the
compiler correctly initializes the 4 bytes for u32 only once, so
this change incurs minimal overhead.
@MadCoder
Copy link
Contributor

MadCoder commented Dec 4, 2017

@swift-ci please test

1 similar comment
@alblue
Copy link

alblue commented Dec 4, 2017

@swift-ci please test

@Lukasa
Copy link
Contributor Author

Lukasa commented Dec 4, 2017

@MadCoder Sorry, I completely missed your feedback until just now! Would you prefer that I update the dispatch codebase to use u64 instead? I agree that it'll clean things up, but it'll be a much more far-reaching patch.

@MadCoder
Copy link
Contributor

MadCoder commented Dec 4, 2017

no that's fine, the patch you did is fine too

@MadCoder
Copy link
Contributor

MadCoder commented Dec 4, 2017

what is happening to the bots now...

@swift-ci please test

@MadCoder
Copy link
Contributor

MadCoder commented Dec 4, 2017

@swift-ci please test

@shahmishal
Copy link
Member

@swift-ci test

@MadCoder MadCoder merged commit 723bd98 into swiftlang:master Dec 14, 2017
ktopley-apple pushed a commit that referenced this pull request Dec 6, 2018
Fully initialize ev.data.

Signed-off-by: Daniel A. Steffen <[email protected]>
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.

5 participants