From 18ea283213a3eb207ef38e4ee0ed8dd9bc17231a Mon Sep 17 00:00:00 2001 From: Mathieu Maret Date: Thu, 9 Nov 2023 20:30:09 +0100 Subject: [PATCH 1/2] Use format attribut and correct associated errors --- arch/x86/exception.c | 10 ++++------ core/alloc.c | 4 ++-- core/allocArea.c | 4 ++-- core/elf.c | 2 +- core/klibc.c | 24 +++++++++++++++++++++++- core/klibc.h | 6 +++--- core/main.c | 6 +++--- core/mem.c | 2 +- core/process.c | 4 ++-- core/stack.c | 2 +- core/synchro.c | 2 +- core/thread.c | 7 ++++--- core/uaddrspace.c | 2 +- tests/test.c | 12 ++++++------ userspace/libc.c | 4 ++-- userspace/libc.h | 10 +++++----- 16 files changed, 61 insertions(+), 40 deletions(-) diff --git a/arch/x86/exception.c b/arch/x86/exception.c index 960634b..68e5cee 100644 --- a/arch/x86/exception.c +++ b/arch/x86/exception.c @@ -46,9 +46,8 @@ int exceptionSetRoutine(int exception, exception_handler handler) void print_handler(struct cpu_state *frame, ulong intr) { int intNbInt = intr; - VGAPrintf(RED, BLACK, 0, VGA_HEIGHT - 1, "EXCEPTION %d %d", intNbInt, intr); - printf("Exception %d (Err %d) at 0x%x\n", intr, intr, - cpu_context_get_PC(frame)); + VGAPrintf(RED, BLACK, 0, VGA_HEIGHT - 1, "EXCEPTION %d", intNbInt); + printf("Exception %lu at 0x%p\n", intr, (void *)cpu_context_get_PC(frame)); asm("hlt"); } @@ -62,9 +61,8 @@ void pagefault_handler(struct cpu_state *frame, ulong intr) if(!uAddrSpaceCheckNAlloc(as, faultAddr)) return; - - printf("page fault while in thread [%s] at 0x%x when trying to access 0x%x err_code 0x%x\n", current->name, - cpu_context_get_PC(frame), faultAddr, cpu_context_get_EX_err(frame)); + printf("page fault while in thread [%s] at 0x%p when trying to access 0x%p err_code 0x%x\n", current->name, + (void *)cpu_context_get_PC(frame), (void *)faultAddr, cpu_context_get_EX_err(frame)); if (cpu_context_is_in_user_mode(frame)) { printf("Killing User Thread\n"); threadExit(); diff --git a/core/alloc.c b/core/alloc.c index 7feb929..90dae78 100644 --- a/core/alloc.c +++ b/core/alloc.c @@ -81,7 +81,7 @@ int allocBookSlab(size_t sizeEl, size_t sizeSlab, int selfContained, int neverEm int ret; int flags; - pr_devel("%s for element of size %lu is self %d\n", __func__, sizeEl, selfContained); + pr_devel("%s for element of size %u is self %d\n", __func__, sizeEl, selfContained); disable_IRQs(flags); list_foreach(slub, slab, slabIdx) @@ -229,7 +229,7 @@ static void *allocFromSlab(struct slabEntry *slab) vaddr_t *next = slab->freeEl; if (*next == (vaddr_t)NULL) { - pr_devel("Slab @%d is now full\n", slab); + pr_devel("Slab @%p is now full\n", slab); slab->full = 1; } slab->freeEl = (void *)(*next); diff --git a/core/allocArea.c b/core/allocArea.c index 6981186..9fa4eee 100644 --- a/core/allocArea.c +++ b/core/allocArea.c @@ -191,7 +191,7 @@ int areaAdd(vaddr_t start, vaddr_t end, int isFree) struct memArea **area; int nbPages = (end - start) / PAGE_SIZE; - pr_devel("Add %s area 0x%x->0x%x (%d)\n", isFree ? "free" : "used", start, end, nbPages); + pr_devel("Add %s area 0x%p->0x%p (%d)\n", isFree ? "free" : "used", (void *)start, (void *)end, nbPages); assert(nbPages > 0); assert(IS_ALIGNED(start, PAGE_SIZE)); @@ -258,7 +258,7 @@ int areaFree(vaddr_t addr) area = areaFindMemArea(usedArea, addr); if (!area) { - pr_info("Cannot find memArea associated to %p\n", addr); + pr_info("Cannot find memArea associated to %p\n", (void *)addr); return -1; } diff --git a/core/elf.c b/core/elf.c index 40e0cf2..d3d0f89 100644 --- a/core/elf.c +++ b/core/elf.c @@ -160,7 +160,7 @@ uaddr_t loadElfProg(const char *prog, struct process * proc) } if (elf_phdrs[i].p_vaddr < PAGING_BASE_USER_ADDRESS) { - printf("User program has an incorrect address 0x%x\n", elf_phdrs[i].p_vaddr); + printf("User program has an incorrect address 0x%p\n", (void *)elf_phdrs[i].p_vaddr); return (uaddr_t)NULL; } diff --git a/core/klibc.c b/core/klibc.c index 364a4f6..257095c 100644 --- a/core/klibc.c +++ b/core/klibc.c @@ -447,7 +447,29 @@ int vsnprintf(char *str, size_t size, const char *format, va_list ap) break; case 'i': case 'd': PRINT_PART(printLint, long int, str, size, c, ret) - case 'u': PRINT_PART(printLuint, long unsigned int, str, size, c, ret) + case 'u': + PRINT_PART(printLuint, long unsigned int, str, size, c, ret) + case 'p': + case 'x': { + char val[sizeof(int) * 2]; + unsigned int valIdx = 0; + long int d = va_arg(ap, long int); + itoa(d, val, 16); + if (str) { + while (val[valIdx]) { + if (size) { + str[c++] = val[valIdx++]; + size--; + ret++; + } else { + return ret; + } + } + } else { + ret += strlen(val); + } + break; + } } i++; break; diff --git a/core/klibc.h b/core/klibc.h index d54b146..f5129c6 100644 --- a/core/klibc.h +++ b/core/klibc.h @@ -31,9 +31,9 @@ int strcmp(const char s1[], const char s2[]); __attribute__ ((access (read_only, 2), access (write_only, 1, 3))) char *strzcpy(char *dst, const char *src, int len); int puts(const char *str); int putc(const char str); -int vsnprintf(char *str, size_t size, const char *format, va_list ap); -int vprintf(const char *format, va_list ap); -int printf(const char *format, ...); +int vsnprintf(char *str, size_t size, const char *format, va_list ap) __attribute__ ((__format__ (printf, 3, 0))); +int vprintf(const char *format, va_list ap) __attribute__ ((__format__ (printf, 1, 0))); +int printf(const char *format, ...) __attribute__ ((__format__ (printf, 1, 2))); // Could be used after malloc is available int asprintf(char **strp, const char *fmt, ...); diff --git a/core/main.c b/core/main.c index be781a1..f4f46b2 100644 --- a/core/main.c +++ b/core/main.c @@ -164,7 +164,7 @@ void kmain(unsigned long magic, unsigned long addr) } if (CHECK_FLAG(mbi->flags, 9)) { - printf("Loaded by %s. ", mbi->boot_loader_name); + printf("Loaded by %s. ", (char *)mbi->boot_loader_name); } /* Is the command line passed? */ @@ -215,9 +215,9 @@ void kmain(unsigned long magic, unsigned long addr) printf("Cannot get memory Mapping information, using default value\n"); memAddBank(lastUsedByMem, upperMemKB * 1024, 1); } - printf("%d pages used by kernel(0x%x->0x%x))", + printf("%ld pages used by kernel(0x%lx->0x%lx))", (lastUsedByMem - firstUsedByMem) / PAGE_SIZE, firstUsedByMem, lastUsedByMem); - printf(" (%d pages for MM)\n", (lastUsedByMem - (paddr_t)&__ld_kernel_end) / PAGE_SIZE); + printf(" (%lu pages for MM)\n", (lastUsedByMem - (paddr_t)&__ld_kernel_end) / PAGE_SIZE); #ifdef RUN_TEST testPhymem(); diff --git a/core/mem.c b/core/mem.c index 37f068e..98f44aa 100644 --- a/core/mem.c +++ b/core/mem.c @@ -23,7 +23,7 @@ int memSetup(paddr_t upperMemKB, paddr_t *firstUsed, paddr_t *lastMemUsedOut) upperMemKB = ALIGN_DOWN(upperMemKB, PAGE_SIZE / 1024); unsigned long nbPage = ((upperMemKB) / (PAGE_SIZE / 1024)); - printf("Available Mem from 0x%x to 0x%x: %dMB in %d Pages of %dB\n", &__ld_kernel_end, + printf("Available Mem from 0x%lx to 0x%lx: %lu MB in %lu Pages of %dB\n",(paddr_t) &__ld_kernel_end, upperMemKB * 1024, (upperMemKB * 1024 - (uint32_t)&__ld_kernel_end) / (1024 * 1024), nbPage, PAGE_SIZE); diff --git a/core/process.c b/core/process.c index 34eaf91..b71ecc7 100644 --- a/core/process.c +++ b/core/process.c @@ -70,9 +70,9 @@ void processListPrint() list_foreach_named(proc->thList, th, nbTh, prevInProcess, nextInProcess) { if (th == cur) { - printf(" th: 0x%x Current\n", th); + printf(" th: 0x%lx Current\n", (vaddr_t)th); } else { - printf(" th: 0x%x in 0x%x\n", th, cpu_context_get_PC(th->cpuState)); + printf(" th: 0x%lx in 0x%lx\n", (vaddr_t)th, cpu_context_get_PC(th->cpuState)); } } } diff --git a/core/stack.c b/core/stack.c index b98a80d..929323e 100644 --- a/core/stack.c +++ b/core/stack.c @@ -42,6 +42,6 @@ void printStackTrace(unsigned int maxFrames) printf("Must be compiled with -fno-omit-frame-pointer for full stack\n"); unsigned int *ebp = &maxFrames - 2; unsigned int *eip = ebp + sizeof(unsigned int); - printf("[0] 0x%x\n", eip); + printf("[0] 0x%x\n", (unsigned int)eip); #endif } diff --git a/core/synchro.c b/core/synchro.c index f9ab375..4205ff5 100644 --- a/core/synchro.c +++ b/core/synchro.c @@ -30,7 +30,7 @@ int mutexFree(struct mutex *m) if (list_is_empty(m->wait)) { #ifdef DEBUG if (m->owner) { - printf("Warning: freeing a owned mutex 0x%. owned by 0x%p 0x%p\n", m, m->owner, + printf("Warning: freeing a owned mutex 0x%p. owned by 0x%p 0x%p\n", m, m->owner, getCurrentThread()); } #endif diff --git a/core/thread.c b/core/thread.c index 5fe0955..ae0f293 100644 --- a/core/thread.c +++ b/core/thread.c @@ -5,6 +5,7 @@ #include "klibc.h" #include "list.h" #include "mmuContext.h" +#include "process.h" #include "time.h" #include "types.h" #include "vga.h" @@ -67,7 +68,7 @@ struct thread *threadCreate(const char *name, cpu_kstate_function_arg1_t func, v return NULL; } #ifdef DEBUG - printf("Alloc stack at 0x%x struct at 0x%x\n", thread->stackAddr, thread); + printf("Alloc stack at 0x%p struct at 0x%p\n", (void *)thread->stackAddr, thread); #endif thread->stackSize = THREAD_DEFAULT_STACK_SIZE; @@ -149,7 +150,7 @@ void threadDelete(struct thread *thread) processRemoveThread(thread); #ifdef DEBUG - printf("Free stack at 0x%x struct at 0x%x\n", thread->stackAddr, thread); + printf("Free stack at 0x%p struct at 0x%p\n", (void *)thread->stackAddr, thread); #endif free((void *)thread->stackAddr); free((void *)thread); @@ -298,7 +299,7 @@ int threadMsleep(unsigned long msec) disable_IRQs(flags); current = currentThread; - assertmsg(current->state == RUNNING, "thread %s is in state %d for %d\n", current->name, + assertmsg(current->state == RUNNING, "thread %s is in state %d for %lu\n", current->name, current->state, msec); current->state = SLEEPING; diff --git a/core/uaddrspace.c b/core/uaddrspace.c index e6b312a..4dd74d2 100644 --- a/core/uaddrspace.c +++ b/core/uaddrspace.c @@ -113,7 +113,7 @@ uaddr_t sysBrk(struct uAddrSpace *as, uaddr_t newHeapTop) int uAddrSpaceCheckNAlloc(struct uAddrSpace *as, vaddr_t addr) { - pr_devel("Checking %p inside %p and %p\n", addr, as->heapStart, as->heapStart +as->heapSize); + pr_devel("Checking %lx inside %lx and %lx\n", addr, as->heapStart, as->heapStart +as->heapSize); if (addr < as->heapStart || addr >= as->heapStart + as->heapSize) { return -1; } diff --git a/tests/test.c b/tests/test.c index 7628a64..3bd9417 100644 --- a/tests/test.c +++ b/tests/test.c @@ -69,8 +69,8 @@ void testPhymem(void) assert((usedPageStatAlloc - usedPageStatBegin) == (uint)allocCount); while ((allocated_page_list != NULL) && (page = list_pop_head(allocated_page_list)) != NULL) { - assertmsg(page->phy_addr == (ulong)freeCount, "page %d modified", page); - assertmsg(unrefPhyPage((ulong)page) >= 0, "Failed to free page %d\n", (ulong)page); + assertmsg(page->phy_addr == (ulong)freeCount, "page %p modified", page); + assertmsg(unrefPhyPage((ulong)page) >= 0, "Failed to free page %p\n", page); freeCount++; } printf("%d pages freed\n", freeCount); @@ -117,7 +117,7 @@ void testAlloc(void) assert((char *)malloc2 == ((char *)malloc1 + sizeof(void *))); free(malloc2); void *malloc3 = malloc(sizeof(void *)); - assertmsg((char *)malloc2 == (char *)malloc3, " %d %d\n", malloc2, malloc3); + assertmsg((char *)malloc2 == (char *)malloc3, " %p %p\n", malloc2, malloc3); free(malloc1); free(malloc3); void *alloc1 = testAllocNSet(1024); @@ -169,8 +169,8 @@ void testPaging(void) while (!list_is_empty(allocated_page_list) && (page = list_pop_head(allocated_page_list)) != NULL) { - assertmsg((char)page->phy_addr == (char)freeCount, "page modified %d but is %d\n", - freeCount, page->phy_addr); + assertmsg((char)page->phy_addr == (char)freeCount, "page modified %d but is %p\n", + freeCount, (void *)page->phy_addr); areaFree((vaddr_t)page); freeCount++; } @@ -305,7 +305,7 @@ void sleepThread(void *arg) threadMsleep(100); } unsigned long ellapsedTime = jiffies_to_msecs(jiffies - initialJiffies); - assertmsg(ellapsedTime >= 500 && ellapsedTime < 510, "ellapsedTime %d\n", ellapsedTime); + assertmsg(ellapsedTime >= 500 && ellapsedTime < 510, "ellapsedTime %lu\n", ellapsedTime); threadMsleep(0); printf("I should never be showed\n"); assert(1); diff --git a/userspace/libc.c b/userspace/libc.c index 5079314..2e660d9 100644 --- a/userspace/libc.c +++ b/userspace/libc.c @@ -257,7 +257,7 @@ int vsnprintf(char *str, size_t size, const char *format, va_list ap) int i = 0; int c = 0; - while (format[i] != '\0' && (size|| !str)) { + while (format[i] != '\0' && (size || !str)) { switch (format[i]) { case '%': switch (format[i + 1]) { @@ -443,7 +443,7 @@ int vasprintf(char **strp, const char *fmt, va_list ap) { int n = 0; size_t size = 0; - char p[256]; //TODO replace by malloc + char *p = malloc(256); /* Determine required size */ diff --git a/userspace/libc.h b/userspace/libc.h index 2b431c0..1c9b83c 100644 --- a/userspace/libc.h +++ b/userspace/libc.h @@ -24,13 +24,13 @@ int strcmp(const char s1[], const char s2[]); __attribute__ ((access (read_only, 2), access (write_only, 1, 3))) char *strzcpy(char *dst, const char *src, int len); int puts(const char *str); int putc(const int c); -int vsnprintf(char *str, size_t size, const char *format, va_list ap); -int vprintf(const char *format, va_list ap); -int printf(const char *format, ...); +int vsnprintf(char *str, size_t size, const char *format, va_list ap) __attribute__ ((__format__ (printf, 3, 0))); +int vprintf(const char *format, va_list ap) __attribute__ ((__format__ (printf, 1, 0))); +int printf(const char *format, ...) __attribute__ ((__format__ (printf, 1, 2))); // Could be used after malloc is available -int asprintf(char **strp, const char *fmt, ...); -int vasprintf(char **strp, const char *fmt, va_list ap); +int asprintf(char **strp, const char *fmt, ...) __attribute__ ((__format__ (printf, 2, 3))); +int vasprintf(char **strp, const char *fmt, va_list ap) __attribute__ ((__format__ (printf, 2, 0))); int syscall5(int id, unsigned int arg1, unsigned int arg2, unsigned int arg3, unsigned int arg4, unsigned int arg5); -- 2.47.0 From 4376201d32a2926251fa1bdf72253431dc43af5a Mon Sep 17 00:00:00 2001 From: Mathieu Maret Date: Thu, 9 Nov 2023 23:38:20 +0100 Subject: [PATCH 2/2] debug symbols are expected in .debug file --- Makefile | 10 +++++----- debug.gdb | 5 +++++ 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index 3dbb44d..48215c5 100644 --- a/Makefile +++ b/Makefile @@ -29,11 +29,11 @@ deps=$(csrc:%.c=%.d) $(gasmsrc:%.S=%.d) docsrc=$(wildcard docs/*.md) docobj=$(docsrc:%.md=%.html) -kernel kernel.sym &: $(asmobj) $(gasmobj) $(cobj) linker.ld +kernel kernel.debug &: $(asmobj) $(gasmobj) $(cobj) linker.ld $(LD) $(LDFLAGS) $(asmobj) $(gasmobj) $(cobj) -o kernel -T linker.ld $(LIBGCC) -Map kernel.map - objcopy --only-keep-debug kernel kernel.sym + objcopy --only-keep-debug kernel kernel.debug objcopy --strip-debug kernel - objcopy --add-gnu-debuglink=kernel.sym kernel + objcopy --add-gnu-debuglink=kernel.debug kernel fd.iso: kernel mkdir -p isodir/boot/grub @@ -90,7 +90,7 @@ run:kernel disk.img ## Run the OS on qemu debug: CFLAGS += $(DEBUG_FLAGS) ## Run the OS on qemu and attach a debugger to it (may need a clean befor to have the debug symbols) debug: CXXFLAGS += $(DEBUG_FLAGS) -debug:kernel kernel.sym disk.img +debug:kernel kernel.debug disk.img gdb -q -x debug.gdb debug_test: CFLAGS += $(DEBUG_FLAGS) -DRUN_TEST @@ -100,7 +100,7 @@ screenshot: shutter --window=qemu -o screenshot_1.png -e && zopflipng screenshot_1.png screenshot_1.png clean: - $(RM) kernel $(asmobj) $(gasmobj) $(cobj) $(deps) $(cinc) fd.iso kernel.sym kernel.map $(docobj) + $(RM) kernel $(asmobj) $(gasmobj) $(cobj) $(deps) $(cinc) fd.iso kernel.debug kernel.map $(docobj) $(RM) -r isodir .PHONY: diff --git a/debug.gdb b/debug.gdb index c6e54ae..167a4e6 100644 --- a/debug.gdb +++ b/debug.gdb @@ -1,4 +1,9 @@ add-symbol-file userspace/user +# Thx to add-gnu-debuglink gdb should know that symbols are in kernel.debug. +# And by default, it should be looking at executable.debug +# But we still have to give him the executable he is suppose to debug (See https://sourceware.org/gdb/current/onlinedocs/gdb.html/Separate-Debug-Files.html) +#add-symbol-file kernel.debug +file kernel source custom_gdb_extension.py #For ASM sources directory arch/x86/:core -- 2.47.0