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

clarify UB for raw ptr deref #1000

Merged
merged 3 commits into from
Apr 7, 2021
Merged

clarify UB for raw ptr deref #1000

merged 3 commits into from
Apr 7, 2021

Conversation

RalfJung
Copy link
Member

@RalfJung RalfJung commented Apr 3, 2021

Copy link
Contributor

@danielhenrymantilla danielhenrymantilla left a comment

Choose a reason for hiding this comment

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

Mentioning / linking to an actual distinction between value and place expression seems totally reasonable when talking about pointer dereference (especially if the rules ended up loosened for to-place-dereferences), so 💯 from me! ✅

Comment on lines 26 to 27
* Evaluating a dereference [place expression] (`*expr`) on a raw pointer that is
[dangling] or unaligned.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this was raised in the Zulip conversation, but I'll nevertheless raise it here: I wonder if a word other than e-valu-ating could be used when talking about a place expression (although I couldn't come up with one satisfactory enough, so feel free to disregard this nit).

Copy link
Member Author

@RalfJung RalfJung Apr 3, 2021

Choose a reason for hiding this comment

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

Clearly the right term is "placating".

On a more serious note, this is one reason why I argued fiercly against calling these things "values"/"value expressions", but I was unable to convince enough people to make a difference...

src/behavior-considered-undefined.md Outdated Show resolved Hide resolved
@@ -23,7 +23,8 @@ code.
</div>

* Data races.
* Dereferencing (using the `*` operator on) a dangling or unaligned raw pointer.
* Evaluating a dereference [place expression] (`*expr`) on a raw pointer that is
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels weird to me, because a dereference is always a place expression.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, it can be used as a value expression as in

let x = *expr;

Arguably, here it is correct to say that *expr is (used as) a value expression.

Copy link
Contributor

Choose a reason for hiding this comment

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

We call that a "value expression context" (and "place expression context" for where a place is wanted). Is it only UB to evaluate an unaligned/dangling pointer in place expression context, or always?

Copy link
Member Author

@RalfJung RalfJung Apr 6, 2021

Choose a reason for hiding this comment

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

"value expression contexts" are a superset of "place expression contexts". IOW, the code above is sugar for

let x = place2value(*expr);

Evaluating a place expression in value expression context consists of first evaluating the place expression as normal, as then performing place-to-value conversion.

So, it is impossible to write *expr anywhere without it being also a place expression, but sometimes, it is both a place expression and a value expression (or really, place2value(*expr) is the value expression, but since we have no syntax for this, people tend to say that *expr is [used as] a value expression).

That's the way I am thinking about this, anyway.

So, the answer is that it is always UB to evaluate an unaligned/dangling *expr since doing so always evaluates the place expression -- and then sometimes goes on performing place-to-value conversion. The important point that I hope to clarify in the docs is that it is the place expression evaluation, and not the place-to-value conversion, that is causing the UB.

Copy link
Contributor

Choose a reason for hiding this comment

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

"value expression contexts" are a superset of "place expression contexts"

We have a difference of perspectives here; these two categories are distinct and split the set of expression contexts. A place expression in value expression context is allowed and has the place2value effect, but it's still always a place expression.

So, under my perspective, the [place expression] should be , even in [place expression contexts],.


Evaluating a place expression in value expression context consists of first evaluating the place expression as normal, as then performing place-to-value conversion.

I like this sentence. I think I'll try and get it into the PR I submitted last night (#1003 )

Copy link
Member Author

Choose a reason for hiding this comment

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

So, more like this?

@digama0 I'd be interested in your opinion here, is this current wording sufficiently clear? (I know you'd like the rules to be more relaxed; so do I. That is a longer process. For now the goal is to make sure that the rules as they currently are are described unambiguously.)

Copy link

Choose a reason for hiding this comment

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

LGTM

@Havvy Havvy merged commit 6f57cf4 into rust-lang:master Apr 7, 2021
@Havvy
Copy link
Contributor

Havvy commented Apr 7, 2021

💟 Thanks!

Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2021
Update books

## nomicon

1 commits in 6fe476943afd53a9a6e91f38a6ea7bb48811d8ff..8551afbb2ca6f5ea37fe58380318b209785e4e02
2021-03-10 07:28:57 +0900 to 2021-04-01 21:58:50 +0900
- Add example of thinking about Send/Sync's soundness (rust-lang/nomicon#259)

