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.

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

unions: call out field offset issues #627

Merged
merged 2 commits into from Jun 29, 2019
Merged

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Jun 29, 2019

@gnzlbg we do guarantee offset 0 for repr(C), right? That just follows from being C-compatible?

src/items/unions.md Outdated Show resolved Hide resolved
@gnzlbg
Copy link
Contributor

gnzlbg commented Jun 29, 2019

we do guarantee offset 0 for repr(C), right? That just follows from being C-compatible?

Yes (https://github.com/rust-lang/unsafe-code-guidelines/blob/master/reference/src/layout/unions.md#c-compatible-layout-repr-c):

For unions tagged #[repr(C)], the compiler will apply the C layout scheme. Per sections 6.5.8.5 and 6.7.2.1.16 of the C11 specification, this means that the offset of every field is 0. Unsafe code can cast a pointer to the union to a field type to obtain a pointer to any field, and vice versa.

union field reads the bits of the union at the field's type. Fields might have a
non-zero offset (except when `#[repr(C)]` is used); in that case the bits
starting at the offset of the fields are read. It is the programmer's
responsibility to make sure that the data is valid at the field's type. Failing
Copy link
Contributor

Choose a reason for hiding this comment

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

It's not really the role of the reference to say "it is the programmer's responsibility" or "this is how you use X" but it's fine here imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you have a suggestion for wording it differently? I think it is useful to think in terms of responsibilities/obligations when talking about unsafe code, and hence I think it is useful for the reference to explain it as such.

I also have 0 experience writing standard-esque text.^^ Given my experience reading it though, it seems often so useless that I am not sure I am sad about that...

Copy link
Contributor

@Centril Centril Jun 29, 2019

Choose a reason for hiding this comment

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

Not really; the text itself is good but it's probably more aimed at users (e.g. the nomicon) whereas the reference is intended as the seeds of a spec with just definitions of the language but not the "why" and "this is how I use it". That is, should be more like the details of one of your academic papers with operational semantics and judgements... it isn't now, but it should be. :)

In any case, I think we can afford a smidgen of tips towards the programmer also here because some users will read this.

Copy link
Member Author

@RalfJung RalfJung Jun 29, 2019

Choose a reason for hiding this comment

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

You mean the parts of the paper (or more often, the appendix) that only 2 people on this planet read because they are incomprehensible? ;)

Copy link
Contributor

@Centril Centril Jun 29, 2019

Choose a reason for hiding this comment

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

That's not the papers I read :)

I think something like the WASM spec has a nice structure:

That is, a mix of formally written semantics and English text to make it a bit less dense and more informative for non-experts.

@Centril Centril merged commit bccb2c7 into rust-lang:master Jun 29, 2019
@RalfJung RalfJung deleted the union branch June 29, 2019 18:21
bors added a commit to rust-lang/rust that referenced this pull request Jul 17, 2019
Update mdbook, cargo, books

This updates the last of the books using mdbook 0.1, finally removing it from the build.  Cargo, the reference, and the rustc book were the last to change.  This should have a noticeable impact on CI times.

I decided to remove the version switcher from the rustbook tool.  It seemed a little awkward keeping it with one version.  If we ever need it in the future, we can always add it back, it is a relatively small amount of code.  I would just like to avoid the temptation to split the versions in the future.

## cargo

12 commits in 677a180f4c8ca1dcef594f68dd0e63e4f08621f5..e3563dbdcd2e370bc4f11d080f739d82d25773fd
2019-07-08 13:43:02 +0000 to 2019-07-16 19:22:44 +0000
- Add Cirrus CI badge to manifest (rust-lang/cargo#7119)
- Update mdbook to 0.3. (rust-lang/cargo#7140)
- Remove unused feature filter. (rust-lang/cargo#7131)
- Remove now-unused `WorkspaceResolve` (rust-lang/cargo#7127)
- Fix some clippy warnings. (rust-lang/cargo#7135)
- Also ignore remap-path-prefix in metadata for `cargo rustc`. (rust-lang/cargo#7134)
- Fix some formatting for some strings. (rust-lang/cargo#7129)
- Handle activation conflicts for `[patch]` sources (rust-lang/cargo#7118)
- [BETA] Fix `cargo new` in root directory. (rust-lang/cargo#7126)
- Remove MyFnOnce (rust-lang/cargo#7125)
- Don't suppress error messages with `-q` (rust-lang/cargo#7116)
- Fix changelog date (rust-lang/cargo#7115)

## book

2 commits in 6c0d83499c8e77e06a71d28c5e1adccec278d4f3..7ddc46460f09a5cd9bd2a620565bdc20b3315ea9
2019-06-23 20:25:30 -0400 to 2019-06-27 09:50:36 -0400
- reexport to re-export
- Fixes made in final layout check

## reference

4 commits in 7a5aab5..8e7d614
2019-06-20 17:38:52 +0200 to 2019-07-16 21:02:33 +0100
- Update to mdbook 0.3. (rust-lang/reference#641)
- Remove extra parenthesis (rust-lang/reference#621)
- Correct sentence about unsized coercions (rust-lang/reference#628)
- unions: call out field offset issues (rust-lang/reference#627)

## edition-guide

4 commits in f8072acde5ce29c7570d7986180bbded2d22e287..f6c8b92d4e63edd28e862be952f33861f35956f8
2019-06-14 23:27:05 +0200 to 2019-07-06 22:10:32 +0200
- Fix syntax highlighting in macro guide (rust-lang/edition-guide#182)
- Fix error example for modern compilers. (rust-lang/edition-guide#160)
- Try cargo install-upgrade for mdbook. (rust-lang/edition-guide#181)
- correct the travis build badge URL (rust-lang/edition-guide#180)

## embedded-book

2 commits in ef27b517dcd0b990c888c0d7caeff52a5a115619..ff334e74fdb9f197e621efa6d7c3105be892e888
2019-06-18 22:59:47 +0000 to 2019-07-16 13:47:34 +0000
- use_core() is required for #![no_std] compatible generated bindings  (rust-embedded/book#197)
- concurrency: update shared resource examples  (rust-embedded/book#196)

## nomicon

7 commits in 341c221116a8b9f1010cf1eece952b80c5ec7f54..b7f0aba2f88a8feade70595efcfa3438e54e96c0
2019-06-19 09:08:47 -0700 to 2019-07-11 15:11:36 -0400
- chore: Remove redundant Eq import
- Fix link to rfc1857
- Move word "reading" out of the link to "The Book"
- update the link to `mdbook`
- re-wrap more text
- re-wrap the text to 80 columns
- Better explain how to use `mdbook`

## rust-by-example

8 commits in 62b3ff2cb44dd8b648c3ef2a9347c3706d148014..e3679e214d8db44586aca9b20aa27517007d1923
2019-06-24 09:17:21 -0300 to 2019-07-15 11:13:44 -0300
- Collection of fixes for the 'Vectors` chapter. (rust-lang/rust-by-example#1216)
- Changed mutable collected_iterator into immutable (rust-lang/rust-by-example#1219)
- Fix minor formatting inconsistencies (rust-lang/rust-by-example#1220)
- Make `use` example runnable (rust-lang/rust-by-example#1215)
- Fix: Automatically Typo (rust-lang/rust-by-example#1214)
- Collection of fixes for the `Strings` chapter. (rust-lang/rust-by-example#1212)
- Added link to russian translation (rust-lang/rust-by-example#1213)
- A couple of fixes for the `HashMap` chapter. (rust-lang/rust-by-example#1211)
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.

None yet

3 participants