From dd2574bc1097a912e799340172b8b6ef42ac5ceb Mon Sep 17 00:00:00 2001 From: Robert Morris 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 39a99d7a8f..62fd0f8487 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 a3c962b37d..d1b6bb9049 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 1a50203d9a..d20534c150 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 147bc7635a..f21fa264e9 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 b32c200ee8..39ab074779 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 5c5c78295b..81787467fa 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 4d398d5069..04013cadb0 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 7f35c383f1..55debe7dcc 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");