Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Jon Foster]

Hi,

Kronos wrote:
> Comments on the patch?
> 
> diff -Nru -X dontdiff linux-2.4-vanilla/include/asm-i386/page.h linux-2.4/include/asm-i386/page.h
> --- linux-2.4-vanilla/include/asm-i386/page.h	Tue Nov 11 18:05:52 2003
> +++ linux-2.4/include/asm-i386/page.h	Wed Feb  4 14:43:00 2004
> @@ -95,14 +95,28 @@
>   * undefined" opcode for parsing in the trap handler.
>   */
>  
> +#ifdef __bug
> +static inline void __dummy_noreturn(void) __bug;
> +static inline void __dummy_noreturn(void) {
> +	while(1) {}
> +}

My first thought was "this obviously makes the kernel bigger".  GCC will
actually compile this loop - it's only a single jump instruction, possibly
with a nop for branch target alignment, but it's duplicated for every
call to BUG().

On the other hand, marking BUG() as noreturn means that GCC won't have
to generate the code following the BUG().  Even if that code is just
a jump, it's a similar size to the code that this patch adds.  So it's
not as obvious as I first thought, and does need measuring.

Tested with Linux 2.4.22-gentoo-r5 & my normal kernel config, by
measuring total uncompressed size of vmlinux:

Without patch:   3,475,213 bytes
With patch:      3,475,149 bytes
This patch saves:       64 bytes

OK, that saving is lost in the noise, but it seems that this patch
isn't going to change the kernel size much (if at all).  And it is
good to let the compiler know about BUG(), so it doesn't emit
spurious warnings and can catch unused code.  So I like this patch.

Obviously, the most elegent (and space-saving) solution would be
if GCC allowed you to mark a block of inline assembly as noreturn.
Any GCC folks out there able to help?

Kind regards,

Jon


> +#else
> +#define __dummy_noreturn() do {} while(0)
> +#endif
> +
> #if 1 /* Set to zero for a slightly smaller kernel */
> -#define BUG() \
> - __asm__ __volatile__( "ud2\n" \
> - "\t.word %c0\n" \
> - "\t.long %c1\n" \
> - : : "i" (__LINE__), "i" (__FILE__))
> +#define BUG() do { \
> + __asm__ __volatile__( "ud2\n" \
> + "\t.word %c0\n" \
> + "\t.long %c1\n" \
> + : : "i" (__LINE__), "i" (__FILE__)); \
> + __dummy_noreturn(); \
> + } while(0)
> #else
> -#define BUG() __asm__ __volatile__("ud2\n")
> +#define BUG() do { \
> + __asm__ __volatile__("ud2\n"); \
> + __dummy_noreturn(); \
> + } while(0)
> #endif
> 
> #define PAGE_BUG(page) do { \
> diff -Nru -X dontdiff linux-2.4-vanilla/include/linux/compiler.h linux-2.4/include/linux/compiler.h
> --- linux-2.4-vanilla/include/linux/compiler.h Tue Sep 18 23:12:45 2001
> +++ linux-2.4/include/linux/compiler.h Tue Feb 3 18:29:56 2004
> @@ -13,4 +13,11 @@
> #define likely(x) __builtin_expect((x),1)
> #define unlikely(x) __builtin_expect((x),0)
> 
> +#if __GNUC__ >= 3
> +/* __noreturn__ is implemented since gcc 2.5.
> + * __always_inline__ is not present in 2.9x
> + */
> +#define __bug __attribute__((__noreturn__, __always_inline__))
> +#endif
> +
> #endif /* __LINUX_COMPILER_H */

Re: [Compile Regression] 2.4.25-pre8: 126 warnings 0 errors

[Kronos]

Marcelo Tosatti <marcelo.tosatti@cyclades.com> ha scritto:
> This is a shame. These warnings piled up during time.
> 
> It is very likely that all of them are harmless,
> but they need to be fixed.
> 
> Will find to look into some of them. Help is appreciated.

If you want I can work on them in the weekend.

Luca
-- 
Home: http://kronoz.cjb.net
"La teoria e` quando sappiamo come funzionano le cose ma non funzionano.
 La pratica e` quando le cose funzionano ma non sappiamo perche`.
 Abbiamo unito la teoria e la pratica: le cose non funzionano piu` e non
 sappiamo il perche`." -- A. Einstein

Re: [Compile Regression] 2.4.25-pre8: 126 warnings 0 errors

[Kronos]

Il Fri, Jan 30, 2004 at 06:57:03PM -0200, Marcelo Tosatti ha scritto: 
> On Fri, 30 Jan 2004, Kronos wrote:
> > Marcelo Tosatti <marcelo.tosatti@cyclades.com> ha scritto:
> > > This is a shame. These warnings piled up during time.
> > >
> > > It is very likely that all of them are harmless,
> > > but they need to be fixed.
> > >
> > > Will find to look into some of them. Help is appreciated.
> >
> > If you want I can work on them in the weekend.
> 
> If you feel like it, sure :)
> 
> I want only "obvious" fixes for 2.4.25, the rest during 2.4.26-pre.

Done. Allmost all of them seem obvious. I'm splitting diffs right now, I'll 
send patches in reply to this mail. Below is a summary:


Warning List:

aachba.c:1682: warning: cast to pointer from integer of different size
aachba.c:1744: warning: cast to pointer from integer of different size

There is a comment that says that they are ok.

ac97_plugin_ad1980.c:92: warning: initialization from incompatible pointer type

Fixed.

ad1889.c:361: warning: unsigned int format, different type arg (arg 4)

It happens only with CONFIG_HIGHMEM64G=y as dma_addr_t becomes u64.
Fixed by casting dma_addr_t to dma64_addr_t.

agpgart_be.c:5647:17: warning: extra tokens at end of #undef directive

Trivial fix.

../aha152x.c:853: warning: `do_setup' defined but not used

Hum, cannot reproduce.  do_setup is defined and _used_ if
!defined(MODULE).

aha1542.c:114: warning: `setup_str' defined but not used

Fixed.

ali5455.c:2939: warning: `addr2' might be used uninitialized in this function
ali5455.c:2939: warning: `data' might be used uninitialized in this function

Not sure. They are  initialized in a if-branch in a  for loop. I suppose
that sooner or later the branch will be taken.

amd74xx.c:389: warning: `ata66_amd74xx' defined but not used

Yes, this is really unused.

amd76x_pm.c:483: warning: `activate_amd76x_SLP' defined but not used

Fixed.

amd76xrom.c:181: warning: label `err_out_none' defined but not used

This is called by code enclosed in #if 0. Should I remove the label?

amd7930_fn.h:35: warning: `initAMD' defined but not used

There is a static array in a header file. I moved it in the c file.

applicom.c:268:2: warning: #warning "LEAK"
applicom.c:532:2: warning: #warning "Je suis stupide. DW. - copy*user in cli"

Ask the author...

aty128fb.c:1066: warning: `aty128_set_crt_enable' defined but not used
aty128fb.c:1076: warning: `aty128_set_lcd_enable' defined but not used
aty128fb.c:2485: warning: unused variable `fb'
aty128fb.c:2486: warning: unused variable `value'
aty128fb.c:2487: warning: unused variable `rc'

Fixed.

cardbus.c:239: warning: assignment makes pointer from integer without a cast
cardbus.c:239: warning: implicit declaration of function `pci_scan_device_Rsmp_aceae718'

This can  be a real bug.  cardbus.c is calling pci_scan_device  which is
marked as __devinit. Also pci_scan_device prototype isn't exported.  Who
is cardbus maintainer? I founded only a comment by Linus saying that the
file will be removed (dated 2000...).

cfi_cmdset_0001.c:1135: warning: unsigned int format, different type arg (arg 2)
cfi_cmdset_0001.c:1165: warning: unsigned int format, different type arg (arg 2)
cfi_cmdset_0001.c:826: warning: unsigned int format, different type arg (arg 2)
cfi_cmdset_0020.c:1137: warning: unsigned int format, different type arg (arg 3)
cfi_cmdset_0020.c:1286: warning: unsigned int format, different type arg (arg 3)
cfi_cmdset_0020.c:491: warning: unsigned int format, different type arg (arg 3)
cfi_cmdset_0020.c:851: warning: unsigned int format, different type arg (arg 3)

cfi_word can be either 64 or 32 bit wide. Should I cast it to u64 in the
kprintf?

cmdlinepart.c:345: warning: `mtdpart_setup' defined but not used

Fixed.

cpqfcTSi2c.c:62: warning: `i2c_delay' declared `static' but never defined

