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

Sanitizers support #466

Merged
merged 1 commit into from
Nov 5, 2019
Merged

Sanitizers support #466

merged 1 commit into from
Nov 5, 2019

Conversation

tmiasko
Copy link
Contributor

@tmiasko tmiasko commented Oct 14, 2019

Documentation for existing sanitizers support in rustc, as well as changes proposed in rust-lang/rust#65241.

@tmiasko tmiasko mentioned this pull request Oct 14, 2019
Merged
mark-i-m
mark-i-m approved these changes Oct 14, 2019
View reviewed changes
Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Thanks!

One minor nit, if you have time

src/sanitizers.md Outdated
==10029==ABORTING
```

Use of uninitialized memory detected by MemorySanitizer. Note that we are using
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps make the title a markdown heading or bolded or something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added an introductory paragraph to examples and additional headings.

@tmiasko
Copy link
Contributor Author

tmiasko commented Oct 15, 2019

I would probably wait for mentioned PR in rustc to get merged first. While
almost everything described here is already true, the recommendation about using
-Zbuild-std isn't.

nikomatsakis
nikomatsakis reviewed Oct 15, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

This is awesome! That said, I wonder if this documentation would be better placed in the rustc book, which can be found (I think) in src/doc/rustc. What I would expect to include here in the rustc-guide are a few details on how it is implemented. Basically trying to convey the high-level strategy that is implemented in this commit.

@tmiasko
Copy link
Contributor Author

tmiasko commented Oct 16, 2019

I extended "How are sanitizers implemented in rustc?" section, and removed everything else.

mark-i-m
mark-i-m previously approved these changes Oct 18, 2019
View reviewed changes
Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Thanks!

@mark-i-m
Copy link
Member

It looks like CI is failing

nikomatsakis
nikomatsakis previously approved these changes Oct 18, 2019
View reviewed changes
Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

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

Wow this is so great, thank you thank you. =)

src/sanitizers.md Outdated

To enable a sanitizer compile with `-Zsanitizer=...` option, where value is one
of `address`, `leak`, `memory` or `thread`. For more details how to use
sanitizers please refer to rustc book.
Copy link
Contributor

Choose a reason for hiding this comment

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

For some reason I can't make suggestions on this repo, but this should be

please refer to [the unstable book](https://doc.rust-lang.org/unstable-book/).

src/sanitizers.md Outdated

The implementation of sanitizers relies entirely on LLVM. It consists of
compile time instrumentation passes and runtime libraries. The role rustc plays
in the implementation is limited to the execution of following steps:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: executon of the following steps

src/sanitizers.md Outdated
compile time instrumentation passes and runtime libraries. The role rustc plays
in the implementation is limited to the execution of following steps:

1. The sanitizer runtime libraries are part of [compiler-rt] project, and [will
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: the compiler-rt project

@nikomatsakis nikomatsakis mentioned this pull request Oct 18, 2019
Merged
@tmiasko
Copy link
Contributor Author

tmiasko commented Oct 18, 2019

Thanks. I applied the suggestions.

mark-i-m
mark-i-m approved these changes Nov 5, 2019
View reviewed changes
Copy link
Member

@mark-i-m mark-i-m left a comment

Choose a reason for hiding this comment

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

Thanks!

@mark-i-m mark-i-m merged commit 06908de into rust-lang:master Nov 5, 2019
@tmiasko tmiasko deleted the sanitizers branch January 17, 2020 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mark-i-m mark-i-m mark-i-m approved these changes

@nikomatsakis nikomatsakis nikomatsakis left review comments

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

Successfully merging this pull request may close these issues.
3 participants
@tmiasko @mark-i-m @nikomatsakis