On Mon, Oct 04, 2010 at 02:02:12PM -0600, Myles Watson wrote:
On Sun, Oct 3, 2010 at 5:10 PM, Peter Stuge peter@stuge.se wrote:
Rudolf just found a bug in the sb700 code:
u32 dword; .. dword = pci_read_config8(dev, 0x64); dword |= 1 << 10; pci_write_config8(dev, 0x64, dword);
And I'm ranting now, because a pci_set8() macro/function could have found this bug at compile time, and because I don't like these constructs. (Compiler warnings would also have indicated a problem. They're currently disabled for this code.)
How and where are compiler warnings disabled for this code?
That was a misunderstanding I think, I mentioed (on IRC) warnings being disabled for some parts of the coreboot code (tiny bootblock and others), but that was not related to this specific code (sb700_sm.c).
The file gets compiled with:
gcc -m32 -Wa,--divide -fno-stack-protector -Wl,--build-id=none -MMD -Isrc -Isrc/include -Ibuild -Isrc/arch/i386/include -Isrc/devices/oprom/include -include /home/uwe/v4_bar/build/config.h -Os -nostdinc -pipe -g -nostdlib -Wall -Wundef -Wstrict-prototypes -Wmissing-prototypes -Wwrite-strings -Wredundant-decls -Wno-trigraphs -Wstrict-aliasing -Wshadow -Werror -fno-common -ffreestanding -fno-builtin -fomit-frame-pointer -c -o build/southbridge/amd/sb700/sb700_sm.driver.o src/southbridge/amd/sb700/sb700_sm.c
However, that doesn't expose the bug in this case. What would work is to add '-Wconversion', that finds the bug:
src/southbridge/amd/sb700/sb700_sm.c:88: error: conversion to ‘uint8_t’ from ‘u32’ may alter its value
However, it also spews 30 or more of these lines for every single file we have in coreboot, so it's not really a practical option to enable permanently (or at least not at this point in time).
Uwe.