From dd2574bc1097a912e799340172b8b6ef42ac5ceb Mon Sep 17 00:00:00 2001
From: Robert Morris <rtm@csail.mit.edu>
Date: Sun, 31 Dec 2023 12:21:46 -0500
Subject: [PATCH] tell gcc to check printf format strings add %ld %lld %u %lu
%llu %lx %llx to kernel and user printf
---
Makefile | 9 +++++-
kernel/defs.h | 2 +-
kernel/printf.c | 75 +++++++++++++++++++++++++++++++++++++-----------
kernel/trap.c | 8 +++---
user/ls.c | 4 +--
user/printf.c | 52 ++++++++++++++++++++++++++++++---
user/user.h | 4 +--
user/usertests.c | 52 ++++++++++++++++-----------------
8 files changed, 150 insertions(+), 56 deletions(-)
diff --git a/Makefile b/Makefile
index 39a99d7..62fd0f8 100644
--- a/Makefile
+++ b/Makefile
@@ -59,7 +59,14 @@ OBJDUMP = $(TOOLPREFIX)objdump
CFLAGS = -Wall -Werror -O -fno-omit-frame-pointer -ggdb -gdwarf-2
CFLAGS += -MD
CFLAGS += -mcmodel=medany
-CFLAGS += -ffreestanding -fno-common -nostdlib -mno-relax
+# CFLAGS += -ffreestanding -fno-common -nostdlib -mno-relax
+CFLAGS += -fno-common -nostdlib -mno-relax
+CFLAGS += -fno-builtin-strncpy -fno-builtin-strncmp -fno-builtin-strlen -fno-builtin-memset
+CFLAGS += -fno-builtin-memmove -fno-builtin-memcmp -fno-builtin-log -fno-builtin-bzero
+CFLAGS += -fno-builtin-strchr -fno-builtin-exit -fno-builtin-malloc -fno-builtin-putc
+CFLAGS += -fno-builtin-free
+CFLAGS += -fno-builtin-memcpy -Wno-main
+CFLAGS += -fno-builtin-printf -fno-builtin-fprintf -fno-builtin-vprintf
CFLAGS += -I.
CFLAGS += $(shell $(CC) -fno-stack-protector -E -x c /dev/null >/dev/null 2>&1 && echo -fno-stack-protector)
diff --git a/kernel/defs.h b/kernel/defs.h
index a3c962b..d1b6bb9 100644
--- a/kernel/defs.h
+++ b/kernel/defs.h
@@ -77,7 +77,7 @@ int piperead(struct pipe*, uint64, int);
int pipewrite(struct pipe*, uint64, int);
// printf.c
-void printf(char*, ...);
+int printf(char*, ...) __attribute__ ((format (printf, 1, 2)));
void panic(char*) __attribute__((noreturn));
void printfinit(void);
diff --git a/kernel/printf.c b/kernel/printf.c
index 1a50203..d20534c 100644
--- a/kernel/printf.c
+++ b/kernel/printf.c
@@ -26,13 +26,13 @@ static struct {
static char digits[] = "0123456789abcdef";
static void
-printint(int xx, int base, int sign)
+printint(long long xx, int base, int sign)
{
char buf[16];
int i;
- uint x;
+ unsigned long long x;
- if(sign && (sign = xx < 0))
+ if(sign && (sign = (xx < 0)))
x = -xx;
else
x = xx;
@@ -59,30 +59,71 @@ printptr(uint64 x)
consputc(digits[x >> (sizeof(uint64) * 8 - 4)]);
}
-// Print to the console. only understands %d, %x, %p, %s.
-void
+// Print to the console.
+int
printf(char *fmt, ...)
{
va_list ap;
- int i, c, locking;
+ int i, cx, c0, c1, c2, locking;
char *s;
locking = pr.locking;
if(locking)
acquire(&pr.lock);
- if (fmt == 0)
- panic("null fmt");
-
va_start(ap, fmt);
- for(i = 0; (c = fmt[i] & 0xff) != 0; i++){
- if(c != '%'){
- consputc(c);
+ for(i = 0; (cx = fmt[i] & 0xff) != 0; i++){
+ if(cx != '%'){
+ consputc(cx);
continue;
}
- c = fmt[++i] & 0xff;
- if(c == 0)
+ i++;
+ c0 = fmt[i+0] & 0xff;
+ c1 = c2 = 0;
+ if(c0) c1 = fmt[i+1] & 0xff;
+ if(c1) c2 = fmt[i+2] & 0xff;
+ if(c0 == 'd'){
+ printint(va_arg(ap, int), 10, 1);
+ } else if(c0 == 'l' && c1 == 'd'){
+ printint(va_arg(ap, uint64), 10, 1);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'd'){
+ printint(va_arg(ap, uint64), 10, 1);
+ i += 2;
+ } else if(c0 == 'u'){
+ printint(va_arg(ap, int), 10, 0);
+ } else if(c0 == 'l' && c1 == 'u'){
+ printint(va_arg(ap, uint64), 10, 0);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'u'){
+ printint(va_arg(ap, uint64), 10, 0);
+ i += 2;
+ } else if(c0 == 'x'){
+ printint(va_arg(ap, int), 16, 0);
+ } else if(c0 == 'l' && c1 == 'x'){
+ printint(va_arg(ap, uint64), 16, 0);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'x'){
+ printint(va_arg(ap, uint64), 16, 0);
+ i += 2;
+ } else if(c0 == 'p'){
+ printptr(va_arg(ap, uint64));
+ } else if(c0 == 's'){
+ if((s = va_arg(ap, char*)) == 0)
+ s = "(null)";
+ for(; *s; s++)
+ consputc(*s);
+ } else if(c0 == '%'){
+ consputc('%');
+ } else if(c0 == 0){
break;
+ } else {
+ // Print unknown % sequence to draw attention.
+ consputc('%');
+ consputc(c0);
+ }
+
+#if 0
switch(c){
case 'd':
printint(va_arg(ap, int), 10, 1);
@@ -108,11 +149,14 @@ printf(char *fmt, ...)
consputc(c);
break;
}
+#endif
}
va_end(ap);
if(locking)
release(&pr.lock);
+
+ return 0;
}
void
@@ -120,8 +164,7 @@ panic(char *s)
{
pr.locking = 0;
printf("panic: ");
- printf(s);
- printf("\n");
+ printf("%s\n", s);
panicked = 1; // freeze uart output from other CPUs
for(;;)
;
diff --git a/kernel/trap.c b/kernel/trap.c
index 147bc76..f21fa26 100644
--- a/kernel/trap.c
+++ b/kernel/trap.c
@@ -68,8 +68,8 @@ usertrap(void)
} else if((which_dev = devintr()) != 0){
// ok
} else {
- printf("usertrap(): unexpected scause %p pid=%d\n", r_scause(), p->pid);
- printf(" sepc=%p stval=%p\n", r_sepc(), r_stval());
+ printf("usertrap(): unexpected scause %lx pid=%d\n", r_scause(), p->pid);
+ printf(" sepc=%lx stval=%lx\n", r_sepc(), r_stval());
setkilled(p);
}
@@ -145,8 +145,8 @@ kerneltrap()
panic("kerneltrap: interrupts enabled");
if((which_dev = devintr()) == 0){
- printf("scause %p\n", scause);
- printf("sepc=%p stval=%p\n", r_sepc(), r_stval());
+ printf("scause %lx\n", scause);
+ printf("sepc=%lx stval=%lx\n", r_sepc(), r_stval());
panic("kerneltrap");
}
diff --git a/user/ls.c b/user/ls.c
index b32c200..39ab074 100644
--- a/user/ls.c
+++ b/user/ls.c
@@ -45,7 +45,7 @@ ls(char *path)
switch(st.type){
case T_DEVICE:
case T_FILE:
- printf("%s %d %d %l\n", fmtname(path), st.type, st.ino, st.size);
+ printf("%s %d %d %d\n", fmtname(path), st.type, st.ino, (int) st.size);
break;
case T_DIR:
@@ -65,7 +65,7 @@ ls(char *path)
printf("ls: cannot stat %s\n", buf);
continue;
}
- printf("%s %d %d %d\n", fmtname(buf), st.type, st.ino, st.size);
+ printf("%s %d %d %d\n", fmtname(buf), st.type, st.ino, (int) st.size);
}
break;
}
diff --git a/user/printf.c b/user/printf.c
index 5c5c782..8178746 100644
--- a/user/printf.c
+++ b/user/printf.c
@@ -52,18 +52,61 @@ void
vprintf(int fd, const char *fmt, va_list ap)
{
char *s;
- int c, i, state;
+ int c0, c1, c2, i, state;
state = 0;
for(i = 0; fmt[i]; i++){
- c = fmt[i] & 0xff;
+ c0 = fmt[i] & 0xff;
if(state == 0){
- if(c == '%'){
+ if(c0 == '%'){
state = '%';
} else {
- putc(fd, c);
+ putc(fd, c0);
}
} else if(state == '%'){
+ c1 = c2 = 0;
+ if(c0) c1 = fmt[i+1] & 0xff;
+ if(c1) c2 = fmt[i+2] & 0xff;
+ if(c0 == 'd'){
+ printint(fd, va_arg(ap, int), 10, 1);
+ } else if(c0 == 'l' && c1 == 'd'){
+ printint(fd, va_arg(ap, uint64), 10, 1);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'd'){
+ printint(fd, va_arg(ap, uint64), 10, 1);
+ i += 2;
+ } else if(c0 == 'u'){
+ printint(fd, va_arg(ap, int), 10, 0);
+ } else if(c0 == 'l' && c1 == 'u'){
+ printint(fd, va_arg(ap, uint64), 10, 0);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'u'){
+ printint(fd, va_arg(ap, uint64), 10, 0);
+ i += 2;
+ } else if(c0 == 'x'){
+ printint(fd, va_arg(ap, int), 16, 0);
+ } else if(c0 == 'l' && c1 == 'x'){
+ printint(fd, va_arg(ap, uint64), 16, 0);
+ i += 1;
+ } else if(c0 == 'l' && c1 == 'l' && c2 == 'x'){
+ printint(fd, va_arg(ap, uint64), 16, 0);
+ i += 2;
+ } else if(c0 == 'p'){
+ printptr(fd, va_arg(ap, uint64));
+ } else if(c0 == 's'){
+ if((s = va_arg(ap, char*)) == 0)
+ s = "(null)";
+ for(; *s; s++)
+ putc(fd, *s);
+ } else if(c0 == '%'){
+ putc(fd, '%');
+ } else {
+ // Unknown % sequence. Print it to draw attention.
+ putc(fd, '%');
+ putc(fd, c0);
+ }
+
+#if 0
if(c == 'd'){
printint(fd, va_arg(ap, int), 10, 1);
} else if(c == 'l') {
@@ -89,6 +132,7 @@ vprintf(int fd, const char *fmt, va_list ap)
putc(fd, '%');
putc(fd, c);
}
+#endif
state = 0;
}
}
diff --git a/user/user.h b/user/user.h
index 4d398d5..04013ca 100644
--- a/user/user.h
+++ b/user/user.h
@@ -29,8 +29,8 @@ char* strcpy(char*, const char*);
void *memmove(void*, const void*, int);
char* strchr(const char*, char c);
int strcmp(const char*, const char*);
-void fprintf(int, const char*, ...);
-void printf(const char*, ...);
+void fprintf(int, const char*, ...) __attribute__ ((format (printf, 2, 3)));
+void printf(const char*, ...) __attribute__ ((format (printf, 1, 2)));
char* gets(char*, int max);
uint strlen(const char*);
void* memset(void*, int, uint);
diff --git a/user/usertests.c b/user/usertests.c
index 7f35c38..55debe7 100644
--- a/user/usertests.c
+++ b/user/usertests.c
@@ -44,7 +44,7 @@ copyin(char *s)
}
int n = write(fd, (void*)addr, 8192);
if(n >= 0){
- printf("write(fd, %p, 8192) returned %d, not -1\n", addr, n);
+ printf("write(fd, %p, 8192) returned %d, not -1\n", (void*)addr, n);
exit(1);
}
close(fd);
@@ -52,7 +52,7 @@ copyin(char *s)
n = write(1, (char*)addr, 8192);
if(n > 0){
- printf("write(1, %p, 8192) returned %d, not -1 or 0\n", addr, n);
+ printf("write(1, %p, 8192) returned %d, not -1 or 0\n", (void*)addr, n);
exit(1);
}
@@ -63,7 +63,7 @@ copyin(char *s)
}
n = write(fds[1], (char*)addr, 8192);
if(n > 0){
- printf("write(pipe, %p, 8192) returned %d, not -1 or 0\n", addr, n);
+ printf("write(pipe, %p, 8192) returned %d, not -1 or 0\n", (void*)addr, n);
exit(1);
}
close(fds[0]);
@@ -88,7 +88,7 @@ copyout(char *s)
}
int n = read(fd, (void*)addr, 8192);
if(n > 0){
- printf("read(fd, %p, 8192) returned %d, not -1 or 0\n", addr, n);
+ printf("read(fd, %p, 8192) returned %d, not -1 or 0\n", (void*)addr, n);
exit(1);
}
close(fd);
@@ -105,7 +105,7 @@ copyout(char *s)
}
n = read(fds[0], (void*)addr, 8192);
if(n > 0){
- printf("read(pipe, %p, 8192) returned %d, not -1 or 0\n", addr, n);
+ printf("read(pipe, %p, 8192) returned %d, not -1 or 0\n", (void*)addr, n);
exit(1);
}
close(fds[0]);
@@ -124,7 +124,7 @@ copyinstr1(char *s)
int fd = open((char *)addr, O_CREATE|O_WRONLY);
if(fd >= 0){
- printf("open(%p) returned %d, not -1\n", addr, fd);
+ printf("open(%p) returned %d, not -1\n", (void*)addr, fd);
exit(1);
}
}
@@ -264,7 +264,7 @@ rwsbrk()
}
n = write(fd, (void*)(a+4096), 1024);
if(n >= 0){
- printf("write(fd, %p, 1024) returned %d, not -1\n", a+4096, n);
+ printf("write(fd, %p, 1024) returned %d, not -1\n", (void*)a+4096, n);
exit(1);
}
close(fd);
@@ -277,7 +277,7 @@ rwsbrk()
}
n = read(fd, (void*)(a+4096), 10);
if(n >= 0){
- printf("read(fd, %p, 10) returned %d, not -1\n", a+4096, n);
+ printf("read(fd, %p, 10) returned %d, not -1\n", (void*)a+4096, n);
exit(1);
}
close(fd);
@@ -589,7 +589,7 @@ writebig(char *s)
for(i = 0; i < MAXFILE; i++){
((int*)buf)[0] = i;
if(write(fd, buf, BSIZE) != BSIZE){
- printf("%s: error: write big file failed\n", s, i);
+ printf("%s: error: write big file failed i=%d\n", s, i);
exit(1);
}
}
@@ -773,7 +773,7 @@ pipe1(char *s)
cc = sizeof(buf);
}
if(total != N * SZ){
- printf("%s: pipe1 oops 3 total %d\n", total);
+ printf("%s: pipe1 oops 3 total %d\n", s, total);
exit(1);
}
close(fds[0]);
@@ -1069,7 +1069,7 @@ mem(char *s)
}
m1 = malloc(1024*20);
if(m1 == 0){
- printf("couldn't allocate mem?!!\n", s);
+ printf("%s: couldn't allocate mem?!!\n", s);
exit(1);
}
free(m1);
@@ -1161,14 +1161,14 @@ fourfiles(char *s)
pid = fork();
if(pid < 0){
- printf("fork failed\n", s);
+ printf("%s: fork failed\n", s);
exit(1);
}
if(pid == 0){
fd = open(fname, O_CREATE | O_RDWR);
if(fd < 0){
- printf("create failed\n", s);
+ printf("%s: create failed\n", s);
exit(1);
}
@@ -1197,7 +1197,7 @@ fourfiles(char *s)
while((n = read(fd, buf, sizeof(buf))) > 0){
for(j = 0; j < n; j++){
if(buf[j] != '0'+i){
- printf("wrong char\n", s);
+ printf("%s: wrong char\n", s);
exit(1);
}
}
@@ -1223,7 +1223,7 @@ createdelete(char *s)
for(pi = 0; pi < NCHILD; pi++){
pid = fork();
if(pid < 0){
- printf("fork failed\n", s);
+ printf("%s: fork failed\n", s);
exit(1);
}
@@ -1544,7 +1544,7 @@ subdir(char *s)
}
if(mkdir("/dd/dd") != 0){
- printf("subdir mkdir dd/dd failed\n", s);
+ printf("%s: subdir mkdir dd/dd failed\n", s);
exit(1);
}
@@ -1569,7 +1569,7 @@ subdir(char *s)
close(fd);
if(link("dd/dd/ff", "dd/dd/ffff") != 0){
- printf("link dd/dd/ff dd/dd/ffff failed\n", s);
+ printf("%s: link dd/dd/ff dd/dd/ffff failed\n", s);
exit(1);
}
@@ -1591,7 +1591,7 @@ subdir(char *s)
exit(1);
}
if(chdir("dd/../../../dd") != 0){
- printf("chdir dd/../../dd failed\n", s);
+ printf("%s: chdir dd/../../dd failed\n", s);
exit(1);
}
if(chdir("./..") != 0){
@@ -2034,7 +2034,7 @@ sbrkbasic(char *s)
for(i = 0; i < 5000; i++){
b = sbrk(1);
if(b != a){
- printf("%s: sbrk test failed %d %x %x\n", s, i, a, b);
+ printf("%s: sbrk test failed %d %p %p\n", s, i, a, b);
exit(1);
}
*b = 1;
@@ -2092,7 +2092,7 @@ sbrkmuch(char *s)
}
c = sbrk(0);
if(c != a - PGSIZE){
- printf("%s: sbrk deallocation produced wrong address, a %x c %x\n", s, a, c);
+ printf("%s: sbrk deallocation produced wrong address, a %p c %p\n", s, a, c);
exit(1);
}
@@ -2100,7 +2100,7 @@ sbrkmuch(char *s)
a = sbrk(0);
c = sbrk(PGSIZE);
if(c != a || sbrk(0) != a + PGSIZE){
- printf("%s: sbrk re-allocation failed, a %x c %x\n", s, a, c);
+ printf("%s: sbrk re-allocation failed, a %p c %p\n", s, a, c);
exit(1);
}
if(*lastaddr == 99){
@@ -2112,7 +2112,7 @@ sbrkmuch(char *s)
a = sbrk(0);
c = sbrk(-(sbrk(0) - oldbrk));
if(c != a){
- printf("%s: sbrk downsize failed, a %x c %x\n", s, a, c);
+ printf("%s: sbrk downsize failed, a %p c %p\n", s, a, c);
exit(1);
}
}
@@ -2131,7 +2131,7 @@ kernmem(char *s)
exit(1);
}
if(pid == 0){
- printf("%s: oops could read %x = %x\n", s, a, *a);
+ printf("%s: oops could read %p = %x\n", s, a, *a);
exit(1);
}
int xstatus;
@@ -2155,7 +2155,7 @@ MAXVAplus(char *s)
}
if(pid == 0){
*(char*)a = 99;
- printf("%s: oops wrote %x\n", s, a);
+ printf("%s: oops wrote %p\n", s, (void*)a);
exit(1);
}
int xstatus;
@@ -2408,7 +2408,7 @@ stacktest(char *s)
char *sp = (char *) r_sp();
sp -= PGSIZE;
// the *sp should cause a trap.
- printf("%s: stacktest: read below stack %p\n", s, *sp);
+ printf("%s: stacktest: read below stack %d\n", s, *sp);
exit(1);
} else if(pid < 0){
printf("%s: fork failed\n", s);
@@ -2868,7 +2868,7 @@ diskfull(char *s)
// this mkdir() is expected to fail.
if(mkdir("diskfulldir") == 0)
- printf("%s: mkdir(diskfulldir) unexpectedly succeeded!\n");
+ printf("%s: mkdir(diskfulldir) unexpectedly succeeded!\n", s);
unlink("diskfulldir");