GCC 6.1.1 doesn't build sweb
dkales opened this issue · 5 comments
The newest version of gcc doesn't build sweb anymore.
In file included from /**********/userspace/libc/include/unistd.h:23:0,
from /**********/userspace/libc/src/printf.c:20:
/**********/userspace/libc/src/printf.c: In function ‘printf’:
/**********/userspace/libc/src/printf.c:420:23: error: second argument to va_arg’ is of incomplete type ‘void’
va_arg(args,void);
^
/**********/userspace/libc/include/stdarg.h:60:40: note: in definition of macro ‘va_arg’
#define va_arg(v,l) __builtin_va_arg(v,l)
^
userspace/libc/CMakeFiles/userspace_libc.dir/build.make:422: recipe for target 'userspace/libc/CMakeFiles/userspace_libc.dir/src/printf.c.o' failed
The problematic piece of code is the default case of the format string handling in printf.c
default:
//jump over unknown arg
//++args;
va_arg(args,void);
break;
We probably need another way to handle the default case.
works when changing void to size_t, but I haven't tested if this has any side effects:
va_arg(args,size_t);
yeah that was my local fix as well, but i wanted to start a discussion before submitting a pull request.
I also think size_t should be fine, but maybe we should have a verbose error instead?
The reduced functionality of the userspace printf has caused some confusion for some students in the past.
@dgruss opinions?
The exact handling is probably not really that important, but supporting GCC 6.1 out of the box would be good to have sooner than later.
I checked with the libc on Linux, there is no error when using a wrong format specifier. It is just skipped, and '%' is displayed. As we already get a compiler warning unknown conversion type character 0x20 in format [-Wformat=], I think we should adopt the libc behavior and silently ignore the format specifier.
My (new) suggested patch is
- va_arg(args,void);
+ *output_string.ptr++ = '%';
+ ++output_string.length;
+1 consistency with the libc implementation seems like the best idea
There is also the issue that '%f' is not supported by the reduced printf implementation, causing the aforementioned confusion. (Since there is obviously no compiler warning)
The printed '%' should probably be enough information though.