## reference

10 commits in fd97729e2d82f8b08d68a31c9bfdf0c37a7fd542..e1abb17cd94cd5a8a374b48e1bc8134a2208ed48
2021-03-28 14:29:19 -0700 to 2021-04-07 08:09:48 -0700
- Update introduction.md (rust-lang/reference#1004)
- clarify UB for raw ptr deref (rust-lang/reference#1000)
- Update lint level documentation. (rust-lang/reference#998)
- Add rustdoc to tool lints. (rust-lang/reference#997)
- Link to ptr::addr_of on raw pointer docs (rust-lang/reference#993)
- apply rust-lang/reference#950 to STYLE.md (rust-lang/reference#980)
- Tuple Passover rust-lang/reference#2 (rust-lang/reference#990)
- Fix typo in macros-by-example.md (rust-lang/reference#996)
- Clarify object safety rules for methods striked from the vtable (rust-lang/reference#965)
- Add const generic args to const contexts. (rust-lang/reference#995)

## rust-by-example

1 commits in 29d91f591c90dd18fdca6d23f1a9caf9c139d0d7..c80f0b09fc15b9251825343be910c08531938ab2
2021-03-23 09:03:39 -0300 to 2021-04-08 10:28:17 -0300
- fix compile bug with panic! (rust-lang/rust-by-example#1433)

## rustc-dev-guide

11 commits in 0687daac28939c476df51778f5a1d1aff1a3fddf..a9bd2bbf31e4f92b5d3d8e80b22839d0cc7a2022
2021-03-28 13:33:56 -0400 to 2021-04-09 18:12:21 -0400
- Improve formatting and update info in "method lookup" section
- Change wording a bit: `module` =&gt; `crate`
- fix typo (rust-lang/rustc-dev-guide#1107)
- fix typo
- Mention CI build of LLVM in build instruction
- Fix rustdocs test command typo (rust-lang/rustc-dev-guide#1103)
- Update the "LLVM updates" section
- Fix a link about Rustdoc internals
- Add quickstart for adding a new optimization (rust-lang/rustc-dev-guide#1094)
- Add back example of {{cwd}} (rust-lang/rustc-dev-guide#1099)
- Document test input normalization

## embedded-book

1 commits in d3f2ace94d51610cf3e3c265705bb8416d37f8e4..569c3391f5c0cc43433bc77831d17f8ff4d76602
2021-03-17 07:53:09 +0000 to 2021-04-07 08:32:11 +0000
- Make it easier to copy and paste example commands.  (rust-embedded/book#289)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2021
Update books

## nomicon

1 commits in 6fe476943afd53a9a6e91f38a6ea7bb48811d8ff..8551afbb2ca6f5ea37fe58380318b209785e4e02
2021-03-10 07:28:57 +0900 to 2021-04-01 21:58:50 +0900
- Add example of thinking about Send/Sync's soundness (rust-lang/nomicon#259)

## reference

10 commits in fd97729e2d82f8b08d68a31c9bfdf0c37a7fd542..e1abb17cd94cd5a8a374b48e1bc8134a2208ed48
2021-03-28 14:29:19 -0700 to 2021-04-07 08:09:48 -0700
- Update introduction.md (rust-lang/reference#1004)
- clarify UB for raw ptr deref (rust-lang/reference#1000)
- Update lint level documentation. (rust-lang/reference#998)
- Add rustdoc to tool lints. (rust-lang/reference#997)
- Link to ptr::addr_of on raw pointer docs (rust-lang/reference#993)
- apply rust-lang/reference#950 to STYLE.md (rust-lang/reference#980)
- Tuple Passover rust-lang/reference#2 (rust-lang/reference#990)
- Fix typo in macros-by-example.md (rust-lang/reference#996)
- Clarify object safety rules for methods striked from the vtable (rust-lang/reference#965)
- Add const generic args to const contexts. (rust-lang/reference#995)

## rust-by-example

1 commits in 29d91f591c90dd18fdca6d23f1a9caf9c139d0d7..c80f0b09fc15b9251825343be910c08531938ab2
2021-03-23 09:03:39 -0300 to 2021-04-08 10:28:17 -0300
- fix compile bug with panic! (rust-lang/rust-by-example#1433)

## rustc-dev-guide

11 commits in 0687daac28939c476df51778f5a1d1aff1a3fddf..a9bd2bbf31e4f92b5d3d8e80b22839d0cc7a2022
2021-03-28 13:33:56 -0400 to 2021-04-09 18:12:21 -0400
- Improve formatting and update info in "method lookup" section
- Change wording a bit: `module` =&gt; `crate`
- fix typo (rust-lang/rustc-dev-guide#1107)
- fix typo
- Mention CI build of LLVM in build instruction
- Fix rustdocs test command typo (rust-lang/rustc-dev-guide#1103)
- Update the "LLVM updates" section
- Fix a link about Rustdoc internals
- Add quickstart for adding a new optimization (rust-lang/rustc-dev-guide#1094)
- Add back example of {{cwd}} (rust-lang/rustc-dev-guide#1099)
- Document test input normalization

## embedded-book

1 commits in d3f2ace94d51610cf3e3c265705bb8416d37f8e4..569c3391f5c0cc43433bc77831d17f8ff4d76602
2021-03-17 07:53:09 +0000 to 2021-04-07 08:32:11 +0000
- Make it easier to copy and paste example commands.  (rust-embedded/book#289)
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Apr 14, 2021
Update books

## nomicon

1 commits in 6fe476943afd53a9a6e91f38a6ea7bb48811d8ff..8551afbb2ca6f5ea37fe58380318b209785e4e02
2021-03-10 07:28:57 +0900 to 2021-04-01 21:58:50 +0900
- Add example of thinking about Send/Sync's soundness (rust-lang/nomicon#259)

## reference

10 commits in fd97729e2d82f8b08d68a31c9bfdf0c37a7fd542..e1abb17cd94cd5a8a374b48e1bc8134a2208ed48
2021-03-28 14:29:19 -0700 to 2021-04-07 08:09:48 -0700
- Update introduction.md (rust-lang/reference#1004)
- clarify UB for raw ptr deref (rust-lang/reference#1000)
- Update lint level documentation. (rust-lang/reference#998)
- Add rustdoc to tool lints. (rust-lang/reference#997)
- Link to ptr::addr_of on raw pointer docs (rust-lang/reference#993)
- apply rust-lang/reference#950 to STYLE.md (rust-lang/reference#980)
- Tuple Passover rust-lang/reference#2 (rust-lang/reference#990)
- Fix typo in macros-by-example.md (rust-lang/reference#996)
- Clarify object safety rules for methods striked from the vtable (rust-lang/reference#965)
- Add const generic args to const contexts. (rust-lang/reference#995)

## rust-by-example

1 commits in 29d91f591c90dd18fdca6d23f1a9caf9c139d0d7..c80f0b09fc15b9251825343be910c08531938ab2
2021-03-23 09:03:39 -0300 to 2021-04-08 10:28:17 -0300
- fix compile bug with panic! (rust-lang/rust-by-example#1433)

## rustc-dev-guide

11 commits in 0687daac28939c476df51778f5a1d1aff1a3fddf..a9bd2bbf31e4f92b5d3d8e80b22839d0cc7a2022
2021-03-28 13:33:56 -0400 to 2021-04-09 18:12:21 -0400
- Improve formatting and update info in "method lookup" section
- Change wording a bit: `module` =&gt; `crate`
- fix typo (rust-lang/rustc-dev-guide#1107)
- fix typo
- Mention CI build of LLVM in build instruction
- Fix rustdocs test command typo (rust-lang/rustc-dev-guide#1103)
- Update the "LLVM updates" section
- Fix a link about Rustdoc internals
- Add quickstart for adding a new optimization (rust-lang/rustc-dev-guide#1094)
- Add back example of {{cwd}} (rust-lang/rustc-dev-guide#1099)
- Document test input normalization

## embedded-book

1 commits in d3f2ace94d51610cf3e3c265705bb8416d37f8e4..569c3391f5c0cc43433bc77831d17f8ff4d76602
2021-03-17 07:53:09 +0000 to 2021-04-07 08:32:11 +0000
- Make it easier to copy and paste example commands.  (rust-embedded/book#289)
@RalfJung RalfJung deleted the raw-deref branch June 17, 2021 17:28
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.

4 participants