Improve miri's error reporting in check_in_alloc

[RalfJung]

The check_in_alloc function currently takes a InboundsCheck for error reporting purposes. That's suboptimal for two reasons: (a) passing InboundsCheck::Live/MaybeDead sounds like it would affect the check that is performed, but it does not; and (b) the error still doesn't actually say what we tried to do with this pointer (access memory, in-bounds arithmetic, pointer equality test, ...).

The InboundsCheck argument should be replaced by something else, and that something else should also be put into the PointerOutOfBounds variant of EvalErrorKind. That could either be an &'static str or a more informative enum. I'd probably start with the string, since it seems unlikely we will want to use this for anything but displaying errors to the user, and it seems okay to not be able to use format!.

Cc @oli-obk

[FrankSD]

Can i please take this up?I am a first timer contributor

[FrankSD]

I just forked and cloned rust-lang project on my local but my local branch folder rust/src/librustc/mir/interpret/ is missing pointer.rs and allocation.rs files.The pointer.rs file does contain the check_in_alloc function.It seems i am missing something basic here....my understanding all the code in the current master are uptodate.I clone the current master branch

[RalfJung]

@FrankSD sure, you can give it a try. Let's see if we can start by getting Rust to compile on your system :)

my local branch folder rust/src/librustc/mir/interpret/ is missing pointer.rs and allocation.rs files

What does git log say is the latest commit?

Also, did you maybe accidentally look into src/librustc_mir/interpret? Mind the _ vs /.

If you are on Linux/macOS, you can always do find -name pointer.rs:

$ find -name pointer.rs
./src/librustc/mir/interpret/pointer.rs

[FrankSD]

git log last commit:

 frank@frank-desktop ~/new-rust/rust $ git log
commit caed80ba4ba8d9f4d3fa8aa9af6c4092d779cd9d
Merge: ede5551 002f03b
Author: bors <bors@rust-lang.org>
Date:   Sun Aug 26 09:41:28 2018 +0000

   Auto merge of #53629 - nnethercote:lazier-SparseBitMatrix, r=nikomatsakis
   
   Lazier sparse bit matrix
   
   A small NLL win.
   
   r? @nikomatsakis

And find -name pointer.rs doesnt return anything

rank@frank-desktop ~/new-rust/rust $ find -name pointer.rs

[RalfJung]

Date: Sun Aug 26 09:41:28 2018 +0000

Your compiler is 4 months outdated. Try getting the latest version, it will have the files you are missing.

Maybe what happened is that you forked rustc 4 months ago, and then you cloned your fork. Forks don't get updated when the main rsutc repo moves along. You should always clone the main rustc repo, and only push to your fork for creating PRs.

[FrankSD]

Thank you@RalfJung....that make sense

[FrankSD]

It compiles fine and all the files i can see now....but it fails when trying to install. I have resolved most of the issues but trying to install bootstrap,did fails with :-
"

 "failed to run: /home/frank/new-rust/rust/build/bootstrap/debug/bootstrap install
Build completed unsuccessfully in 0:00:01

[FrankSD]

I have resolved the build issue.I am starting to check the Issue

[RalfJung]

