Fix UB caused by interactions with null streams

diff --git a/src/machine/lib_machine/mod.rs b/src/machine/lib_machine/mod.rs
index 24f22fe78ad701ba4ae998aed33bd426f81b0d77..35668e64e87ec4586ead74de9d8f833ca0dc89ac 100644 (file)
--- a/src/machine/lib_machine/mod.rs
+++ b/src/machine/lib_machine/mod.rs
@@ -535,7 +535,7 @@ impl Machine {
     /// Consults a module into the [`Machine`] from a string.
     pub fn consult_module_string(&mut self, module_name: &str, program: impl Into<String>) {
         let stream = Stream::from_owned_string(program.into(), &mut self.machine_st.arena);
-        self.machine_st.registers[1] = stream_as_cell!(stream);
+        self.machine_st.registers[1] = stream.into();
         self.machine_st.registers[2] = atom_as_cell!(&atom_table::AtomTable::build_with(
             &self.machine_st.atom_tbl,
             module_name

diff --git a/src/machine/mod.rs b/src/machine/mod.rs
index a62c0caa8e79fc6db41d9b9a5bf7e5526800a809..0313a2d90214a7f75755b62cb64090b907644657 100644 (file)
--- a/src/machine/mod.rs
+++ b/src/machine/mod.rs
@@ -291,7 +291,7 @@ impl Machine {
     }
 
     fn load_file(&mut self, path: &str, stream: Stream) {
-        self.machine_st.registers[1] = stream_as_cell!(stream);
+        self.machine_st.registers[1] = stream.into();
         self.machine_st.registers[2] =
             atom_as_cell!(AtomTable::build_with(&self.machine_st.atom_tbl, path));
 

diff --git a/src/machine/streams.rs b/src/machine/streams.rs
index d434c569a7e5554777e86a4a8e093e617de2a1f5..7b616b920fdff3b823441e52473584f8074440cd 100644 (file)
--- a/src/machine/streams.rs
+++ b/src/machine/streams.rs
@@ -982,6 +982,20 @@ fn cursor_position<T>(
     }
 }
 
+impl From<Stream> for HeapCellValue {
+    #[inline(always)]
+    fn from(stream: Stream) -> Self {
+        if stream.is_null_stream() {
+            let res = atom!("null_stream");
+            atom_as_cell!(res)
+        } else {
+            let res = stream.as_ptr();
+            debug_assert!(!res.is_null());
+            raw_ptr_as_cell!(res)
+        }
+    }
+}
+
 impl Stream {
     #[inline]
     pub(crate) fn position(&mut self) -> Option<(u64, usize)> {
@@ -1626,7 +1640,7 @@ impl MachineState {
                             match_untyped_arena_ptr!(ptr,
                                 (ArenaHeaderTag::Stream, stream) => {
                                     return if stream.is_null_stream() {
-                                        Err(self.open_permission_error(stream_as_cell!(stream), caller, arity))
+                                        Err(self.open_permission_error(HeapCellValue::from(stream), caller, arity))
                                     } else {
                                         Ok(stream)
                                     };
@@ -1689,7 +1703,7 @@ impl MachineState {
             if let Some(alias) = stream.options().get_alias() {
                 atom_as_cell!(alias)
             } else {
-                stream_as_cell!(stream)
+                stream.into()
             },
         );
 
@@ -1893,3 +1907,81 @@ impl MachineState {
         }
     }
 }
+
+#[cfg(test)]
+mod test {
+    use super::*;
+    use crate::machine::config::*;
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn current_input_null_stream() {
+        let mut machine = MachineBuilder::new()
+            .with_streams(StreamConfig::in_memory())
+            .build();
+
+        let results = machine.run_query("current_input(S).").collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_ok());
+    }
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn read_null_stream() {
+        let mut machine = MachineBuilder::new()
+            .with_streams(StreamConfig::in_memory())
+            .build();
+
+        let results = machine.run_query("get_code(C).").collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_err());
+    }
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn current_output_null_stream() {
+        // TODO: switch to a proper solution for configuring the machine with null streams
+        // once `StreamConfig` supports it.
+        let mut machine = MachineBuilder::new().build();
+        machine.user_output = Stream::Null(StreamOptions::default());
+        machine.configure_streams();
+
+        let results = machine.run_query("current_output(S).").collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_ok());
+    }
+
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn write_null_stream() {
+        // TODO: switch to a proper solution for configuring the machine with null streams
+        // once `StreamConfig` supports it.
+        let mut machine = MachineBuilder::new().build();
+        machine.user_output = Stream::Null(StreamOptions::default());
+        machine.configure_streams();
+
+        let results = machine.run_query("write(hello).").collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_err());
+    }
+
+    /// A variant of the [`write_null_stream`] that tries to write to a (null) input stream.
+    #[test]
+    #[cfg_attr(miri, ignore)]
+    fn write_null_input_stream() {
+        let mut machine = MachineBuilder::new()
+            .with_streams(StreamConfig::in_memory())
+            .build();
+
+        let results = machine
+            .run_query("current_input(Stream), write(Stream, hello).")
+            .collect::<Vec<_>>();
+
+        assert_eq!(results.len(), 1);
+        assert!(results[0].is_err());
+    }
+}

diff --git a/src/machine/system_calls.rs b/src/machine/system_calls.rs
index 14f9094b8b2596d51c21710e451c89b72347ec95..11fe5c5f18735d4abc85fadec3f6a45b61ab7ec8 100644 (file)
--- a/src/machine/system_calls.rs
+++ b/src/machine/system_calls.rs
@@ -1881,7 +1881,7 @@ impl Machine {
         let stream = self.user_input;
 
         if let Some(var) = addr.as_var() {
-            self.machine_st.bind(var, stream_as_cell!(stream));
+            self.machine_st.bind(var, stream.into());
             return Ok(());
         }
 
@@ -1916,7 +1916,7 @@ impl Machine {
         let stream = self.user_output;
 
         if let Some(var) = addr.as_var() {
-            self.machine_st.bind(var, stream_as_cell!(stream));
+            self.machine_st.bind(var, stream.into());
             return Ok(());
         }
 
@@ -3273,7 +3273,7 @@ impl Machine {
             match stream.write_all(&bytes) {
                 Ok(_) => {}
                 _ => {
-                    let addr = stream_as_cell!(stream);
+                    let addr = stream.into();
                     let err = self
                         .machine_st
                         .existence_error(ExistenceError::Stream(addr));
@@ -3323,7 +3323,7 @@ impl Machine {
                         _ => {
                             let err = self
                                 .machine_st
-                                .existence_error(ExistenceError::Stream(stream_as_cell!(stream)));
+                                .existence_error(ExistenceError::Stream(stream.into()));
 
                             return Err(self.machine_st.error_form(err, stub_gen()));
                         }
@@ -3336,9 +3336,9 @@ impl Machine {
                                 return Ok(());
                             }
                             _ => {
-                                let err = self.machine_st.existence_error(ExistenceError::Stream(
-                                    stream_as_cell!(stream),
-                                ));
+                                let err = self
+                                    .machine_st
+                                    .existence_error(ExistenceError::Stream(stream.into()));
 
                                 return Err(self.machine_st.error_form(err, stub_gen()));
                             }
@@ -3730,7 +3730,7 @@ impl Machine {
         self.indices.streams = self.indices.streams.sub(&null_streams);
 
         if let Some(first_stream) = first_stream {
-            let stream = stream_as_cell!(first_stream);
+            let stream = first_stream.into();
 
             let var = self.deref_register(1).as_var().unwrap();
 
@@ -3761,8 +3761,7 @@ impl Machine {
         if let Some(next_stream) = next_stream {
             let var = self.deref_register(2).as_var().unwrap();
 
-            let next_stream = stream_as_cell!(next_stream);
-            self.machine_st.bind(var, next_stream);
+            self.machine_st.bind(var, next_stream.into());
         } else {
             self.machine_st.fail = true;
         }
@@ -3779,7 +3778,7 @@ impl Machine {
 
         if !stream.is_output_stream() {
             let stub = functor_stub(atom!("flush_output"), 1);
-            let addr = stream_as_cell!(stream);
+            let addr = HeapCellValue::from(stream);
 
             let err =
                 self.machine_st
@@ -3899,7 +3898,7 @@ impl Machine {
 
             if close_result.is_err() {
                 let stub = functor_stub(atom!("close"), 1);
-                let addr = stream_as_cell!(stream);
+                let addr = stream.into();
                 let err = self
                     .machine_st
                     .existence_error(ExistenceError::Stream(addr));
@@ -4472,10 +4471,9 @@ impl Machine {
 
                     self.indices.streams.insert(stream);
 
-                    let stream = stream_as_cell!(stream);
-
                     let stream_addr = self.deref_register(2);
-                    self.machine_st.bind(stream_addr.as_var().unwrap(), stream);
+                    self.machine_st
+                        .bind(stream_addr.as_var().unwrap(), stream.into());
                 }
                 Err(_) => {
                     self.machine_st.fail = true;
@@ -4689,7 +4687,7 @@ impl Machine {
                 *stream.options_mut() = StreamOptions::default();
                 stream.options_mut().set_stream_type(StreamType::Binary);
                 self.indices.streams.insert(stream);
-                let stream = stream_as_cell!(stream);
+                let stream: HeapCellValue = stream.into();
 
                                 let handle: TypedArenaPtr<HttpResponse> = arena_alloc!(request.response, &mut self.machine_st.arena);
 
@@ -4792,27 +4790,26 @@ impl Machine {
 
         read_heap_cell!(culprit,
             (HeapCellValueTag::Cons, cons_ptr) => {
-            match_untyped_arena_ptr!(cons_ptr,
-                (ArenaHeaderTag::HttpResponse, http_response) => {
-                let mut stream = Stream::from_http_sender(
-                    http_response,
-                    status_code,
-                    headers,
-                    &mut self.machine_st.arena
+                match_untyped_arena_ptr!(cons_ptr,
+                    (ArenaHeaderTag::HttpResponse, http_response) => {
+                        let mut stream = Stream::from_http_sender(
+                            http_response,
+                            status_code,
+                            headers,
+                            &mut self.machine_st.arena
+                        );
+                        *stream.options_mut() = StreamOptions::default();
+                        stream.options_mut().set_stream_type(StreamType::Binary);
+                        self.indices.streams.insert(stream);
+                        self.machine_st.bind(stream_addr.as_var().unwrap(), stream.into());
+                    }
+                    _ => {
+                        unreachable!();
+                    }
                 );
-                *stream.options_mut() = StreamOptions::default();
-                stream.options_mut().set_stream_type(StreamType::Binary);
-                self.indices.streams.insert(stream);
-                let stream = stream_as_cell!(stream);
-                self.machine_st.bind(stream_addr.as_var().unwrap(), stream);
-                }
-                _ => {
-                unreachable!();
-                }
-            );
             }
             _ => {
-            unreachable!();
+                unreachable!();
             }
         );
 
@@ -5125,7 +5122,7 @@ impl Machine {
 
             let stream_var = self.deref_register(3);
             self.machine_st
-                .bind(stream_var.as_var().unwrap(), stream_as_cell!(stream));
+                .bind(stream_var.as_var().unwrap(), stream.into());
         } else {
             let err = self
                 .machine_st
@@ -6559,7 +6556,7 @@ impl Machine {
 
                 self.indices.streams.insert(stream);
 
-                stream_as_cell!(stream)
+                HeapCellValue::from(stream)
             }
             Err(ErrorKind::PermissionDenied) => {
                 return Err(self.machine_st.open_permission_error(
@@ -6715,14 +6712,13 @@ impl Machine {
 
                                  self.indices.streams.insert(tcp_stream);
 
-                                 let tcp_stream = stream_as_cell!(tcp_stream);
                                  let client = atom_as_cell!(client);
 
                                  let client_addr = self.deref_register(2);
                                  let stream_addr = self.deref_register(3);
 
                                  self.machine_st.bind(client_addr.as_var().unwrap(), client);
-                                 self.machine_st.bind(stream_addr.as_var().unwrap(), tcp_stream);
+                                 self.machine_st.bind(stream_addr.as_var().unwrap(), tcp_stream.into());
                              }
                              None => {
                                  self.machine_st.fail = true;
@@ -6770,10 +6766,11 @@ impl Machine {
             let stream = Stream::from_tls_stream(addr, stream, &mut self.machine_st.arena);
             self.indices.streams.insert(stream);
 
-            self.machine_st.heap.push(stream_as_cell!(stream));
+            // FIXME: why are we pushing a random, unreferenced cell on the heap?
+            self.machine_st.heap.push(stream.into());
             let stream_addr = self.deref_register(3);
             self.machine_st
-                .bind(stream_addr.as_var().unwrap(), stream_as_cell!(stream));
+                .bind(stream_addr.as_var().unwrap(), stream.into());
 
             Ok(())
         } else {
@@ -6826,7 +6823,7 @@ impl Machine {
 
             let stream_addr = self.deref_register(4);
             self.machine_st
-                .bind(stream_addr.as_var().unwrap(), stream_as_cell!(stream));
+                .bind(stream_addr.as_var().unwrap(), stream.into());
         } else {
             unreachable!();
         }
@@ -6875,7 +6872,7 @@ impl Machine {
             let err = self.machine_st.permission_error(
                 Permission::Reposition,
                 atom!("stream"),
-                stream_as_cell!(stream),
+                HeapCellValue::from(stream),
             );
 
             return Err(self.machine_st.error_form(err, stub));
@@ -8099,7 +8096,7 @@ impl Machine {
                 let lib_stream = Stream::from_static_string(library, &mut self.machine_st.arena);
                 unify!(
                     self.machine_st,
-                    stream_as_cell!(lib_stream),
+                    HeapCellValue::from(lib_stream),
                     self.machine_st.registers[2]
                 );
 

diff --git a/src/macros.rs b/src/macros.rs
index 7778c89e6dc58add8fe3cd7badef50bda24df8f9..6349593046777736523fed26f58bedce7659814a 100644 (file)
--- a/src/macros.rs
+++ b/src/macros.rs
@@ -218,12 +218,6 @@ macro_rules! string_as_pstr_cell {
     }};
 }
 
-macro_rules! stream_as_cell {
-    ($ptr:expr) => {
-        raw_ptr_as_cell!($ptr.as_ptr())
-    };
-}
-
 macro_rules! cell_as_stream {
     ($cell:expr) => {{
         let ptr = cell_as_untyped_arena_ptr!($cell);