| Message ID | 5199DBE2.5080707@asianux.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Chen Gang <gang.chen@asianux.com> wrote: > >If neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, the BUG() will >defined as empty (e.g. randconfig with MMU for arm s5pv210) > >In this case, need 'return NULL' to let upper caller knows the failure. Seriously? The correct fix it would seem is to give a useful default BUG definition. Say *NULL. Further we should never hit that code in the first place if it calls BUG. So upper callers should never get there. If it is ever possible to get there the callers need to be fixed. Eric >Signed-off-by: Chen Gang <gang.chen@asianux.com> >--- > include/linux/posix_acl.h | 1 + > 1 files changed, 1 insertions(+), 0 deletions(-) > >diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h >index 7931efe..2c48d06 100644 >--- a/include/linux/posix_acl.h >+++ b/include/linux/posix_acl.h >@@ -104,6 +104,7 @@ static inline struct posix_acl **acl_by_type(struct >inode *inode, int type) > return &inode->i_default_acl; > default: > BUG(); >+ return NULL; > } > } > >-- >1.7.7.6
On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote: > Seriously? The correct fix it would seem is to give a useful default BUG > definition. Say *NULL. Absolutely. The real question is... how is this happening. I've seen it occasionally in the randconfig builds - though I don't have any logs to hand which show that in the build system at the moment. So, looking at the files: arch/arm/include/asm/bug.h: #ifdef CONFIG_BUG #define BUG() _BUG(__FILE__, __LINE__, BUG_INSTR_VALUE) #define _BUG(file, line, value) __BUG(file, line, value) #ifdef CONFIG_DEBUG_BUGVERBOSE #define __BUG(__file, __line, __value) \ ... unreachable(); \ ... #else /* not CONFIG_DEBUG_BUGVERBOSE */ #define __BUG(__file, __line, __value) \ ... unreachable(); \ ... #endif /* CONFIG_DEBUG_BUGVERBOSE */ #define HAVE_ARCH_BUG #endif /* CONFIG_BUG */ #include <asm-generic/bug.h> So here, whenever we have CONFIG_BUG enabled, we always define a non-empty BUG() macro to override the generic version. So far so good. include/asm-generic/bug.h: #ifdef CONFIG_BUG ... we're providing our own version ... #else /* !CONFIG_BUG */ #ifndef HAVE_ARCH_BUG #define BUG() do {} while(0) #endif #ifndef HAVE_ARCH_BUG_ON #define BUG_ON(condition) do { if (condition) ; } while(0) #endif So, the generic stuff becomes no-ops which just fall through. Seriously? Is this really what we want? So either we need to fix the generic bug.h code, or fix every site which uses BUG()/BUG_ON(). Note - I'm not saying *NULL as you did, because that isn't guaranteed to fault on all platforms. NoMMU may allow *NULL to succeed without causing an exception.
On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote: > On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote: >> Seriously? The correct fix it would seem is to give a useful default BUG >> definition. Say *NULL. > > Absolutely. > > The real question is... how is this happening. I've seen it occasionally > in the randconfig builds - though I don't have any logs to hand which > show that in the build system at the moment. So, looking at the files: > Maybe it is my fault for the incorrect configuration. My operation: make EXTRA_CFLAGS=-W ARCH=arm randconfig make EXTRA_CFLAGS=-W ARCH=arm menuconfig set 'arm-linux-gnu-' as cross compiler prefix. select "MMU-based Paged Memory" in "System Type". select "Samsung S5PV210/S5PC110" in "ARM System type" make EXTRA_CFLAGS=-W ARCH=arm ... In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110 Machines" in "System Type", and no machines will be chosen as default. I think, it may be my fault, or need provide a default machine for S5P* with randconfig (also better to select MMU automatically). Thanks.
On Tue, May 21, 2013 at 09:51:57AM +0800, Chen Gang wrote: > On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote: > > On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote: > >> Seriously? The correct fix it would seem is to give a useful default BUG > >> definition. Say *NULL. > > > > Absolutely. > > > > The real question is... how is this happening. I've seen it occasionally > > in the randconfig builds - though I don't have any logs to hand which > > show that in the build system at the moment. So, looking at the files: > > > > Maybe it is my fault for the incorrect configuration. > > My operation: > make EXTRA_CFLAGS=-W ARCH=arm randconfig > make EXTRA_CFLAGS=-W ARCH=arm menuconfig > set 'arm-linux-gnu-' as cross compiler prefix. > select "MMU-based Paged Memory" in "System Type". > select "Samsung S5PV210/S5PC110" in "ARM System type" > make EXTRA_CFLAGS=-W ARCH=arm > ... > > In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110 > Machines" in "System Type", and no machines will be chosen as default. You know, telling us that you're using a random configuration to produce a problem and not providing the actual configuration file is utterly insane. How many iterations do you think it might take to generate a random configuration which shows the same problem you're seeing?
On 05/21/2013 06:59 PM, Russell King - ARM Linux wrote: > On Tue, May 21, 2013 at 09:51:57AM +0800, Chen Gang wrote: >> On 05/21/2013 12:31 AM, Russell King - ARM Linux wrote: >>> On Mon, May 20, 2013 at 07:40:24AM -0700, Eric W. Biederman wrote: >>>> Seriously? The correct fix it would seem is to give a useful default BUG >>>> definition. Say *NULL. >>> >>> Absolutely. >>> >>> The real question is... how is this happening. I've seen it occasionally >>> in the randconfig builds - though I don't have any logs to hand which >>> show that in the build system at the moment. So, looking at the files: >>> >> >> Maybe it is my fault for the incorrect configuration. >> >> My operation: >> make EXTRA_CFLAGS=-W ARCH=arm randconfig >> make EXTRA_CFLAGS=-W ARCH=arm menuconfig >> set 'arm-linux-gnu-' as cross compiler prefix. >> select "MMU-based Paged Memory" in "System Type". >> select "Samsung S5PV210/S5PC110" in "ARM System type" >> make EXTRA_CFLAGS=-W ARCH=arm >> ... >> >> In menuconfig, I do not touch the "S5PV210 Machines" or "S5PC110 >> Machines" in "System Type", and no machines will be chosen as default. > > You know, telling us that you're using a random configuration to produce > a problem and not providing the actual configuration file is utterly > insane. How many iterations do you think it might take to generate a > random configuration which shows the same problem you're seeing? > > Oh, it is my fault for my original reply (it is another issue which also need further discussed) But for current issue, we can repeat it only in menuconfig, unset " > General setup > Configure standard kernel features (expert users) > BUG() Support ", it will report related warnings. For arm architecture, allmodconfig make ARCH=arm allmodconfig make ARCH=arm menuconfig set "arm-linux-gnu-" as cross compiler prefix. unset " > General setup > Configure standard kernel features (expert users) > BUG() Support " make ARCH=arm ... kernel/sched/core.c:2353:1: warning: control reaches end of non-void function [-Wreturn-type] mm/bootmem.c:386:1: warning: control reaches end of non-void function [-Wreturn-type] include/linux/posix_acl.h:108:1: warning: control reaches end of non-void function [-Wreturn-type] ... For x86 architecture, allmodconfig: make allmodconfig make menuconfig unset " > General setup > Configure standard kernel features (expert users) > BUG() Support " make ARCH=arm ... arch/x86/kvm/x86.c:4298:5: warning: ‘exchanged’ may be used uninitialized in this function [-Wuninitialized] kernel/sched/core.c:2353:1: warning: control reaches end of non-void function [-Wreturn-type] mm/mempolicy.c:1755:1: warning: control reaches end of non-void function [-Wreturn-type] include/linux/posix_acl.h:108:1: warning: control reaches end of non-void function [-Wreturn-type] ... So, we need fix the generic bug.h code, it is not arm specific. Thanks.-- Chen Gang Asianux Corporation
diff --git a/include/linux/posix_acl.h b/include/linux/posix_acl.h index 7931efe..2c48d06 100644 --- a/include/linux/posix_acl.h +++ b/include/linux/posix_acl.h @@ -104,6 +104,7 @@ static inline struct posix_acl **acl_by_type(struct inode *inode, int type) return &inode->i_default_acl; default: BUG(); + return NULL; } }
If neither CONFIG_BUG nor HAVE_ARCH_BUG is defined, the BUG() will defined as empty (e.g. randconfig with MMU for arm s5pv210) In this case, need 'return NULL' to let upper caller knows the failure. Signed-off-by: Chen Gang <gang.chen@asianux.com> --- include/linux/posix_acl.h | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)