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

Bad file/lines in error (assert/fail/alt check) messages from inline functions #1972

Closed
marijnh opened this issue Mar 13, 2012 · 23 comments
Closed
Labels
A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority

Comments

@marijnh
Copy link
Contributor

marijnh commented Mar 13, 2012

Spans of inlined things currently end up pointing at the start of the crate file. This makes them quite useless and confusing.

Example (adapted from pnkfelix's comments below):

% cat -n /tmp/foo.rs
     1  #[link(name="foo", vers="0.0")];
     2  
     3  #[inline(always)]
     4  pub fn access_inline_always(a: &[int], i: uint) -> int {
     5      info!("foo::access_inline_always");
     6      a[i]
     7  }
     8  
     9  #[inline(never)]
    10  pub fn access_inline_never(a: &[int], i: uint) -> int {
    11      info!("foo::access_inline_never");
    12      a[i]
    13  }
% cat -n /tmp/bar.rs
     1  extern mod foo;
     2  
     3  fn main() {
     4      let arr = [1, 2, 3];
     5      let a_0 = foo::access_inline_never(arr, 0);
     6      let a_1 = foo::access_inline_always(arr, 1);
     7      println(format!("arr[0]: {:?} arr[1]: {:?}", a_0, a_1));
     8      let a_4 = foo::access_inline_always(arr, 4);
     9      println(format!("arr[4]: {:?}", a_4));
    10  }
% rustc --cfg debug --lib /tmp/foo.rs
warning: no debug symbols in executable (-arch x86_64)
% rustc -L/tmp /tmp/bar.rs
warning: no debug symbols in executable (-arch x86_64)
% RUST_LOG=foo /tmp/bar
"foo::access_inline_never"
"foo::access_inline_always"
arr[0]: 1 arr[1]: 2
"foo::access_inline_always"
task '<unnamed>' failed at 'index out of bounds: the len is 3 but the index is 4', /tmp/bar.rs:1

Note that the error message points one to /tmp/bar.rs:1 but it should probably point at the vector access itself at line foo.rs:6

@nikomatsakis
Copy link
Contributor

oh yes to do this we have to serialize the codemap info a bit more than we do today (otherwise we cannot interpret the spans)

@marijnh
Copy link
Contributor Author

marijnh commented Mar 13, 2012

the span info will be huge though (and the codemap info also significant). May be better to somehow distinguish 'significant' spans on the few items that output span info to the binary, and only write those out (in explicit line/char/file format).

@nikomatsakis
Copy link
Contributor

We are already serializing all the spans, we just disregard them on deserialization. However, with the new work that I am doing on the serialization process, it should be fairly trivial to customize this so that we do not serialize spans or serialize them in a special way.

@pnkfelix
Copy link
Member

Not critical for 0.6; de-milestoning

@metajack
Copy link
Contributor

metajack commented May 9, 2013

#3740 is a prereq for this and is in the backwards compatible milestone. Nominating for production ready.

@graydon
Copy link
Contributor

graydon commented May 30, 2013

accepted for production-ready milestone

@emberian
Copy link
Member

I don't really understand the issue; how can it be reproduced?

@emberian
Copy link
Member

(@nikomatsakis, @marijnh ?)

@pnkfelix
Copy link
Member

@cmr I'm actually unsure whether this is still broken.

Here is an elaborate attempt to recreate the bug:

% cat -n /tmp/foo.rs
     1  use std::num::Zero;
     2  use std::num::One;
     3  
     4  #[inline(always)]
     5  pub fn add2<I:Integer>(x: I) -> I {
     6      assert!(x >= Zero::zero());
     7      (x + One::one()) + One::one()
     8  }
% cat -n /tmp/bar.rs
     1  extern mod foo;
     2  use foo::*;
     3  
     4  fn main() {
     5      let four = add2(2);
     6      println(fmt!("2+2 == %d", four));
     7      let zero = add2(-2);
     8      println(fmt!("-2+2 == %d", zero));
     9  }
% rustc --lib /tmp/foo.rs && rustc -L/tmp /tmp/bar.rs && /tmp/bar
warning: missing crate link meta `name`, using `foo` as default
warning: missing crate link meta `vers`, using `0.0` as default
warning: no debug symbols in executable (-arch x86_64)
warning: no debug symbols in executable (-arch x86_64)
2+2 == 4
rust: task failed at 'assertion failed: x >= Zero::zero()', /tmp/foo.rs:6
rust: domain main @0x7f965c014c10 root task failed

as you can see, the assertion failure in the call to foo's add2 is indeed printing the correct line number within foo.rs.

(There is an issue that was blocking this that remains unfixed, namely #3740, but that issue was partially fixed, so it may no longer be a blocker for this.)

@nikomatsakis
Copy link
Contributor

Still broken afaik.

@pnkfelix
Copy link
Member

@nikomatsakis can you elaborate what sort of inlined thing one would need to use in another crate to illustrate the bug? I started with something small (which seemed to work to me) and added more and more stuff to it (in the hopes of exposing the bug) until I ended up with the example I wrote above.

Do I need to incorporate uses of -Z debug-info or something?

@nikomatsakis
Copy link
Contributor

I believe your example works because the macro-expander is inserting the filename/line-number information before inlining occurs. I imagine you would see the wrong filename/line-number on an out-of-bounds array access, though.

@pnkfelix
Copy link
Member

@cmr okay @nikomatsakis is correct. Here is a demo of ... well, a semi-related bug. (I forgot to turn on inlining, but I think we see it here due to the parametric definition. Will switch to something that inlines later.)

% cat /tmp/foo.rs
pub fn access<T:Clone>(a: &[T], i: uint) -> T {
    a[i].clone()
}
% cat /tmp/bar.rs
extern mod foo;
use foo::*;

pub fn main() {
    let arr = [1, 2, 3];
    let a_4 = access(arr, 4);
    println(fmt!("arr[4]: %?", a_4));
}
% rustc --lib /tmp/foo.rs && rustc -L/tmp /tmp/bar.rs && /tmp/bar
warning: missing crate link meta `name`, using `foo` as default
warning: missing crate link meta `vers`, using `0.0` as default
warning: no debug symbols in executable (-arch x86_64)
warning: no debug symbols in executable (-arch x86_64)
rust: task failed at 'index out of bounds: the len is 3 but the index is 4', /tmp/bar.rs:1
rust: domain main @0x7fd1e8814c10 root task failed
% 

@pnkfelix
Copy link
Member

@cmr Here is a better example: It shows how the problem arises in the example @nikomatsakis gave (out of bounds array access), but it also shows another instance of the same problem: the info! directive for the inlined functions ends up being associated with the caller module bar rather than the callee module foo where the function is defined, which leads to weird discrepancies for what settings one needs to use for RUST_LOG in order to observe the info! output:

% cat -n /tmp/foo.rs
     1  #[link(name="foo", vers="0.0")];
     2  
     3  #[inline(always)]
     4  pub fn access_inline_always(a: &[int], i: uint) -> int {
     5      info!("foo::access_inline_always");
     6      a[i]
     7  }
     8  
     9  #[inline(never)]
    10  pub fn access_inline_never(a: &[int], i: uint) -> int {
    11      info!("foo::access_inline_never");
    12      a[i]
    13  }
% cat -n /tmp/bar.rs
     1  extern mod foo;
     2  
     3  fn main() {
     4      let arr = [1, 2, 3];
     5      let a_0 = foo::access_inline_never(arr, 0);
     6      let a_1 = foo::access_inline_always(arr, 1);
     7      println(fmt!("arr[0]: %? arr[1]: %?", a_0, a_1));
     8      let a_4 = foo::access_inline_always(arr, 4);
     9      println(fmt!("arr[4]: %?", a_4));
    10  }
% rustc --cfg debug --lib /tmp/foo.rs && rustc -L/tmp /tmp/bar.rs && RUST_LOG=foo=4 /tmp/bar
warning: no debug symbols in executable (-arch x86_64)
warning: no debug symbols in executable (-arch x86_64)
rust: ~"\"foo::access_inline_never\""
arr[0]: 1 arr[1]: 2
rust: task failed at 'index out of bounds: the len is 3 but the index is 4', /tmp/bar.rs:1
rust: domain main @0x7ff333807810 root task failed
% rustc --cfg debug --lib /tmp/foo.rs && rustc -L/tmp /tmp/bar.rs && RUST_LOG=bar=4 /tmp/bar
warning: no debug symbols in executable (-arch x86_64)
warning: no debug symbols in executable (-arch x86_64)
rust: ~"\"foo::access_inline_always\""
arr[0]: 1 arr[1]: 2
rust: ~"\"foo::access_inline_always\""
rust: task failed at 'index out of bounds: the len is 3 but the index is 4', /tmp/bar.rs:1
rust: domain main @0x7fc1b8807810 root task failed
% 

@emberian
Copy link
Member

The inline logging issue is a known problem but I can't find the issue for it.

@catamorphism
Copy link
Contributor

We think this is fixed. Reopen if you have a way to reproduce it.

@nikomatsakis
Copy link
Contributor

Out of curiosity, why do we think this is fixed? Certainly, the spans in inlined ASTs are still invalid:

https://github.com/mozilla/rust/blob/master/src/librustc/middle/astencode.rs#L219

I imagine this would mean that a failure that is generated by trans, such as array deref, would report an incorrect span. @pnkfelix's example seems still relevant.

@pnkfelix
Copy link
Member

My example still shows a problem. (I had to port it to new format! syntax.) I'll update bug description with it.

@pnkfelix pnkfelix reopened this Oct 24, 2013
@emberian
Copy link
Member

Furthermore issues can't just be closed without a test in the suite.

On Thu, Oct 24, 2013 at 7:04 PM, Felix S Klock II
[email protected]:

My example still shows a problem. (I had to port it to new format!syntax.) I'll update bug description with it.


Reply to this email directly or view it on GitHubhttps://github.com//issues/1972#issuecomment-27040689
.

@jdm
Copy link
Contributor

jdm commented Oct 25, 2013

Similarly, any cast::transmute from one type to another that is a different size will trigger this error.

@emberian
Copy link
Member

@nikomatsakis why would this require anything with serializing spans? afaict, this bug is almost entirely about the span of the expression that caused the error not being correct. Am I missing something?

@nikomatsakis
Copy link
Contributor

@cmr the span is incorrect because the AST was serialized into crate metadata and then de-serialized into another crate during trans as part of cross-crate inlining, and we do not currently preserve spans across serialization

michaelwoerister added a commit to michaelwoerister/rust that referenced this issue Jul 2, 2014
…locations from debug info type descriptions.

So far, type names generated for debuginfo where a bit sketchy. It was not clearly defined when a name should be fully qualified and when not, if region parameters should be shown or not, and other things like that.
This commit makes the debuginfo module responsible for creating type names instead of using ppaux::ty_to_str() and brings type names, as they show up in the DWARF information, in line with GCC and Clang:

* The name of the type being described is unqualified. It's path is defined by its position in the namespace hierarchy.
* Type arguments are always fully qualified, no matter if they would actually be in scope at the type definition location.

Care is also taken to reliably make type names consistent across crate boundaries. That is, the code now tries make the type name the same, regardless if the type is in the local crate or reconstructed from metadata. Otherwise LLVM will complain about violating the one-definition-rule when using link-time-optimization.

This commit also removes all source location information from type descriptions because these cannot be reconstructed for types instantiated from metadata. Again, with LTO enabled, this can lead to two versions of the debuginfo type description, one with and one without source location information, which then triggers the LLVM ODR assertion.
Fortunately, source location information about types is rarely used, so this has little impact. Once source location information is preserved in metadata (rust-lang#1972) it can also be reenabled for type descriptions.
bors added a commit that referenced this issue Jul 3, 2014
…=luqmana

So far, type names generated for debuginfo where a bit sketchy. It was not clearly defined when a name should be fully qualified and when not, if region parameters should be shown or not, and other things like that.
This commit makes the debuginfo module responsible for creating type names instead of using `ppaux::ty_to_str()` and brings type names (as they show up in the DWARF information) in line with GCC and Clang:

* The name of the type being described is unqualified. It's path is defined by its position in the namespace hierarchy.
* Type arguments are always fully qualified, no matter if they would actually be in scope at the type definition location.

Care is also taken to make type names consistent across crate boundaries. That is, the code now tries make the type name the same, regardless if the type is in the local crate or reconstructed from metadata. Otherwise LLVM will complain about violating the one-definition-rule when using link-time-optimization.

This commit also removes all source location information from type descriptions because these cannot be reconstructed for types instantiated from metadata. Again, with LTO enabled, this can lead to two versions of the debuginfo type description, one with and one without source location information, which then triggers the LLVM ODR assertion.
Fortunately, source location information about types is rarely used, so this has little impact. Once source location information is preserved in metadata (#1972) it can also be re-enabled for type descriptions.

`RUSTFLAGS=-g make check` no works again for me locally, including the LTO test cases (note that I've taken care of #15156 by reverting the change in LLVM that @luqmana identified as the culprit for that issue).
@steveklabnik
Copy link
Member

Given that #15199 was merged, I'm giving this a close. Please object if you feel differently 😄

celinval added a commit to celinval/rust-dev that referenced this issue Jun 4, 2024
For runs using --verbose, Kani will now print the time spent in some parts of the code. This include calls to all CProver utilities and a few other places in the compiler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints P-low Low priority
Projects
None yet
Development

No branches or pull requests

9 participants