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

Trying to expand macro in extern block causes excess memory usage #54441

Closed
WarpspeedSCP opened this issue Sep 21, 2018 · 5 comments · Fixed by #54833
Closed

Trying to expand macro in extern block causes excess memory usage #54441

WarpspeedSCP opened this issue Sep 21, 2018 · 5 comments · Fixed by #54833

Comments

@WarpspeedSCP
Copy link

rustc --version --verbose output:

rustc 1.30.0-nightly (3bc2ca7e4 2018-09-20)
binary: rustc
commit-hash: 3bc2ca7e4f8507f82a4c357ee19300166bcd8099
commit-date: 2018-09-20
host: x86_64-unknown-linux-gnu
release: 1.30.0-nightly
LLVM version: 8.0

While trying to implement Lua in Rust, I wanted to write a macro to make writing function signatures easier. When I tested the macro within an extern "C" block, and ran rustc --pretty expanded -Z unstable-options, my computer froze due to all (8 gigs) of my RAM being used up by rustc . An MCVE-

#![feature(macros_in_extern)]
#![feature(concat_idents)]

macro_rules! lua_func {
    ($name: ident, $ret: ty, $var: ident, $type: ty) => {
        let fn_name = concat_idents!(lua_, $name);
        pub fn fn_name (L: *mut lua_State, $var: $type) -> $ret
    };
}

extern "C" {
    lua_func!(toboolean, bool, idx, int);
}
@dtolnay
Copy link
Member

dtolnay commented Oct 1, 2018

Minimized:

#![feature(macros_in_extern)]

macro_rules! m {
    () => {
        let
    };
}

extern "C" {
    m!();
}

@abonander
Copy link
Contributor

This might well be my fault, so I'll try to look at it sometime this week.

@abonander
Copy link
Contributor

abonander commented Oct 4, 2018

I've been able to repro the issue but I haven't run it through an allocation profiler yet*. The latest master doesn't like to build with my local Rust install so I spent a lot of time last night trying to figure out why but it finally worked when I gave up and just let it download a snapshot for stage 0.

@abonander
Copy link
Contributor

abonander commented Oct 5, 2018

So far it looks like the leak involves Parser::parse_foreign_item() calling Parser::check_keyword() repeatedly (which pushes to Parser::expected_keywords every time), because parse_foreign_item() isn't erroring and isn't advancing the parser state and thus this loop runs forever.

Simple fix is to check at the end of parse_foreign_item() what the current token is and error if it's not a closing brace or Eof.

@abonander
Copy link
Contributor

Parser::parse_foreign_item() is a bit of an enimga in that it returns Result<Option<ForeignItem>> whereas the analogues for TraitItem and ImplItem are required to return an item or error, and the caller is responsible for detecting EOF. I think that caused some confusion in ext/expand.rs as a result. parse_foreign_item() should be refactored to have the same obligatory return.

abonander added a commit to abonander/rust that referenced this issue Oct 5, 2018
pietroalbini added a commit to pietroalbini/rust that referenced this issue Oct 5, 2018
make `Parser::parse_foreign_item()` return a foreign item or error

Fixes `Parser::parse_foreign_item()` to follow the convention of `parse_trait_item()` and `parse_impl_item()` in that it *must* parse an item or return an error, and then the caller is responsible for detecting the closing delimiter.

This prevents it from looping endlessly on an unexpected token in `ext/expand.rs` where it was also leaking memory by continually pushing to `Parser::expected_tokens` via `Parser::check_keyword()`.

closes rust-lang#54441

r? @petrochenkov
cc @dtolnay
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 a pull request may close this issue.

3 participants