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

Make bridge variables respect scopes better #1

Closed
jeremydavis519 opened this issue Apr 26, 2018 · 5 comments
Closed

Make bridge variables respect scopes better #1

jeremydavis519 opened this issue Apr 26, 2018 · 5 comments
Assignees
Labels
bug Something isn't working

Comments

@jeremydavis519
Copy link
Owner

jeremydavis519 commented Apr 26, 2018

Currently, rusty_asm! assumes that bridge variables will always be defined in the same scope as where they're used. This assumption leads to incorrect parsing in two kinds of scenarios:

  1. A rusty_asm! block inside another rusty_asm! block. The inner block should have access to all of the bridge variables defined so far by the outer block (since they're still in scope, according to Rust), but currently, attempting to use them in ASM in that situation fails.

    This is fixable by making rusty_asm_internal!, which would be exactly like the current rusty_asm! except that it would expect a list of inputs, outputs, and clobbers before its statements. rusty_asm! would call rusty_asm_internal! with an empty list, and rusty_asm_internal! would translate all interior calls to rusty_asm! into calls to rusty_asm_internal!.

rusty_asm! {
    let mut a: out("r");
    rusty_asm! {
        asm { "mov %42, $a" } // The parser doesn't recognize `a` here.
    }
}
  1. Declaring a bridge variable in an inner scope and then referencing it after Rust has dropped it. This isn't dangerous in general, since Rust will still fail to compile if we reference a nonexistent variable in asm!, but it is dangerous if that variable was shadowing another that has different in/out semantics. This example, for instance, tells the compiler that
let a: usize = 10;
let b: usize = 25;
rusty_asm! {
    let c: out("r") = a;
    {
        let c: in("r") = b;
        let d: inout("r") = a;
        asm("intel") { "add $d, $c" }
        assert_eq!(d, 35);
    }
    
    // Incorrectly expands to `asm!("add $0, 500" :: "r"(c) :: "intel")`, which
    // tells the compiler that it doesn't need to worry about making sure
    // that `c` is equal to the sum at the end.
    asm("intel") { "add $c, 500" }
    
    assert_eq!(c, 510); // This assertion might pass? We have no way of knowing. `c` is undefined.
}
@jeremydavis519 jeremydavis519 added the bug Something isn't working label Apr 26, 2018
@jeremydavis519 jeremydavis519 self-assigned this Apr 26, 2018
@jeremydavis519
Copy link
Owner Author

On further thought (and trying to fix it), I've realized that problem 1 in the OP can't be solved by a macro (so, for the purposes of this crate, it can't be solved). That's alright, since it's an odd way to use rusty_asm! and doesn't break any of Rust's guarantees even with the current behavior. The reason it can't be solved by a macro is that rusty_asm! could be aliased with literally any fully-qualified or unqualified name, so we can't detect it in order to pass around the inputs, outputs, and clobbers.

The second one, however, is definitely possible. We just need to keep track of every time we enter a new scope. And, come to think of it, we should be doing that anyway! If we don't, this won't compile:

rusty_asm! {
    if condition { // The current parser sees this as one statement and glosses over it,
        asm {      // never seeing the `asm` block.
           "cli
            hlt"
        }
    }
}

@jeremydavis519
Copy link
Owner Author

Because of how Syn works, it's actually really hard to parse the internals of just any arbitrary block. I have an idea for how to extend Syn to allow it, but it's complicated and not backwards-compatible. For now, there is a workaround, which I've added to the documentation: whenever an inner block prevents bridge variables or asm blocks from being understood, put a new invocation of rusty_asm! inside that block and redefine any relevant bridge variables. And sometimes, a loop will require the redefinitions to use different names so the values of the variables can be preserved across iterations. Not pretty, but it works.

@jeremydavis519
Copy link
Owner Author

New idea: We don't need to confirm that everything parses correctly--we only need to find let and asm in the token stream and retain unaltered all tokens that don't fall within our overloaded patterns. There are a few edge cases to keep in mind, though (if I find more, I'll add them here):

  • if and while can be followed by let, and after this change to Rust is stabilized, it won't necessarily be the next token. Fortunately, if and while are always followed (eventually) by blocks, and all blocks are surrounded by braces. So when we find if and while, we need to look for let bindings until we get to the braces, then apply any bridge variables to the inner scope.

Unfortunately, this change will require a significant restructuring of the code.

@jeremydavis519
Copy link
Owner Author

Also, we only need to look for let or asm at the beginning of a block, after a semicolon or closing brace, and (in the case of let) between if or while and the corresponding opening brace. Anywhere else, we should just copy tokens.

@jeremydavis519
Copy link
Owner Author

Implemented. I ended up just looking for every keyword everywhere, except between if or while and the opening brace. Since Rust doesn't allow explicit types in if let and while let and I realized the syntax would probably be too complex, it's not possible to declare a bridge variable in either of those constructions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant