Sparse just seg faulted on me!

Sparse just seg faulted on me!

[Anton Altaparmakov]

I typoed (see CTL_UNNUMBRED should be CTL_UNNUMBERED) and got an  
error which is fine but then sparse seg faulted!  I think that is a  
bug...

$ make CHECKFLAGS="-D__CHECK_ENDIAN__" C=2 modules
[snip]
  CHECK   fs/ntfs/sysctl.c
fs/ntfs/sysctl.c:38:15: error: undefined identifier 'CTL_UNNUMBRED'
/bin/sh: line 1: 26154 Segmentation fault      sparse - 
D__CHECK_ENDIAN__ -Wp,-MD,fs/ntfs/.sysctl.o.d -nostdinc -isystem /usr/ 
lib/gcc/i586-suse-linux/4.1.0/include -D__KERNEL__ -Iinclude -include  
include/linux/autoconf.h -Wall -Wundef -Wstrict-prototypes -Wno- 
trigraphs -fno-strict-aliasing -fno-common -Os -pipe -msoft-float - 
mregparm=3 -mpreferred-stack-boundary=2 -march=i686 -mtune=pentium4 - 
ffreestanding -maccumulate-outgoing-args -DCONFIG_AS_CFI=1 - 
DCONFIG_AS_CFI_SIGNAL_FRAME=1 -Iinclude/asm-i386/mach-default -fno- 
omit-frame-pointer -fno-optimize-sibling-calls -g -fno-stack- 
protector -Wdeclaration-after-statement -Wno-pointer-sign - 
DNTFS_VERSION=\"2.3\" -DDEBUG -DNTFS_RW -DMODULE -D"KBUILD_STR(s)=#s"  
-D"KBUILD_BASENAME=KBUILD_STR(sysctl)" -D"KBUILD_MODNAME=KBUILD_STR 
(ntfs)" fs/ntfs/sysctl.c
make[2]: *** [fs/ntfs/sysctl.o] Error 139
make[1]: *** [fs/ntfs] Error 2
make: *** [fs] Error 2

This is the code snippet should allow you to reproduce it:

#include <linux/module.h>
#include <linux/proc_fs.h>
#include <linux/sysctl.h>

extern int debug_msgs;

/* Definition of the ntfs sysctl. */
static ctl_table ntfs_sysctls[] = {
         {
                 .ctl_name = CTL_UNNUMBRED,      /* No binary ID. */
                 .procname = "ntfs-debug",       /* Text ID. */
                 .data = &debug_msgs,            /* Data pointer and  
size. */
                 .maxlen = sizeof(debug_msgs),
                 .mode = 0644,                   /* Mode. */
                 .proc_handler = &proc_dointvec, /* Proc handler. */
         },
         {}
};

Fixing the typo allows it to complete:

   CHECK   fs/ntfs/sysctl.c
   CC [M]  fs/ntfs/sysctl.o

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

[PATCH] Re: Sparse just seg faulted on me!

[Christopher Li]

> fs/ntfs/sysctl.c:38:15: error: undefined identifier 'CTL_UNNUMBRED'
Sparse left empty ctype when error happen. Sparse should skip
the expand_symbol() at all if error happen. 

Please try this patch:

Fix the segfault when initializer has unknown symbol

Signed-Off-By: Christopher Li <sparse@chrisli.org>