Fixed.

crc32.c:91: warning: static declaration for `fn_calc_memory_chunk_crc32' follows non-static

Fixed. Btw, while fixing it I found 3 crc32.c:
kronos@notebook:~/src/linux-2.4$ find -name crc32.c
./lib/crc32.c
./arch/mips/lasat/crc32.c
(This one under mips is usign a lookup table to compute the crc)
./drivers/net/wan/8253x/crc32.c

Maybe two of them can be removed...

csr.c:120: warning: long unsigned int format, int arg (arg 3)

Fixed.

/datos/kernel/linux/include/asm/smpboot.h:126: warning: deprecated use of label at end of compound statement

Fixed, seems trivial.

/datos/kernel/linux/include/linux/ixjuser.h:45: warning: `ixjuser_h_rcsid' defined but not used

It is used in drivers/telephony/ixj.c but it seems to me quite useless.

/datos/kernel/linux/include/linux/kdev_t.h:81:1: warning: this is the location of the previous definition

Minor is redefined in drivers/char/tipar.c. Fixed.

/datos/kernel/linux/include/linux/modules/sunrpc_syms.ver:38:1: warning: this is the location of the previous definition

Don't know :)

doc1000.c:85:2: warning: #warning This is definitely not SMP safe. Lock the paging mechanism.

Can't fix.

dtc.c:182: warning: `dtc_setup' defined but not used

Fixed.

fbcon.c:2138: warning: unused variable `line'
fbcon.c:2141: warning: unused variable `dst'
fbcon.c:2141: warning: unused variable `src'
fbcon.c:2142: warning: unused variable `x1'
fbcon.c:2142: warning: unused variable `y1'

They are used under several #defines, I don't think that it's worth
moving them around.

fbdev.c:2224: warning: label `err_out_kfree' defined but not used

Fixed.

../fdomain.c:565: warning: `fdomain_setup' defined but not used

Done.

file.c:64: warning: `rc' might be used uninitialized in this function

(under fs/intermezzo) I think it's ok:

static int presto_open_upcall(int minor, struct dentry *de)
{
        int rc;
	struct presto_dentry_data *dd = presto_d2d(de);
	...
	if (dd->remote_ino == 0) {
		rc = presto_get_fileid(minor, fset, de);
	}
	...
	if (dd->remote_ino > 0) {
		...
	} else
		CPRINTF("blabla", rc);

remote_ino is unsigned so rc won't be used without initialization.
	
gamma_dma.c:608:18: warning: #warning list_entry() is needed here

Yup, the same thing should be done in drmP.h...done.

g_NCR5380.c:212: warning: `do_NCR5380_setup' defined but not used
g_NCR5380.c:230: warning: `do_NCR53C400_setup' defined but not used
g_NCR5380.c:248: warning: `do_NCR53C400A_setup' defined but not used
g_NCR5380.c:266: warning: `do_DTC3181E_setup' defined but not used

Fixed.

hc_sl811_rh.c:171: warning: duplicate `const'
hc_sl811_rh.c:416: warning: duplicate `const'
hc_sl811_rh.c:421: warning: duplicate `const'

I doesn't even compile if not build as module... fixed. Warnings comes
from nested min(), gcc 3.3.2 doesn't whine.

hid-core.c:879: warning: implicit declaration of function `hiddev_report_event'

Missing prototype in include/linux/hiddev.h, fixed

i2o_block.c:510: warning: `i2ob_flush' defined but not used

Wrapped with #if 0

i2o_pci.c:393:1: warning: no newline at end of file

Not true :)

idt77252.c:669: warning: unsigned int format, different type arg (arg 5)

dma_addr_t...

interrupt.c:201: warning: unsigned int format, different type arg (arg 4)

dma_addr_t again. Maybe we need a special printf conversion specifier.

