Fix #140

src/header/stdio/printf.rs

@@ -1,7 +1,8 @@
 use alloc::string::String;
 use alloc::string::ToString;
+use alloc::collections::BTreeMap;
 use alloc::vec::Vec;
-use core::ffi::VaList as va_list;
+use core::ffi::VaList;
 use core::ops::Range;
 use core::{fmt, slice};
 use io::{self, Write};
@@ -9,7 +10,15 @@ use io::{self, Write};
 use platform;
 use platform::types::*;
 
-#[derive(PartialEq, Eq)]
+//  ____        _ _                 _       _
+// | __ )  ___ (_) | ___ _ __ _ __ | | __ _| |_ ___ _
+// |  _ \ / _ \| | |/ _ \ '__| '_ \| |/ _` | __/ _ (_)
+// | |_) | (_) | | |  __/ |  | |_) | | (_| | ||  __/_
+// |____/ \___/|_|_|\___|_|  | .__/|_|\__,_|\__\___(_)
+//                           |_|
+
+
+#[derive(Clone, Copy, PartialEq, Eq)]
 enum IntKind {
     Byte,
     Short,
@@ -20,6 +29,7 @@ enum IntKind {
     PtrDiff,
     Size,
 }
+#[derive(Clone, Copy, PartialEq, Eq)]
 enum FmtKind {
     Percent,
 
@@ -42,144 +52,113 @@ enum Number {
     Next
 }
 impl Number {
-    unsafe fn resolve(&self, ap: &mut BufferedVaList) -> usize {
-        match *self {
-            Number::Static(num) => num,
-            Number::Index(i) => ap.index(ArgKind::Int, i).int as usize,
-            Number::Next => ap.next(ArgKind::Int).int as usize,
+    unsafe fn resolve(&self, varargs: &mut VaListCache, ap: &mut VaList) -> usize {
+        let arg = match *self {
+            Number::Static(num) => return num,
+            Number::Index(i) => varargs.get(i-1, ap, None),
+            Number::Next => {
+                let i = varargs.i;
+                varargs.i += 1;
+                varargs.get(i, ap, None)
+            }
+        };
+        match arg {
+            VaArg::c_char(i) => i as usize,
+            VaArg::c_double(i) => i as usize,
+            VaArg::c_int(i) => i as usize,
+            VaArg::c_long(i) => i as usize,
+            VaArg::c_longlong(i) => i as usize,
+            VaArg::c_short(i) => i as usize,
+            VaArg::intmax_t(i) => i as usize,
+            VaArg::pointer(i) => i as usize,
+            VaArg::ptrdiff_t(i) => i as usize,
+            VaArg::ssize_t(i) => i as usize
         }
     }
 }
-enum ArgKind {
-    Byte,
-    Short,
-    Int,
-    Long,
-    LongLong,
-    PtrDiff,
-    Size,
-    IntMax,
-    Double,
-    CharPtr,
-    VoidPtr,
-    IntPtr,
-    ArgDefault,
-}
-
 #[derive(Clone, Copy)]
-union VaArg {
-    byte: c_char,
-    short: c_short,
-    int: c_int,
-    long: c_long,
-    longlong: c_longlong,
-    ptrdiff: ptrdiff_t,
-    size: ssize_t,
-    intmax: intmax_t,
-    double: c_double,
-    char_ptr: *const c_char,
-    void_ptr: *const c_void,
-    int_ptr: *mut c_int,
-    arg_default: usize,
-}
-
-struct BufferedVaList<'a> {
-    list: va_list<'a>,
-    buf: Vec<VaArg>,
-    i: usize,
+enum VaArg {
+    c_char(c_char),
+    c_double(c_double),
+    c_int(c_int),
+    c_long(c_long),
+    c_longlong(c_longlong),
+    c_short(c_short),
+    intmax_t(intmax_t),
+    pointer(*const c_void),
+    ptrdiff_t(ptrdiff_t),
+    ssize_t(ssize_t)
 }
-
-impl<'a> BufferedVaList<'a> {
-    fn new(list: va_list<'a>) -> Self {
-        Self {
-            list,
-            buf: Vec::new(),
-            i: 0,
+impl VaArg {
+    unsafe fn arg_from(arg: &PrintfArg, ap: &mut VaList) -> VaArg {
+        // Per the C standard using va_arg with a type with a size
+        // less than that of an int for integers and double for floats
+        // is invalid. As a result any arguments smaller than an int or
+        // double passed to a function will be promoted to the smallest
+        // possible size. The VaList::arg function will handle this
+        // automagically.
+
+        match (arg.fmtkind, arg.intkind) {
+            (FmtKind::Percent, _) => panic!("Can't call arg_from on %"),
+
+            (FmtKind::Char, _) |
+            (FmtKind::Unsigned, IntKind::Byte) |
+            (FmtKind::Signed, IntKind::Byte) => VaArg::c_char(ap.arg::<c_char>()),
+            (FmtKind::Unsigned, IntKind::Short) |
+            (FmtKind::Signed, IntKind::Short) => VaArg::c_short(ap.arg::<c_short>()),
+            (FmtKind::Unsigned, IntKind::Int) |
+            (FmtKind::Signed, IntKind::Int) => VaArg::c_int(ap.arg::<c_int>()),
+            (FmtKind::Unsigned, IntKind::Long) |
+            (FmtKind::Signed, IntKind::Long) => VaArg::c_long(ap.arg::<c_long>()),
+            (FmtKind::Unsigned, IntKind::LongLong) |
+            (FmtKind::Signed, IntKind::LongLong) => VaArg::c_longlong(ap.arg::<c_longlong>()),
+            (FmtKind::Unsigned, IntKind::IntMax) |
+            (FmtKind::Signed, IntKind::IntMax) => VaArg::intmax_t(ap.arg::<intmax_t>()),
+            (FmtKind::Unsigned, IntKind::PtrDiff) |
+            (FmtKind::Signed, IntKind::PtrDiff) => VaArg::ptrdiff_t(ap.arg::<ptrdiff_t>()),
+            (FmtKind::Unsigned, IntKind::Size) |
+            (FmtKind::Signed, IntKind::Size) => VaArg::ssize_t(ap.arg::<ssize_t>()),
+
+            (FmtKind::AnyNotation, _) | (FmtKind::Decimal, _) | (FmtKind::Scientific, _)
+                => VaArg::c_double(ap.arg::<c_double>()),
+
+            (FmtKind::GetWritten, _) | (FmtKind::Pointer, _) | (FmtKind::String, _)
+                => VaArg::pointer(ap.arg::<*const c_void>()),
         }
     }
-
-    unsafe fn get_arg(&mut self, ty: ArgKind) -> VaArg {
-        match ty {
-            ArgKind::Byte => VaArg {
-                byte: self.list.arg::<c_char>(),
-            },
-            ArgKind::Short => VaArg {
-                short: self.list.arg::<c_short>(),
-            },
-            ArgKind::Int => VaArg {
-                int: self.list.arg::<c_int>(),
-            },
-            ArgKind::Long => VaArg {
-                long: self.list.arg::<c_long>(),
-            },
-            ArgKind::LongLong => VaArg {
-                longlong: self.list.arg::<c_longlong>(),
-            },
-            ArgKind::PtrDiff => VaArg {
-                ptrdiff: self.list.arg::<ptrdiff_t>(),
-            },
-            ArgKind::Size => VaArg {
-                size: self.list.arg::<ssize_t>(),
-            },
-            ArgKind::IntMax => VaArg {
-                intmax: self.list.arg::<intmax_t>(),
-            },
-            ArgKind::Double => VaArg {
-                double: self.list.arg::<c_double>(),
-            },
-            ArgKind::CharPtr => VaArg {
-                char_ptr: self.list.arg::<*const c_char>(),
-            },
-            ArgKind::VoidPtr => VaArg {
-                void_ptr: self.list.arg::<*const c_void>(),
-            },
-            ArgKind::IntPtr => VaArg {
-                int_ptr: self.list.arg::<*mut c_int>(),
-            },
-            ArgKind::ArgDefault => VaArg {
-                arg_default: self.list.arg::<usize>(),
-            },
+}
+#[derive(Default)]
+struct VaListCache {
+    args: Vec<VaArg>,
+    i: usize
+}
+impl VaListCache {
+    unsafe fn get(&mut self, i: usize, ap: &mut VaList, arg: Option<&PrintfArg>) -> VaArg {
+        if let Some(&arg) = self.args.get(i) {
+            return arg;
         }
-    }
-
-    unsafe fn get(&mut self, ty: ArgKind, i: Option<usize>) -> VaArg {
-        match i {
-            None => self.next(ty),
-            Some(i) => self.index(ty, i),
+        while self.args.len() < i {
+            // We can't POSSIBLY know the type if we reach this
+            // point. Reaching here means there are unused gaps in the
+            // arguments. Ultimately we'll have to settle down with
+            // defaulting to c_int.
+            self.args.push(VaArg::c_int(ap.arg::<c_int>()))
         }
+        self.args.push(match arg {
+            Some(arg) => VaArg::arg_from(arg, ap),
+            None => VaArg::c_int(ap.arg::<c_int>())
+        });
+        self.args[i]
     }
+}
 
-    unsafe fn next(&mut self, ty: ArgKind) -> VaArg {
-        if self.i >= self.buf.len() {
-            let arg = self.get_arg(ty);
-            self.buf.push(arg);
-        }
-        let arg = self.buf[self.i];
-        self.i += 1;
-        arg
-    }
+//  ___                 _                           _        _   _
+// |_ _|_ __ ___  _ __ | | ___ _ __ ___   ___ _ __ | |_ __ _| |_(_) ___  _ __  _
+//  | || '_ ` _ \| '_ \| |/ _ \ '_ ` _ \ / _ \ '_ \| __/ _` | __| |/ _ \| '_ \(_)
+//  | || | | | | | |_) | |  __/ | | | | |  __/ | | | || (_| | |_| | (_) | | | |_
+// |___|_| |_| |_| .__/|_|\___|_| |_| |_|\___|_| |_|\__\__,_|\__|_|\___/|_| |_(_)
+//               |_|
 
-    unsafe fn index(&mut self, ty: ArgKind, i: usize) -> VaArg {
-        if self.i >= self.buf.len() {
-            while self.buf.len() < (i - 1) {
-                // Getting a usize here most definitely isn't sane, however,
-                // there's no way to know the type!
-                // Just take this for example:
-                //
-                // printf("%*4$d\n", "hi", 0, "hello", 10);
-                //
-                // This chooses the width 10. How does it know the type of 0 and "hello"?
-                // It clearly can't.
-
-                let arg = self.get_arg(ArgKind::ArgDefault);
-                self.buf.push(arg);
-            }
-            let arg = self.get_arg(ty);
-            self.buf.push(arg);
-        }
-        self.buf[i - 1]
-    }
-}
 
 unsafe fn pop_int_raw(format: &mut *const u8) -> Option<usize> {
     let mut int = None;
@@ -342,6 +321,7 @@ fn fmt_float_normal<W: Write>(
 struct PrintfIter {
     format: *const u8
 }
+#[derive(Clone, Copy)]
 struct PrintfArg {
     index: Option<usize>,
     alternate: bool,
@@ -486,14 +466,40 @@ impl Iterator for PrintfIter {
     }
 }
 
-unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io::Result<c_int> {
+
+unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, mut ap: VaList) -> io::Result<c_int> {
     let w = &mut platform::CountingWriter::new(w);
-    let mut ap = BufferedVaList::new(ap);
 
     let iterator = PrintfIter {
         format: format as *const u8
     };
 
+    // Pre-fetch vararg types
+    let mut varargs = VaListCache::default();
+    let mut positional = BTreeMap::new();
+    // ^ NOTE: This depends on the sorted order, do not change to HashMap or whatever
+
+    for section in iterator {
+        let arg = match section {
+            Ok(PrintfFmt::Plain(text)) => continue,
+            Ok(PrintfFmt::Arg(arg)) => arg,
+            Err(()) => return Ok(-1)
+        };
+        if arg.fmtkind == FmtKind::Percent {
+            continue;
+        }
+        if let Some(i) = arg.index {
+            positional.insert(i-1, arg);
+        } else {
+            varargs.args.push(VaArg::arg_from(&arg, &mut ap));
+        }
+    }
+    // Make sure, in order, the positional arguments exist with the specified type
+    for (i, arg) in positional {
+        varargs.get(i, &mut ap, Some(&arg));
+    }
+
+    // Main loop
     for section in iterator {
         let arg = match section {
             Ok(PrintfFmt::Plain(text)) => {
@@ -503,40 +509,43 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
             Ok(PrintfFmt::Arg(arg)) => arg,
             Err(()) => return Ok(-1)
         };
-        let index = arg.index;
         let alternate = arg.alternate;
         let zero = arg.zero;
         let left = arg.left;
         let sign_reserve = arg.sign_reserve;
         let sign_always = arg.sign_always;
-        let min_width = arg.min_width.resolve(&mut ap);
-        let precision = arg.precision.map(|n| n.resolve(&mut ap));
-        let pad_space = arg.pad_space.resolve(&mut ap);
-        let pad_zero = arg.pad_zero.resolve(&mut ap);
+        let min_width = arg.min_width.resolve(&mut varargs, &mut ap);
+        let precision = arg.precision.map(|n| n.resolve(&mut varargs, &mut ap));
+        let pad_space = arg.pad_space.resolve(&mut varargs, &mut ap);
+        let pad_zero = arg.pad_zero.resolve(&mut varargs, &mut ap);
         let intkind = arg.intkind;
         let fmt = arg.fmt;
         let fmtkind = arg.fmtkind;
 
-        // Finally, type:
+        let index = arg.index
+            .map(|i| i-1)
+            .unwrap_or_else(|| if fmtkind == FmtKind::Percent {
+                0
+            } else {
+                let i = varargs.i;
+                varargs.i += 1;
+                i
+            });
+
         match fmtkind {
             FmtKind::Percent => w.write_all(&[b'%'])?,
             FmtKind::Signed => {
-                let string = match intkind {
-                    // Per the C standard using va_arg with a type with a size
-                    // less than that of an int for integers and double for floats
-                    // is invalid. As a result any arguments smaller than an int or
-                    // double passed to a function will be promoted to the smallest
-                    // possible size. The va_list::arg function will handle this
-                    // automagically.
-                    IntKind::Byte => ap.get(ArgKind::Byte, index).byte.to_string(),
-                    IntKind::Short => ap.get(ArgKind::Short, index).short.to_string(),
-                    // Types that will not be promoted
-                    IntKind::Int => ap.get(ArgKind::Int, index).int.to_string(),
-                    IntKind::Long => ap.get(ArgKind::Long, index).long.to_string(),
-                    IntKind::LongLong => ap.get(ArgKind::LongLong, index).longlong.to_string(),
-                    IntKind::PtrDiff => ap.get(ArgKind::PtrDiff, index).ptrdiff.to_string(),
-                    IntKind::Size => ap.get(ArgKind::Size, index).size.to_string(),
-                    IntKind::IntMax => ap.get(ArgKind::IntMax, index).intmax.to_string(),
+                let string = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_char(i) => i.to_string(),
+                    VaArg::c_double(i) => panic!("this should not be possible"),
+                    VaArg::c_int(i) => i.to_string(),
+                    VaArg::c_long(i) => i.to_string(),
+                    VaArg::c_longlong(i) => i.to_string(),
+                    VaArg::c_short(i) => i.to_string(),
+                    VaArg::intmax_t(i) => i.to_string(),
+                    VaArg::pointer(i) => (i as usize).to_string(),
+                    VaArg::ptrdiff_t(i) => i.to_string(),
+                    VaArg::ssize_t(i) => i.to_string()
                 };
                 let positive = !string.starts_with('-');
                 let zero = precision == Some(0) && string == "0";
@@ -573,18 +582,17 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
                 pad(w, left, b' ', final_len..pad_space)?;
             },
             FmtKind::Unsigned => {
-                let string = match intkind {
-                    // va_list will promote the following two to a c_int
-                    IntKind::Byte => fmt_int(fmt, ap.get(ArgKind::Byte, index).byte),
-                    IntKind::Short => fmt_int(fmt, ap.get(ArgKind::Short, index).short),
-                    IntKind::Int => fmt_int(fmt, ap.get(ArgKind::Int, index).int),
-                    IntKind::Long => fmt_int(fmt, ap.get(ArgKind::Long, index).long),
-                    IntKind::LongLong => {
-                        fmt_int(fmt, ap.get(ArgKind::LongLong, index).longlong)
-                    }
-                    IntKind::PtrDiff => fmt_int(fmt, ap.get(ArgKind::PtrDiff, index).ptrdiff),
-                    IntKind::Size => fmt_int(fmt, ap.get(ArgKind::Size, index).size),
-                    IntKind::IntMax => fmt_int(fmt, ap.get(ArgKind::IntMax, index).intmax),
+                let string = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_char(i) => fmt_int(fmt, i as c_uchar),
+                    VaArg::c_double(i) => panic!("this should not be possible"),
+                    VaArg::c_int(i) => fmt_int(fmt, i as c_uint),
+                    VaArg::c_long(i) => fmt_int(fmt, i as c_ulong),
+                    VaArg::c_longlong(i) => fmt_int(fmt, i as c_ulonglong),
+                    VaArg::c_short(i) => fmt_int(fmt, i as c_ushort),
+                    VaArg::intmax_t(i) => fmt_int(fmt, i as uintmax_t),
+                    VaArg::pointer(i) => fmt_int(fmt, i as usize),
+                    VaArg::ptrdiff_t(i) => fmt_int(fmt, i as size_t),
+                    VaArg::ssize_t(i) => fmt_int(fmt, i as size_t)
                 };
                 let zero = precision == Some(0) && string == "0";
 
@@ -628,20 +636,29 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
                 pad(w, left, b' ', final_len..pad_space)?;
             },
             FmtKind::Scientific => {
-                let mut float = ap.get(ArgKind::Double, index).double;
+                let mut float = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_double(i) => i,
+                    _ => panic!("this should not be possible")
+                };
                 let precision = precision.unwrap_or(6);
 
                 fmt_float_exp(w, fmt, None, false, precision, float, left, pad_space, pad_zero)?;
             },
             FmtKind::Decimal => {
-                let mut float = ap.get(ArgKind::Double, index).double;
+                let mut float = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_double(i) => i,
+                    _ => panic!("this should not be possible")
+                };
                 let precision = precision.unwrap_or(6);
 
                 fmt_float_normal(w, false, precision, float, left, pad_space, pad_zero)?;
             },
             FmtKind::AnyNotation => {
+                let mut float = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_double(i) => i,
+                    _ => panic!("this should not be possible")
+                };
                 let exp_fmt = b'E' | (fmt & 32);
-                let mut float = ap.get(ArgKind::Double, index).double;
                 let precision = precision.unwrap_or(6);
 
                 if !fmt_float_exp(
@@ -661,7 +678,10 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
             FmtKind::String => {
                 // if intkind == IntKind::Long || intkind == IntKind::LongLong, handle *const wchar_t
 
-                let ptr = ap.get(ArgKind::CharPtr, index).char_ptr;
+                let mut ptr = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::pointer(p) => p,
+                    _ => panic!("this should not be possible")
+                } as *const c_char;
 
                 if ptr.is_null() {
                     w.write_all(b"(null)")?;
@@ -680,14 +700,20 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
             FmtKind::Char => {
                 // if intkind == IntKind::Long || intkind == IntKind::LongLong, handle wint_t
 
-                let c = ap.get(ArgKind::Byte, index).byte;
+                let c = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::c_char(c) => c,
+                    _ => panic!("this should not be possible")
+                };
 
                 pad(w, !left, b' ', 1..pad_space)?;
                 w.write_all(&[c as u8])?;
                 pad(w, left, b' ', 1..pad_space)?;
             },
             FmtKind::Pointer => {
-                let ptr = ap.get(ArgKind::VoidPtr, index).int_ptr;
+                let mut ptr = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::pointer(p) => p,
+                    _ => panic!("this should not be possible")
+                };
 
                 let mut len = 1;
                 if ptr.is_null() {
@@ -709,14 +735,27 @@ unsafe fn inner_printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> io
                 pad(w, left, b' ', len..pad_space)?;
             },
             FmtKind::GetWritten => {
-                let ptr = ap.get(ArgKind::IntPtr, index).int_ptr;
-                *ptr = w.written as c_int;
+                let mut ptr = match varargs.get(index, &mut ap, Some(&arg)) {
+                    VaArg::pointer(p) => p,
+                    _ => panic!("this should not be possible")
+                };
+
+                match intkind {
+                    IntKind::Byte => *(ptr as *mut c_char) = w.written as c_char,
+                    IntKind::Short => *(ptr as *mut c_short) = w.written as c_short,
+                    IntKind::Int => *(ptr as *mut c_int) = w.written as c_int,
+                    IntKind::Long => *(ptr as *mut c_long) = w.written as c_long,
+                    IntKind::LongLong => *(ptr as *mut c_longlong) = w.written as c_longlong,
+                    IntKind::IntMax => *(ptr as *mut intmax_t) = w.written as intmax_t,
+                    IntKind::PtrDiff => *(ptr as *mut ptrdiff_t) = w.written as ptrdiff_t,
+                    IntKind::Size => *(ptr as *mut size_t) = w.written as size_t
+                }
             }
         }
     }
     Ok(w.written as c_int)
 }
 
-pub unsafe fn printf<W: Write>(w: W, format: *const c_char, ap: va_list) -> c_int {
+pub unsafe fn printf<W: Write>(w: W, format: *const c_char, ap: VaList) -> c_int {
     inner_printf(w, format, ap).unwrap_or(-1)
 }

tests/stdio/printf.c

@@ -1,7 +1,5 @@
 #include <stdio.h>
 
-#include "test_helpers.h"
-
 int main(void) {
     int sofar = 0;
     int len = printf(