Index: sparse/expand.c
===================================================================
--- sparse.orig/expand.c	2007-02-14 09:28:57.000000000 -0800
+++ sparse/expand.c	2007-02-14 09:44:59.000000000 -0800
@@ -872,7 +872,7 @@ static void verify_nonoverlapping(struct
 	struct expression *b;
 
 	FOR_EACH_PTR(*list, b) {
-		if (a && a->ctype->bit_size && bit_offset(a) == bit_offset(b)) {
+		if (a && a->ctype && a->ctype->bit_size && bit_offset(a) == bit_offset(b)) {
 			sparse_error(a->pos, "Initializer entry defined twice");
 			info(b->pos, "  also defined here");
 			return;

Re: [PATCH] Re: Sparse just seg faulted on me!

[Linus Torvalds]

On Wed, 14 Feb 2007, Christopher Li wrote:
>
> > fs/ntfs/sysctl.c:38:15: error: undefined identifier 'CTL_UNNUMBRED'
>
> Sparse left empty ctype when error happen. Sparse should skip
> the expand_symbol() at all if error happen. 

Actually, it's often better to just explicitly make "ctype" be 
"&bad_ctype" instead.

It's nice if a NULL type means "type has not been evaluated yet", and then 
using "&bad_ctype" to mean "type evaluated to crap".

		Linus

Re: [PATCH] Re: Sparse just seg faulted on me!

[Christopher Li]

On Wed, Feb 14, 2007 at 10:24:09AM -0800, Linus Torvalds wrote:
> Actually, it's often better to just explicitly make "ctype" be 
> "&bad_ctype" instead.
> 
> It's nice if a NULL type means "type has not been evaluated yet", and then 
> using "&bad_ctype" to mean "type evaluated to crap".

Good idea. Some of the code already doing that. I am making it as a separate
patch because touch a lot of code.

Chris

Re: [PATCH] Re: Sparse just seg faulted on me!

[Christopher Li]

On Wed, Feb 14, 2007 at 11:54:47AM -0800, Christopher Li wrote:
> On Wed, Feb 14, 2007 at 10:24:09AM -0800, Linus Torvalds wrote:
> > Actually, it's often better to just explicitly make "ctype" be 
> > "&bad_ctype" instead.
> > 
> > It's nice if a NULL type means "type has not been evaluated yet", and then 
> > using "&bad_ctype" to mean "type evaluated to crap".
> 
> Good idea. Some of the code already doing that. I am making it as a separate
> patch because touch a lot of code.

This is very messy. The current evaluate_* functions are assuming NULL means for
"some error happen in this expression, I can't get the ctype". It is all over the
place.

Chris

Re: [PATCH] Re: Sparse just seg faulted on me!

[Christopher Li]

On Wed, Feb 14, 2007 at 12:48:17PM -0800, Linus Torvalds wrote:
> Yeah. I started out leaving bad stuff as NULL originally, but over time, 
> as I noticed SIGSEGV's, I mostly changed the ones that I ended up having 
> trigger to &bad_ctype.
> 
> Otherwise we either need to test for NULL all the time (and especially 
> since NULL under _some_ circumstances is ok and means "not evaluated yet", 
> that can be confusing), and having to pass up errors higher and higher up.

I think I run into the exact same situations. It start out as simple
rules "&bad_ctype" means error. But then all the test against NULL need
to change to test against &bad_ctpye as well. It is getting subtle for
some place NULL means not evaluated yet.

Chris

Re: [PATCH] Re: Sparse just seg faulted on me!

[Linus Torvalds]

On Wed, 14 Feb 2007, Christopher Li wrote:
> > 
> > It's nice if a NULL type means "type has not been evaluated yet", and then 
> > using "&bad_ctype" to mean "type evaluated to crap".
> 
> Good idea. Some of the code already doing that.

Yeah. I started out leaving bad stuff as NULL originally, but over time, 
as I noticed SIGSEGV's, I mostly changed the ones that I ended up having 
trigger to &bad_ctype.

Otherwise we either need to test for NULL all the time (and especially 
since NULL under _some_ circumstances is ok and means "not evaluated yet", 
that can be confusing), and having to pass up errors higher and higher up.

I at some point considered having a "silent bad_ctype" - a way of saying 
"ok, the type here is bad, but I already warned about the thing that 
_caused_ it to be bad, so don't warn any more about any type clashes".

But I don't think I ever did that, so now you often get two different 
warnings (first a warning about "no such symbol" or something and then a 
warning about "bad types to binary operation" or similar). Anyway, if 
somebody ever gets irritated about the duplicate warnings, I thought I'd 
mention that as a potential way to perhaps avoid havign things that 
already caused errors cause even more verbose errors up the chain..

		Linus

Re: [PATCH] Re: Sparse just seg faulted on me!

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 860 bytes --]

Christopher Li wrote:
> On Wed, Feb 14, 2007 at 11:54:47AM -0800, Christopher Li wrote:
>> On Wed, Feb 14, 2007 at 10:24:09AM -0800, Linus Torvalds wrote:
>>> Actually, it's often better to just explicitly make "ctype" be 
>>> "&bad_ctype" instead.
>>>
>>> It's nice if a NULL type means "type has not been evaluated yet", and then 
>>> using "&bad_ctype" to mean "type evaluated to crap".
>> Good idea. Some of the code already doing that. I am making it as a separate
>> patch because touch a lot of code.
> 
> This is very messy. The current evaluate_* functions are assuming NULL means for
> "some error happen in this expression, I can't get the ctype". It is all over the
> place.

In that case, I will go ahead and apply your patch to eliminate the segfault, and
I can merge a new patch that uses &bad_ctype later.

- Josh Triplett


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]

Re: [PATCH] Re: Sparse just seg faulted on me!

[Josh Triplett]

[-- Attachment #1: Type: text/plain, Size: 333 bytes --]

Christopher Li wrote:
>> fs/ntfs/sysctl.c:38:15: error: undefined identifier 'CTL_UNNUMBRED'
> Sparse left empty ctype when error happen. Sparse should skip
> the expand_symbol() at all if error happen. 
> 
> Please try this patch:
> 
> Fix the segfault when initializer has unknown symbol

Applied.

- Josh Triplett



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 252 bytes --]