ip_conntrack_core.c:1161: warning: passing arg 1 of `unhelp' discards qualifiers from pointer target type

LIST_FIND_W cast to const the first element that gets passed to the
helper function (unhelp). I see no easy way to fix this (I'm not
familiar with netfilter code) because unhelp modifies its first arg.

ircomm_param.c:202: warning: concatenation of string literals with __FUNCTION__ is deprecated

Fixed (btw somebody half-fixed this...)

irlmp.c:1244: warning: concatenation of string literals with __FUNCTION__ is deprecated
irlmp.c:1258: warning: concatenation of string literals with __FUNCTION__ is deprecated
irlmp.c:1277: warning: concatenation of string literals with __FUNCTION__ is deprecated
irlmp.c:1284: warning: concatenation of string literals with __FUNCTION__ is deprecated

Fixed

it8181fb.c:162: warning: `fontname' defined but not used

Fixed + plus removal of unneeded initialization to zero.

ixj.h:41: warning: `ixj_h_rcsid' defined but not used

Shut up! Don't know why it complains. ixj_h_rcsid seems used.

kaweth.c:738: warning: assignment from incompatible pointer type

Fixed.

linuxvfs.c:682: warning: `befs_listxattr' defined but not used
linuxvfs.c:690: warning: `befs_getxattr' defined but not used
linuxvfs.c:697: warning: `befs_setxattr' defined but not used
linuxvfs.c:703: warning: `befs_removexattr' defined but not used

Extended attribute not implemented yet. Wrapped with #if 0 + comment.

ma600.c:51:22: warning: extra tokens at end of #undef directive

Trivial.

main.c:236:2: warning: #warning "Initialisation order race. Must register after usable"

This is /drivers/sound/emu10k1/main.c. Don't know how to fix.

make[1]: warning: jobserver unavailable: using -j1.  Add `+' to parent make rule.

Uh?

matroxfb_g450.c:134: warning: duplicate `const'
matroxfb_g450.c:135: warning: duplicate `const'
matroxfb_g450.c:561: warning: unused variable `minfo'
matroxfb_maven.c:359: warning: duplicate `const'
matroxfb_maven.c:360: warning: duplicate `const'

Fixed.

meye.c:212: warning: passing arg 3 of `dma_alloc_coherent' from incompatible pointer type

Fixed, but be carefull with  this. The driver consider dma_addr_t == u32
which is  not always true  even on i386  (CONFIG_HIGHMEM64G). I replaced
u32 with dma_addr_t where appropriate, but I don't think that the driver
is 64 bit clean anyway.

mpc.c:330: warning: `mpoa_device_type_string' defined but not used

It's marked as unused. Maybe the OP is using an older gcc.

mptbase.c:2906: warning: `pCached' might be used uninitialized in this function

Fixed.

NCR5380.c:402: warning: `NCR5380_print' defined but not used
NCR5380.c:458: warning: `NCR5380_print_phase' defined but not used
NCR5380.c:684: warning: `NCR5380_probe_irq' defined but not used
NCR5380.c:738: warning: `NCR5380_print_options' defined but not used