Notice also that there is no reason to install your locally built Rust (I never did that, didn't even know it is possible). Why did you even want to do that?

[FrankSD]

I was following the official Rust Contribution guide where it mention do
x.py build && x.py install .

[RalfJung]

Could you send me the URL to that? At https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md it doesn't say anything about x.py install.

[FrankSD]

Sorry i think this was from the Readme.md https://github.com/rust-lang/rust/blob/master/README.md

[RalfJung]

Oh I see. That's instructions for installing Rust, not for doing development on rustc itself. The CONTRIBUTING.md I linked above is a better place for you to start.

[FrankSD]

@RalfJung.........Can you please guide on reproducing this Issue.?

[RalfJung]

I am not sure what you are asking. This issue is about refactoring check_in_alloc. Fixing this issue will have very minimal effect on the observable behavior of the compiler (some CTFE [compile-time function evaluation] error messages might change, but that's not even the goal).

So there's nothing to reproduce.

[FrankSD]

Got it.Sorry i am relatively new to rust so i am learning along the way.

[RalfJung]

So, do you understand what this issue is about, and what it takes to fix it?

[FrankSD]

I do understand what this issue is all about. Correct me if I am wrong, what I understand is: instead of using InboundsCheck type, use &'static str type to report errors. I think one thing I am still trying to understand is how "check_in_alloc function work" specifically when "PointerOutOfBounds" comes into a picture. So before I can touch any code I need to wrap my head around the big picture. As I said I am totally newbie to Rust... actually i am learning alot now. And I am excited to make my first Pull Request ever :). I will appreciate if you can enlighten me along the way. Thanks Sir.

[RalfJung]

Correct me if I am wrong, what I understand is: instead of using InboundsCheck type, use &'static str type to report errors

Yes. :)

Here's a somewhat more detailed list of steps to follow for this PR:

• Replace InboundsCheck by &'static str here and here. Also rename the check field to something else, maybe msg or details.

• You can now compile with ./x.py --stage 1 build src/libstd. This change will have a bunch of fallout, the rest of the PR is about making things compile again.

• Some places will complain about the field rename, that should be clear from the error message.

  Other places will be calling check_in_alloc with the wrong type for the last argument. Replace those by strings, messages explaining in what context this out-of-bounds pointer occurred -- was it doing a memory access, just pointer arithmetic, or what? If you don't know, put in some dummy text just to make it compile. We can fix that later :)

I hope this will get you started. Never hesitate to ask!

(I will be on a conference for the next 10 days, so I might not be able to reply before I come back. Feel free to ask anyone on Discord or Zulip or IRC as well.)

[FrankSD]

Thank you so much@RalfJung

[FrankSD]

Thank you for letting me know about Discord and Zulip..........

[FrankSD]

i have fixed most of the errors below is the only error i am still working on......Any idea why i am getting " expected identifier" Error
--> src/librustc/mir/interpret/pointer.rs:5:32
|
5 | AllocId, EvalResult, msg::&'static str="To be determined"
| ^ expected identifier

[oli-obk]

Looks like a rebase or search-and-replace gone wrong to me, can you open a PR so we can have a look at the code?

[FrankSD]

I dont think a rebase or search and replace gone wrong......... i am simply asking what is the correct rust syntax for ``` --> src/librustc/mir/interpret/pointer.rs:5:32 change?

[RalfJung]

That's hard to say without any context. Could you push this somewhere and point us to the right branch?

[saleemjaffer]

@RalfJung Can I take a shot at this issue? Newbie to rust.

[RalfJung]

@FrankSD what is your current status on this?

[FrankSD]

Sorry I am working on this..i hv been busy with my current job but I should submit a pull request shortly

…

On Feb 14, 2019 2:17 AM, "Ralf Jung" ***@***.***> wrote: @FrankSD <https://github.com/FrankSD> what is your current status on this? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#57128 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFXIJCWPSVKIOZ3YyJ_oqhUJqTsXadRnks5vNRt_gaJpZM4ZhsUa> .

[FrankSD]

Sorry guys i have been so busy with my current work.Can someone please take this on?I have given it a short several times but i keep getting lots of errors on my branch.I would like to see when someone fixes this issue.Thanks for your understanding

[LooMaclin]

@FrankSD np, don't worry about this, i take it

[LooMaclin]

@RalfJung @oli-obk

1. &'static str doesn't implementing Decodable (parent enum marked as RustcDecodable)
2. so, I created enum that represents all messages and implementsDisplay
3. I found use of InboundsCheck in logic and it confused me: https://github.com/rust-lang/rust/blob/master/src/librustc_mir/interpret/memory.rs#L442-L450

[RalfJung]

(I'm traveling, will come back to you next weekend.)

[RalfJung]

&'static str doesn't implementing Decodable (parent enum marked as RustcDecodable)

Ouch. @oli-obk any suggestions? I don't like the overhead of a new enum for this, that makes adding a new usage site very invoncenient.

I found use of InboundsCheck in logic and it confused me: /src/librustc_mir/interpret/memory.rs@master#L442-L450

Indeed the argument is meaningful for check_bounds_ptr and get_size_and_align, but not for check_in_alloc. So it should only be removed from the latter.

[oli-obk]

We can just use String. No need to be efficient in the error case.

[oli-obk]

Hmm... actually we do need to be efficient, since we catch errors. Alternatively we can use a Cow, but enums are the best choice here oterhwise.