minor tweaks and comments

Author: RalfJung

src/concurrency/thread.rs

@@ -677,6 +677,8 @@ trait EvalContextPrivExt<'tcx>: MiriInterpCxExt<'tcx> {
     fn run_on_stack_empty(&mut self) -> InterpResult<'tcx, Poll<()>> {
         let this = self.eval_context_mut();
         // Inform GenMC that a thread has finished all user code. GenMC needs to know this for scheduling.
+        // FIXME(GenMC): Thread-local destructors *are* user code, so this is odd. Also now that we
+        // support pre-main constructors, it can get called there as well.
         if let Some(genmc_ctx) = this.machine.data_race.as_genmc_ref() {
             let thread_id = this.active_thread();
             genmc_ctx.handle_thread_stack_empty(thread_id);

src/eval.rs

@@ -18,7 +18,7 @@ use rustc_session::config::EntryFnType;
 use crate::concurrency::GenmcCtx;
 use crate::concurrency::thread::TlsAllocAction;
 use crate::diagnostics::report_leaks;
-use crate::shims::{ctor, tls};
+use crate::shims::{global_ctor, tls};
 use crate::*;
 
 #[derive(Copy, Clone, Debug)]
@@ -219,9 +219,11 @@ impl Default for MiriConfig {
 #[derive(Debug)]
 enum MainThreadState<'tcx> {
     GlobalCtors {
-        ctor_state: ctor::GlobalCtorState<'tcx>,
+        ctor_state: global_ctor::GlobalCtorState<'tcx>,
+        /// The main function to call.
         entry_id: DefId,
         entry_type: MiriEntryFnType,
+        /// Arguments passed to `main`.
         argc: ImmTy<'tcx>,
         argv: ImmTy<'tcx>,
     },
@@ -324,12 +326,9 @@ pub fn create_ecx<'tcx>(
         MiriMachine::new(config, layout_cx, genmc_ctx),
     );
 
-    // First argument is constructed later, because it's skipped for `miri_start.`
-
-    // Second argument (argc): length of `config.args`.
+    // Compute argc and argv from `config.args`.
     let argc =
         ImmTy::from_int(i64::try_from(config.args.len()).unwrap(), ecx.machine.layouts.isize);
-    // Third argument (`argv`): created from `config.args`.
     let argv = {
         // Put each argument in memory, collect pointers.
         let mut argvs = Vec::<Immediate<Provenance>>::with_capacity(config.args.len());
@@ -354,7 +353,7 @@ pub fn create_ecx<'tcx>(
             ecx.write_immediate(arg, &place)?;
         }
         ecx.mark_immutable(&argvs_place);
-        // Store `argc` and `argv` for macOS `_NSGetArg{c,v}`.
+        // Store `argc` and `argv` for macOS `_NSGetArg{c,v}`, and for the GC to see them.
         {
             let argc_place =
                 ecx.allocate(ecx.machine.layouts.isize, MiriMemoryKind::Machine.into())?;
@@ -369,7 +368,7 @@ pub fn create_ecx<'tcx>(
             ecx.machine.argv = Some(argv_place.ptr());
         }
         // Store command line as UTF-16 for Windows `GetCommandLineW`.
-        {
+        if tcx.sess.target.os == "windows" {
             // Construct a command string with all the arguments.
             let cmd_utf16: Vec<u16> = args_to_utf16_command_string(config.args.iter());
 
@@ -392,16 +391,18 @@ pub fn create_ecx<'tcx>(
 
     // Some parts of initialization require a full `InterpCx`.
     MiriMachine::late_init(&mut ecx, config, {
-        let mut state = MainThreadState::GlobalCtors {
+        let mut main_thread_state = MainThreadState::GlobalCtors {
             entry_id,
             entry_type,
             argc,
             argv,
-            ctor_state: ctor::GlobalCtorState::default(),
+            ctor_state: global_ctor::GlobalCtorState::default(),
         };
 
         // Cannot capture anything GC-relevant here.
-        Box::new(move |m| state.on_main_stack_empty(m))
+        // `argc` and `argv` *are* GC_relevant, but they also get stored in `machine.argc` and
+        // `machine.argv` so we are good.
+        Box::new(move |m| main_thread_state.on_main_stack_empty(m))
     })?;
 
     // Make sure we have MIR. We check MIR for some stable monomorphic function in libcore.

src/shims/ctor.rs renamed to src/shims/global_ctor.rs

@@ -45,8 +45,11 @@ impl<'tcx> GlobalCtorState<'tcx> {
 
                                 segment_name == Some("__DATA")
                                     && section_name == Some("__mod_init_func")
-                                    // The `mod_init_funcs` directive ensures that the `S_MOD_INIT_FUNC_POINTERS` flag
-                                    // is set on the section, but it is not strictly required.
+                                    // The `mod_init_funcs` directive ensures that the
+                                    // `S_MOD_INIT_FUNC_POINTERS` flag is set on the section. LLVM
+                                    // adds this automatically so we currently do not require it.
+                                    // FIXME: is this guaranteed LLVM behavior? If not, we shouldn't
+                                    // implicitly add it here.
                                     && matches!(section_type, None | Some("mod_init_funcs"))
                             })?,
 

src/shims/mod.rs

@@ -11,10 +11,10 @@ mod wasi;
 mod windows;
 mod x86;
 
-pub mod ctor;
 pub mod env;
 pub mod extern_static;
 pub mod foreign_items;
+pub mod global_ctor;
 pub mod io_error;
 pub mod os_str;
 pub mod panic;