Not  sure. Comments   says  that  NCR5380_probe_irq  is   defined  under
AUTOPROBE_IRQ but it's not. It's  also stated that NCR5380_print_options
must be called  in the init code (but again  it's not). You'd better ask
the maintainer.

nfs3proc.c:48:1: warning: "rpc_call_sync" redefined

Hum, here the function rpc_call_sync is overridded with a macro. At
least this is unclean.

ns83820.c:1708: warning: `ns83820_probe_phy' defined but not used

Fixed.

pc300_tty.c:706: warning: passing arg 2 of pointer to function discards qualifiers from pointer target type

We are losing a volatile and I don't see why it's used. Also there a lot
of "volatile" all over the driver. Not touched.

pci-pc.c:180:1: warning: "PCI_CONF1_ADDRESS" redefined
pci-pc.c:58:1: warning: this is the location of the previous definition

PCI_CONF1_ADDRESS is defined on line 58 #if CONFIG_MULTIQUAD. It's
redefined on line 180 (outside #if CONFIG_MULTIQUAD). I added an #undef
right before the end of the CONFIG_MULTIQUAD block

pm3fb.c:3835: warning: passing arg 2 of `__release_region_Rsmp_d49501d4' makes integer from pointer without a cast
pm3fb.c:3838: warning: passing arg 2 of `__release_region_Rsmp_d49501d4' makes integer from pointer without a cast

The author didn't understand how to use ioremap ;) I think that it's ok
on i386 though, I can add a cast to shut up gcc.

r128_cce.c:357: warning: unsigned int format, different type arg (arg 3)

dma_addr_t

radeon_cp.c:908: warning: unsigned int format, different type arg (arg 3)

idem

radeon_mem.c:135: warning: `print_heap' defined but not used

Gone.

scsi_merge.c:104: warning: long unsigned int format, different type arg (arg 4)

page_to_phys returns a u64 if CONFIG_HIGHMEM64G is defined.

sddr09.c:448: warning: `sddr09_request_sense' defined but not used

-EWONTFIX ;) sddr09_request_sense is called by the init function, which
is wrapped by a #if 0.

siimage.c:65: warning: control reaches end of non-void function

The last statement before the end is BUG(), but I added a return to
silence the warning.

sstfb.c:971: warning: unused variable `i'

Fixed.

st5481_b.c:122: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_b.c:176: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_b.c:367: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_d.c:362: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_d.c:386: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_d.c:419: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_d.c:453: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_d.c:602: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:134: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:188: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:244: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:253: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:263: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:360: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:474: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:504: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:510: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:512: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:514: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:51: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:522: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:609: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:612: warning: concatenation of string literals with __FUNCTION__ is deprecated
st5481_usb.c:67: warning: concatenation of string literals with __FUNCTION__ is deprecated

Fixed.

sundance.c:1678: warning: unsigned int format, different type arg (arg 3)
sundance.c:994: warning: unsigned int format, different type arg (arg 3)

dma_addr_t

super.c:164: warning: `fork' might be used uninitialized in this function
super.c:164: warning: `names' might be used uninitialized in this function

This may be real. Don't know how to fix.

tipar.c:76:1: warning: "minor" redefined

Fixed, see above.

tms380tr.c:1449: warning: long unsigned int format, different type arg (arg 3)

Bad cast, fixed.

/datos/kernel/linux/include/linux/autoconf.h:2070:1: warning: "CONFIG_SERIAL_SHARE_IRQ" redefined
vac-serial.c:13:1: warning: this is the location of the previous definition

Fixed. But why vac-serial.c and serial.c defines
CONFIG_SERIAL_SHARE_IRQ? It should be defined by the build system (if
user configured it...)

vesafb.c:93: warning: `fbcon_cmap' defined but not used

It's used if CONFIG_FBCON_CFB16 or CONFIG_FBCON_CFB32 are defined.

wd7000.c:832: warning: `p' might be used uninitialized in this function

Can't happen. needed is always >= 1 so p will be initialized. gcc is not
smart enough ;)

xfs_log_recover.c:1534: warning: `flags' might be used uninitialized in this function

Fixed by Nathan Scott

yellowfin.c:1282: warning: unsigned int format, different type arg (arg 2)
yellowfin.c:1294: warning: unsigned int format, different type arg (arg 2)

Fixed.


ciao,
Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
Se non puoi convincerli, confondili.

[Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Kronos]

siimage.c:65: warning: control reaches end of non-void function

The last statement before the end is BUG(), but I added a return to
silence the warning.

diff -Nru -X dontdiff linux-2.4-vanilla/drivers/ide/pci/siimage.c linux-2.4/drivers/ide/pci/siimage.c
--- linux-2.4-vanilla/drivers/ide/pci/siimage.c	Tue Nov 11 17:51:38 2003
+++ linux-2.4/drivers/ide/pci/siimage.c	Sat Jan 31 19:07:56 2004
@@ -62,6 +62,9 @@
 			return 0;
 	}
 	BUG();
+
+	/* gcc will complain */
+	return 0;
 }
  
 /**

-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
You and me baby ain't nothin' but mammals
So let's do it like they do on the Discovery Channel

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Geert Uytterhoeven]

On Mon, 2 Feb 2004, Kronos wrote:
> siimage.c:65: warning: control reaches end of non-void function
>
> The last statement before the end is BUG(), but I added a return to
> silence the warning.
>
> diff -Nru -X dontdiff linux-2.4-vanilla/drivers/ide/pci/siimage.c linux-2.4/drivers/ide/pci/siimage.c
> --- linux-2.4-vanilla/drivers/ide/pci/siimage.c	Tue Nov 11 17:51:38 2003
> +++ linux-2.4/drivers/ide/pci/siimage.c	Sat Jan 31 19:07:56 2004
> @@ -62,6 +62,9 @@
>  			return 0;
>  	}
>  	BUG();
> +
> +	/* gcc will complain */
> +	return 0;
>  }

What about adding `attribute ((noreturn))' to the declaration of BUG() instead?

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Kronos]

