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

Static variables have different addresses in different crates #8179

Closed
alexcrichton opened this issue Aug 1, 2013 · 7 comments
Closed

Static variables have different addresses in different crates #8179

alexcrichton opened this issue Aug 1, 2013 · 7 comments
Labels
A-codegen Area: Code generation

Comments

@alexcrichton
Copy link
Member

// foo.rs
extern mod bar;
fn main() {
  unsafe {
    bar::test(&bar::argh);
  }
}

// bar.rs
use std::cast;
static mut argh: int = 1;

pub unsafe fn test(a: &'static int) {
  let a: uint = cast::transmute(a);
  let b: uint = cast::transmute(&argh);
  assert_eq!(a, b);
}

Currently it appears that whenever a static value is referenced from another crate, a local translation is made and then the address of that is taken instead. This means that every static referenced from another crate will be re-translated into the current crate, thereby giving it a different address than ones in other crates.

For optimizations, I can see where this is nice to know the value of the static, but this breaks any usage of cross-crate TLS apis. Namely, this prevents conditions from working cross-crate. I can think of two fixes for this:

  1. All addresses of external statics are resolved through whatever LLVM's equivalent of extern references are.
  2. Statics can be tagged with some #[] attribute meaning that their address is significant or that they basically shouldn't be re-translated into other crates.

I would prefer option 1 over option 2, but I'm not sure if performance for optimizations would suffer as a result. I think that the fix might go into trans_addr_of, but I'm not sure where the best place for this would be.

cc @graydon (you were interested in cross-crate conditions not working)

@alexcrichton
Copy link
Member Author

Nominating for production-ready, this prevents conditions from being used cross-crate.

I would love for this to be higher priority like backwards-compatible though. I think that the internal libraries would make much more heavy use of conditions if they worked cross-crate (and those changes would be backwards-compatible)

@thestinger
Copy link
Contributor

It's possible to make the value available without creating a new symbol/address as far as I know. Maybe with available_externally + linkonce_odr.

@alexcrichton
Copy link
Member Author

Right now this isn't even an LLVM or linkage problem, this is simply that we re-run trans_const (at least form what I can tell) on external constants. This literally emits the same IR for the static into the local crate and then you take the address from that. Obviously all statics still have the same value, but they're just stored in different locations.

Although for linkage we might need those or other fancy llvm attributes.

@thestinger
Copy link
Contributor

The available_externally attribute can be put on a global (function or not) we inline if we give it the same symbol as the source crate, and LLVM will be able to make optimizations based on the definition and also correctly associate it with the source.

@alexcrichton
Copy link
Member Author

Oh that's perfect! Do we also have to inform LLVM that it's an external symbol as well?

@thestinger
Copy link
Contributor

@alexcrichton: I think available_externally will do everything we need:

Globals with “available_externally” linkage are never emitted into the object file corresponding to the LLVM module. They exist to allow inlining and other optimizations to take place given knowledge of the definition of the global, which is known to be somewhere outside the module. Globals with available_externally linkage are allowed to be discarded at will, and are otherwise the same as linkonce_odr. This linkage type is only allowed on definitions, not declarations.

@alexcrichton
Copy link
Member Author

Awesome! I'll see if I can test this out...

@bors bors closed this as completed in ecefeb0 Aug 2, 2013
flip1995 pushed a commit to flip1995/rust that referenced this issue Jan 13, 2022
…Frednet

Extend unused_io_amount to cover async io.

Clippy helpfully warns about code like this, telling you that you
probably meant "write_all":

    fn say_hi<W:Write>(w: &mut W) {
       w.write(b"hello").unwrap();
    }

This patch attempts to extend the lint so it also covers this
case:

    async fn say_hi<W:AsyncWrite>(w: &mut W) {
       w.write(b"hello").await.unwrap();
    }

(I've run into this second case several times in my own programming,
and so have my coworkers, so unless we're especially accident-prone
in this area, it's probably worth addressing?)

Since this is my first attempt at a clippy patch, I've probably
made all kinds of mistakes: please help me fix them?  I'd like
to learn more here.

Open questions I have:

  * Should this be a separate lint from unused_io_amount?  Maybe
    unused_async_io_amount?  If so, how should I structure their
    shared code?
  * Should this cover tokio's AsyncWrite too?
  * Is it okay to write lints for stuff that isn't part of
    the standard library?  I see that "regex" also has lints,
    and I figure that "futures" is probably okay too, since it's
    an official rust-lang repository.
  * What other tests are needed?
  * How should I improve the code?

Thanks for your time!

---

changelog: [`unused_io_amount`] now supports async read and write traits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-codegen Area: Code generation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants