Skip to content

Avoid underflow in render_source_line - #47677

Merged
bors merged 1 commit into
rust-lang:masterfrom
etaoins:avoid-underflow-in-rennder-source-line
Jan 31, 2018
Merged

Avoid underflow in render_source_line#47677
bors merged 1 commit into
rust-lang:masterfrom
etaoins:avoid-underflow-in-rennder-source-line

Conversation

@etaoins

Copy link
Copy Markdown
Contributor

While testing #47655 I was able to make the compiler panic when it's compiled with debug assertions:

> rustc /dev/null --crate-type proc-macro
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.25.0-dev running on x86_64-apple-darwin
note: run with `RUST_BACKTRACE=1`for a backtrace
thread 'rustc' panicked at 'attempt to subtract with overflow', librustc_errors/emitter.rs:287:49

Without debug assertions the following warning is emitted:

> rustc /dev/null --crate-type proc-macro
warning: unused variable: `registrar`
--> /dev/null:0:1
||
= note: #[warn(unused_variables)] on by default
= note: to avoid this warning, consider using `_registrar` instead

The panic is due to the unused variable warning being spanned to /dev/null:0:1. When render_source_line subtracts 1 from the line number to look up the source line it panics due to underflow. Without debug assertions this would wrap and cause us to return a blank string instead.

Fix by explicitly testing for 0 and exiting early. I'm unsure how to automatically test this now that #46655 has been approved.

While testing #47655 I was able to make the compiler panic
when it's compiled with debug assertions:
```shell
> rustc /dev/null --crate-type proc-macro
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.25.0-dev running on x86_64-apple-darwin
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'attempt to subtract with overflow', librustc_errors/emitter.rs:287:49
```
Without debug assertions the following warning is emitted:
```shell
> rustc /dev/null --crate-type proc-macro
warning: unused variable: `registrar`
--> /dev/null:0:1
|
|
= note: #[warn(unused_variables)] on by default
= note: to avoid this warning, consider using `_registrar` instead
```
The panic is due to the unused variable warning being spanned to
`/dev/null:0:1`. When `render_source_line` subtracts 1 from the line
number to look up the source line it panics due to underflow. Without
debug assertions this would wrap and cause us to return a blank string
instead.
Fix by explicitly testing for 0 and exiting early. I'm unsure how to
automatically test this now that #46655 has been approved.
@rust-highfive

Copy link
Copy Markdown
Contributor

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @pnkfelix (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

@kennytm

Copy link
Copy Markdown
Member

Thanks for the PR! We’ll periodically check in on it to make sure that @pnkfelix or someone else from the team reviews it soon.

@kennytmkennytm added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 24, 2018
@kennytmkennytm added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 31, 2018
@kennytm

Copy link
Copy Markdown
Member

Review ping for you @pnkfelix! Also cc @rust-lang/compiler if anyone want to step in.

@estebank

Copy link
Copy Markdown
Contributor

@bors r+ rollup

@bors

bors commented Jan 31, 2018

Copy link
Copy Markdown
Collaborator

📌 Commit c6377f8 has been approved by estebank

@kennytmkennytm added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jan 31, 2018
@kennytmkennytm mentioned this pull request Jan 31, 2018
kennytm added a commit to kennytm/rust that referenced this pull request Jan 31, 2018
…source-line, r=estebank
Avoid underflow in render_source_line
While testing rust-lang#47655 I was able to make the compiler panic when it's compiled with debug assertions:
```shell
> rustc /dev/null --crate-type proc-macro
error: internal compiler error: unexpected panic
note: the compiler unexpectedly panicked. this is a bug.
note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports
note: rustc 1.25.0-dev running on x86_64-apple-darwin
note: run with `RUST_BACKTRACE=1` for a backtrace
thread 'rustc' panicked at 'attempt to subtract with overflow', librustc_errors/emitter.rs:287:49
```
Without debug assertions the following warning is emitted:
```shell
> rustc /dev/null --crate-type proc-macro
warning: unused variable: `registrar`
--> /dev/null:0:1
|
|
= note: #[warn(unused_variables)] on by default
= note: to avoid this warning, consider using `_registrar` instead
```
The panic is due to the unused variable warning being spanned to `/dev/null:0:1`. When `render_source_line` subtracts 1 from the line number to look up the source line it panics due to underflow. Without debug assertions this would wrap and cause us to return a blank string instead.
Fix by explicitly testing for 0 and exiting early. I'm unsure how to automatically test this now that rust-lang#46655 has been approved.
bors added a commit that referenced this pull request Jan 31, 2018
@bors
bors merged commit c6377f8 into rust-lang:masterJan 31, 2018
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

S-waiting-on-borsStatus: Waiting on bors to run and complete tests. Bors will change the label on completion.T-compilerRelevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@etaoins@rust-highfive@kennytm@estebank@bors@pnkfelix