Il Mon, Feb 02, 2004 at 10:08:48PM +0100, Geert Uytterhoeven ha scritto: 
> On Mon, 2 Feb 2004, Kronos wrote:
> > siimage.c:65: warning: control reaches end of non-void function
> >
> > The last statement before the end is BUG(), but I added a return to
> > silence the warning.
> >
> > diff -Nru -X dontdiff linux-2.4-vanilla/drivers/ide/pci/siimage.c linux-2.4/drivers/ide/pci/siimage.c
> > --- linux-2.4-vanilla/drivers/ide/pci/siimage.c	Tue Nov 11 17:51:38 2003
> > +++ linux-2.4/drivers/ide/pci/siimage.c	Sat Jan 31 19:07:56 2004
> > @@ -62,6 +62,9 @@
> >  			return 0;
> >  	}
> >  	BUG();
> > +
> > +	/* gcc will complain */
> > +	return 0;
> >  }
> 
> What about adding `attribute ((noreturn))' to the declaration of BUG() instead?

BUG() is a macro, it won't work.

Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
#include <stdio.h> 
int main(void) {printf("\t\t\b\b\b\b\b\b");
printf("\t\t\b\b\b\b\b\b");return 0;}

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Kronos]

Il Mon, Feb 02, 2004 at 10:08:48PM +0100, Geert Uytterhoeven ha scritto: 
> On Mon, 2 Feb 2004, Kronos wrote:
> > siimage.c:65: warning: control reaches end of non-void function
> >
> > The last statement before the end is BUG(), but I added a return to
> > silence the warning.
> >
> > diff -Nru -X dontdiff linux-2.4-vanilla/drivers/ide/pci/siimage.c linux-2.4/drivers/ide/pci/siimage.c
> > --- linux-2.4-vanilla/drivers/ide/pci/siimage.c	Tue Nov 11 17:51:38 2003
> > +++ linux-2.4/drivers/ide/pci/siimage.c	Sat Jan 31 19:07:56 2004
> > @@ -62,6 +62,9 @@
> >  			return 0;
> >  	}
> >  	BUG();
> > +
> > +	/* gcc will complain */
> > +	return 0;
> >  }
> 
> What about adding `attribute ((noreturn))' to the declaration of BUG() instead?

Ok, I cooked up this patch. BUG() is a function marked as noreturn that is
always inlined. Unfortunately, gcc prior to 3.x does not support
__always_inline__ attribute, so I had to revert to the old macro with older
compiler.

The patch fixes warnings with newer compiler, but not with older ones.
Note that the while(1) is needed, otherwise gcc will say that the
function marked as noreturn does actually return.

Comments?

diff -Nru -X dontdiff linux-2.4-vanilla/include/asm-i386/page.h linux-2.4/include/asm-i386/page.h
--- linux-2.4-vanilla/include/asm-i386/page.h	Tue Nov 11 18:05:52 2003
+++ linux-2.4/include/asm-i386/page.h	Tue Feb  3 07:26:04 2004
@@ -10,6 +10,7 @@
 #ifndef __ASSEMBLY__
 
 #include <linux/config.h>
+#include <linux/compiler.h>
 
 #ifdef CONFIG_X86_USE_3DNOW
 
@@ -94,6 +95,26 @@
  * The offending file and line are encoded after the "officially
  * undefined" opcode for parsing in the trap handler.
  */
+#ifdef __bug
+#if 1	/* Set to zero for a slightly smaller kernel */
+__bug void __bugfn(void) {
+	while(1) {
+		 __asm__ __volatile__(	"ud2\n"
+					"\t.word %c0\n"
+					"\t.long %c1\n"
+					: : "i" (__LINE__), "i" (__FILE__));
+ 	}
+}
+#else
+__bug void __bugfn(void) {
+	while(1) {
+		__asm__ __volatile__("ud2\n");
+	}
+}
+#endif
+#define BUG() __bugfn()
+
+#else /* __bug */
 
 #if 1	/* Set to zero for a slightly smaller kernel */
 #define BUG()				\
@@ -104,6 +125,8 @@
 #else
 #define BUG() __asm__ __volatile__("ud2\n")
 #endif
