Skip to content

Guard against overflow in codemap::span_to_lines. #24976

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

Closed
wants to merge 1 commit into from

Conversation

pnkfelix
Copy link
Member

Guard against overflow in codemap::span_to_lines.

There a number of recent issues that report the bug here. See e.g. #24761 and #24954.

This change does not fix them; it just makes the assert more immediate (and also injects a more conservative check, in that we are also checking that the files match up).

So, this does not directly fix any bugs, and is expected to actually inject new ICE's. But those will all represent latent bugs that need fixing.

There a number of recent issues that report the bug here.  See
e.g. rust-lang#24761 and rust-lang#24954.

This change does *not* fix them; it just makes the assert more
immediate (and also injects a more conservative check, in that we are
also checking that the files match up).

So, this does not directly fix any bugs, and is expected ot actually
inject new ICE's.  But those will all represent latent bugs that need
fixing.
@rust-highfive
Copy link
Contributor

r? @pcwalton

(rust_highfive has picked a reviewer for you, use r? to override)

@huonw
Copy link
Member

huonw commented Apr 30, 2015

@bors r+

@bors
Copy link
Collaborator

bors commented Apr 30, 2015

📌 Commit 194a6af has been approved by huonw

@bors
Copy link
Collaborator

bors commented Apr 30, 2015

⌛ Testing commit 194a6af with merge 6631de0...

@bors
Copy link
Collaborator

bors commented Apr 30, 2015

💔 Test failed - auto-mac-64-opt

@pnkfelix
Copy link
Member Author

heh, looks like I was a little too optimistic about whether our own test suite would tickle the ICE's that this injects. I'll work on addressing those.

@pnkfelix
Copy link
Member Author

(closing; I'll put up a different PR later that is a bit more aggressive, e.g. by switching to a Result returning interface here, much like span_to_snippet and addressing whatever ICE's I uncover in the process.)

@pnkfelix pnkfelix closed this Apr 30, 2015
bors added a commit that referenced this pull request May 7, 2015
Guard against overflow in `codemap::span_to_lines`.

(Revised/expanded version of PR #24976)

Make `span_to_lines` to return a `Result`.

In `diagnostic`, catch `Err` from `span_to_lines` and print `"(unprintable span)"` instead.

----

There a number of recent issues that report the bug here.  See e.g. #24761 and #24954.

This change *might* fix them. However, that is *not* its main goal. The main goals are:

 1. Make it possible for callers to recover from an error here, and

 2. Insert a more conservative check, in that we are also checking that the files match up.

----

As a drive-by, fix #24997 , which was causing my attempts to `make check-stage1` on an `--enable-debug` build to fail.
bors added a commit that referenced this pull request May 7, 2015
Guard against overflow in `codemap::span_to_lines`.

(Revised/expanded version of PR #24976)

Make `span_to_lines` to return a `Result`.

In `diagnostic`, catch `Err` from `span_to_lines` and print `"(unprintable span)"` instead.

----

There a number of recent issues that report the bug here.  See e.g. #24761 and #24954.

This change *might* fix them. However, that is *not* its main goal. The main goals are:

 1. Make it possible for callers to recover from an error here, and

 2. Insert a more conservative check, in that we are also checking that the files match up.

----

As a drive-by, fix #24997 , which was causing my attempts to `make check-stage1` on an `--enable-debug` build to fail.
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