Help needed on chasing a big ICE of the compiler

help
1

A really unfortunate bug was spotted out.

Playground

fn foo(mut f: Box<dyn FnOnce()>) {
    f()
}
fn main() {}

By focusing on the following piece of the backtrace message:

thread 'main' panicked at 'cannot access a scoped thread local variable without calling `set` first',...
...
   7: <scoped_tls::ScopedKey<T>>::with
   8: syntax_pos::<impl syntax_pos::span_encoding::Span>::macro_backtrace

I limited the scope on crate libsyntax_pos and assumed there is a need for it to defense itself against outer errors.

By looking at the function macro_backtrace I didn't see a direct call to with but an inline function chain self.ctxt().outer() seems do.

Here is the outer function above defined in src/libsyntax_pos/hygiene.rs:

    #[inline]
    pub fn outer(self) -> Mark {
        HygieneData::with(|data| data.syntax_contexts[self.0 as usize].outer_mark)
    }

HygieneData::with is defined in the same file:

    fn with<T, F: FnOnce(&mut HygieneData) -> T>(f: F) -> T {
        GLOBALS.with(|globals| f(&mut *globals.hygiene_data.borrow_mut()))
    }

The error message shows an offensive use of scope_tls, and GLOBALS is defined in lib.rs as the following:

pub struct Globals {
    symbol_interner: Lock<symbol::Interner>,
    span_interner: Lock<span_encoding::SpanInterner>,
    hygiene_data: Lock<hygiene::HygieneData>,
}

impl Globals {
    pub fn new() -> Globals {
        Globals {
            symbol_interner: Lock::new(symbol::Interner::fresh()),
            span_interner: Lock::new(span_encoding::SpanInterner::default()),
            hygiene_data: Lock::new(hygiene::HygieneData::new()),
        }
    }
}

scoped_thread_local!(pub static GLOBALS: Globals);

So everything point to the fact that GLOBALS was not in "set" status when called the with method. I think I can try to write defensive code like this:

    fn with<T, F: FnOnce(&mut HygieneData) -> T>(f: F) -> T {
        if !GLOBALS.is_set { GLOBALS.set(Globals::new(), || {
            GLOBALS.with(|globals| f(&mut *globals.hygiene_data.borrow_mut()))
        }); } else {
            GLOBALS.with(|globals| f(&mut *globals.hygiene_data.borrow_mut()))
        }
    }

And this actually avoided this specific error regarding scoped_tls. The error message is now:

thread 'main' panicked at 'no errors encountered even though `delay_span_bug` issued', librustc_errors\lib.rs:314:17

Much better for further investigation.

A few questions arises when I review the above code:

  • scoped_thread_local!(pub static GLOBALS: Globals); is paradoxical: if something is Global it is not scoped.

  • The documentation of scoped_tls says:

// When inserting a value, the value is only in place for the duration
// of the closure specified.
FOO.set(&1, || {
    FOO.with(|slot| {
        assert_eq!(*slot, 1);
    });
});

The use cases in libsyntax_pos of GLOBALS are however, all without the set part like the already demostrated.

  • GLOBALS is public and so it must have been set in other crates such that we didn't see this problem before. But in this case, I think it was reset during stack unwinding. So I am not sure my "fix" is correct: would this "fix" lost the context set in previous stages?

  • I feel like GLOBALS should not be pub. We can instead have a function globals that returns a set guaranteed instance, as a "global" fix of the code.

Anyways, I need help on clarify my understanding as the above, to help contribution on fixing this issue.

2

Now I think I found the proper fix.

In src/librustc_driver/lib.rs, the following

pub fn run<F>(...
{
   ...
       let (result, session) = run_compiler();
   ...
}
pub fn run_compiler<'a>(...
{
    syntax::with_globals(|| {
       ...
    })
}

should be

pub fn run<F>(...
{
    ...
    syntax::with_globals(|| {
       let (result,session) = run_compiler();
       ...
    })
   ...
}
pub fn run_compiler<'a>(...
{
   ...
}

The GLOBALS variable in the previous post, was set in the context of the with_globals call. However, as this call didn't conver the full run function, the variable was not set in the rest of the code. And that code does panic or other stuff, so when it do need to use the GLOBALS variable, it is not set.

This is my very first pull request.