+
+#endif /* __bug */
 
 #define PAGE_BUG(page) do { \
 	BUG(); \
diff -Nru -X dontdiff linux-2.4-vanilla/include/linux/compiler.h linux-2.4/include/linux/compiler.h
--- linux-2.4-vanilla/include/linux/compiler.h	Tue Sep 18 23:12:45 2001
+++ linux-2.4/include/linux/compiler.h	Tue Feb  3 07:25:57 2004
@@ -13,4 +13,11 @@
 #define likely(x)	__builtin_expect((x),1)
 #define unlikely(x)	__builtin_expect((x),0)
 
+#if __GNUC__ >= 3
+/* __noreturn__ is implemented since gcc 2.5. 
+ * __always_inline__ is not present in 2.9x
+ */
+#define __bug __attribute__((__noreturn__, __always_inline__))
+#endif
+
 #endif /* __LINUX_COMPILER_H */


Luca
PS: I wrote the patch early this morning, on train. It may be a huge pile of
crap ;)
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
The trouble with computers is that they do what you tell them, 
not what you want.
D. Cohen

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Philippe Elie]

On Tue, 03 Feb 2004 at 22:07 +0000, Kronos wrote:

> Ok, I cooked up this patch. BUG() is a function marked as noreturn that is
> always inlined. Unfortunately, gcc prior to 3.x does not support
> __always_inline__ attribute, so I had to revert to the old macro with older
> compiler.
> 
> The patch fixes warnings with newer compiler, but not with older ones.
> Note that the while(1) is needed, otherwise gcc will say that the
> function marked as noreturn does actually return.
> 
> Comments?
> 
> diff -Nru -X dontdiff linux-2.4-vanilla/include/asm-i386/page.h linux-2.4/include/asm-i386/page.h
> --- linux-2.4-vanilla/include/asm-i386/page.h	Tue Nov 11 18:05:52 2003
> +++ linux-2.4/include/asm-i386/page.h	Tue Feb  3 07:26:04 2004
> @@ -10,6 +10,7 @@
>  #ifndef __ASSEMBLY__
>  
>  #include <linux/config.h>
> +#include <linux/compiler.h>
>  
>  #ifdef CONFIG_X86_USE_3DNOW
>  
> @@ -94,6 +95,26 @@
>   * The offending file and line are encoded after the "officially
>   * undefined" opcode for parsing in the trap handler.
>   */
> +#ifdef __bug
> +#if 1	/* Set to zero for a slightly smaller kernel */
> +__bug void __bugfn(void) {
> +	while(1) {
> +		 __asm__ __volatile__(	"ud2\n"
> +					"\t.word %c0\n"
> +					"\t.long %c1\n"
> +					: : "i" (__LINE__), "i" (__FILE__));
> + 	}
> +}

You must pass __LINE__ and __FILE__ as parameter to this function.

regards,
Philippe Elie

Re: [Compile Regression in 2.4.25-pre8][PATCH 37/42]

[Kronos]

Il Tue, Feb 03, 2004 at 11:14:21PM +0000, Philippe Elie ha scritto: 
> > diff -Nru -X dontdiff linux-2.4-vanilla/include/asm-i386/page.h linux-2.4/include/asm-i386/page.h
> > --- linux-2.4-vanilla/include/asm-i386/page.h	Tue Nov 11 18:05:52 2003
> > +++ linux-2.4/include/asm-i386/page.h	Tue Feb  3 07:26:04 2004
> > @@ -10,6 +10,7 @@
> >  #ifndef __ASSEMBLY__
> >  
> >  #include <linux/config.h>
> > +#include <linux/compiler.h>
> >  
> >  #ifdef CONFIG_X86_USE_3DNOW
> >  
> > @@ -94,6 +95,26 @@
> >   * The offending file and line are encoded after the "officially
> >   * undefined" opcode for parsing in the trap handler.
> >   */
> > +#ifdef __bug
> > +#if 1	/* Set to zero for a slightly smaller kernel */
> > +__bug void __bugfn(void) {
> > +	while(1) {
> > +		 __asm__ __volatile__(	"ud2\n"
> > +					"\t.word %c0\n"
> > +					"\t.long %c1\n"
> > +					: : "i" (__LINE__), "i" (__FILE__));
> > + 	}
> > +}
> 
> You must pass __LINE__ and __FILE__ as parameter to this function.

