Skip to content

Infinite recursion in macro expansion crashes rustc #17628

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

Closed
uwap opened this issue Sep 29, 2014 · 15 comments
Closed

Infinite recursion in macro expansion crashes rustc #17628

uwap opened this issue Sep 29, 2014 · 15 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.

Comments

@uwap
Copy link

uwap commented Sep 29, 2014

Defining macros recursively like so

macro_rules! recursive(
  () => (
    recursive!()
  )
)

fn main() {
  recursive!()
}

will lead rustc to cause a stack overflow.
It would be useful to improve the error message for that problem.
There is currently no error message specified for that issue.

@kmcallister kmcallister changed the title Improve error messages on recursive macro definitions Infinite recursion in macro expansion crashes rustc Sep 29, 2014
@kmcallister kmcallister added A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. A-diagnostics Area: Messages for errors, warnings, and lints labels Sep 29, 2014
@kmcallister
Copy link
Contributor

We already have a recursion_limit field in Session. We can copy it to libsyntax's ExpansionConfig and then track the recursion depth in ExtCtxt.

@kmcallister kmcallister added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Sep 29, 2014
@fhahn
Copy link
Contributor

fhahn commented Sep 29, 2014

I am looking into this issue. Howerver it seems like the same ExtCtxt and MacroExpander are used for macro expansion, so it seems like recursion depth cannot be tracked naively.

@kmcallister
Copy link
Contributor

The ExtCtxt methods bt_push and bt_pop already maintain a sort of stack of macro invocations. They can also maintain a counter so that we don't have to walk that stack on every check (it's a kind of indirect linked list).

bors added a commit that referenced this issue Oct 1, 2014
…lexcrichton

This is a patch for #17628, thanks to @kmcallister for your helpful hints!
@bors bors closed this as completed in 49e976d Oct 1, 2014
@donkopotamus
Copy link
Contributor

@fhahn Now that recursion depth is limited, is there anyway to configure it from the command line? The current depth setting (64?) is too aggressive for some macro magic I am working on, that recurses heavily.

@uwap
Copy link
Author

uwap commented Oct 15, 2014

I could at least increase it to 256 if that would help. Other than that someone else (@fhan maybe) should look into it.

@fhahn
Copy link
Contributor

fhahn commented Oct 15, 2014

My patch did not include any method to customize the depth limit. I just used 64 because Session.recursion_limit defaults to that as well. I am not aware of a command line interface to change session attributes, but I am not very familiar with rustc`s command line interface.

Another way to customize the recursion depth for macro expansions could be using something like #[recursion_limit(256)] for macros which result in deeper recursions, but no in an infinite recursion.

Anyhow, if there is a decision to do it one way or another, I'd be happy to work on a patch.

@donkopotamus
Copy link
Contributor

Could we at least for now bump the limit up to something much larger? The current setting of 64 is probably too low for macros that rely on reduction rules. (ie that match a portion of a statement and pass the remainder to another macro (often itself) for further expansion).

I don't have a good handle on the expense of each macro expansion in terms of compiler memory/time ... but is there any reason for it not to be a number on a much larger scale, such as 1024?

@donkopotamus
Copy link
Contributor

I like the idea of a file/crate attribute to govern the recursion depth of macros expanded within that crate.

@uwap
Copy link
Author

uwap commented Oct 16, 2014

I will try to increase it to 2048. Someone else should maybe look into the attribute or command line argument thing.

@uwap
Copy link
Author

uwap commented Oct 16, 2014

Seems that 2048 is way to high and already causes stack overflows. If just the compiler wouldn't compile forever...

@donkopotamus
Copy link
Contributor

@uwap Seems surprising ... are you changing the ExpansionConfig.recursion_limit to be 2048, or changing it at the Session level as well?

@uwap
Copy link
Author

uwap commented Oct 16, 2014

Just the ExpansionConfig limit, since the session limit won't go crazy way too far. I guess 1024 is still ok for macros. It is compiling and testing right now, so I can tell you more later.

@donkopotamus
Copy link
Contributor

Ah, I misunderstood you, I thought you meant the overflows were happening when compiling the compiler! You mean rather that an infinite recursion test will overflow before it hits the 2048 limit I think.

@uwap
Copy link
Author

uwap commented Oct 16, 2014

Yes

@uwap
Copy link
Author

uwap commented Oct 20, 2014

I got a workaround, but I guess it is not the best way. Sadly I got ill, so I won't be able to work on it further. Maybe someone else could go on? /cc @fhahn (uwap@47a78a9)

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 A-macros Area: All kinds of macros (custom derive, macro_rules!, proc macros, ..) E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics.
Projects
None yet
Development

No branches or pull requests

4 participants