Good point. It's not that easy though ;)
BUG() places file name and line number after the invalid opcode (ud2) and
the kernel trap handler decodes these information when printing a BUG().
In order to do that you need immediate values to pass to assembler.

I think that the following patch will work. This is a BUG() from a dummy module
which calls BUG() in the init function:

kernel BUG at /tmp/mod3.c:22!
invalid operand: 0000 [#3]
CPU:    0
EIP:    0060:[<e09ed003>]    Tainted: PF
EFLAGS: 00010202
EIP is at modinit+0x3/0x20 [mod3]
eax: e09ed000   ebx: cdf9a000   ecx: c036de78   edx: 00000000
esi: e09ed180   edi: c036de78   ebp: cdf9bfa4   esp: cdf9bfa4
ds: 007b   es: 007b   ss: 0068
Process insmod (pid: 1960, threadinfo=cdf9a000 task=dd7546b0)
Stack: cdf9bfbc c01345d8 0804b018 080486dd 4009199b c036de5c cdf9a000 c0109067
       0804b018 0000055c 0804b008 080486dd 4009199b bffffb28 00000080 0000007b
       0000007b 00000080 400fb4ce 00000073 00000246 bffffae0 0000007b
Call Trace:
 [<c01345d8>] sys_init_module+0x118/0x240
 [<c0109067>] syscall_call+0x7/0xb

Code: 0f 0b 16 00 25 d0 9e e0 90 8d 74 26 00 eb fe 8d b4 26 00 00
 Segmentation fault
		 
It seems ok to me (kernel is tainted because I forgot MODULE_LICENSE...).
Comments on the patch?

diff -Nru -X dontdiff linux-2.4-vanilla/include/asm-i386/page.h linux-2.4/include/asm-i386/page.h
--- linux-2.4-vanilla/include/asm-i386/page.h	Tue Nov 11 18:05:52 2003
+++ linux-2.4/include/asm-i386/page.h	Wed Feb  4 14:43:00 2004
@@ -95,14 +95,28 @@
  * undefined" opcode for parsing in the trap handler.
  */
 
+#ifdef __bug
+static inline void __dummy_noreturn(void) __bug;
+static inline void __dummy_noreturn(void) {
+	while(1) {}
+}
+#else
+#define __dummy_noreturn() do {} while(0)
+#endif
+
 #if 1	/* Set to zero for a slightly smaller kernel */
-#define BUG()				\
- __asm__ __volatile__(	"ud2\n"		\
-			"\t.word %c0\n"	\
-			"\t.long %c1\n"	\
-			 : : "i" (__LINE__), "i" (__FILE__))
+#define BUG() do {						\
+		__asm__ __volatile__(	"ud2\n"			\
+			"\t.word %c0\n"				\
+			"\t.long %c1\n"				\
+			 : : "i" (__LINE__), "i" (__FILE__));	\
+		__dummy_noreturn();				\
+	} while(0)
 #else
-#define BUG() __asm__ __volatile__("ud2\n")
+#define BUG() do {						\
+		__asm__ __volatile__("ud2\n");			\
+		__dummy_noreturn();				\
+	} while(0)
 #endif
 
 #define PAGE_BUG(page) do { \
diff -Nru -X dontdiff linux-2.4-vanilla/include/linux/compiler.h linux-2.4/include/linux/compiler.h
--- linux-2.4-vanilla/include/linux/compiler.h	Tue Sep 18 23:12:45 2001
+++ linux-2.4/include/linux/compiler.h	Tue Feb  3 18:29:56 2004
@@ -13,4 +13,11 @@
 #define likely(x)	__builtin_expect((x),1)
 #define unlikely(x)	__builtin_expect((x),0)
 
+#if __GNUC__ >= 3
+/* __noreturn__ is implemented since gcc 2.5. 
+ * __always_inline__ is not present in 2.9x
+ */
+#define __bug __attribute__((__noreturn__, __always_inline__))
+#endif
+
 #endif /* __LINUX_COMPILER_H */


Luca
-- 
Reply-To: kronos@kronoz.cjb.net
Home: http://kronoz.cjb.net
"L'ottimista pensa che questo sia il migliore dei mondi possibili. 
 Il pessimista sa che e` vero" -- J. Robert Oppenheimer