Subject: segmentation fault on fclose?

From: [email-masked] (Andrew Gillham)

Just two questions...  Should the following code cause
a segmentation fault?  It doesn't on SunOS and Solaris, but
it does on my NetBSD v1.0_beta..
I admit that I am not much of a code hacker, so these may be
*real* obvious screwups..  In case anyone is curious, I'm trying
to get the 'mSQL' SQL database working under NetBSD..

The offensive code:

#include <stdio.h>

main()
{
FILE *fp;
fp = NULL;
fclose(fp);
}

Also, I am getting a segmentation fault like this:

Program received signal SIGSEGV (11), Segmentation fault
0x6a41 in readKey (key=0x90580) at msqldb.c:1094
1094    {
(gdb) where
#0  0x6a41 in readKey (key=0x90580) at msqldb.c:1094
#1  0x91b6 in msqlInsert (table=0x9c050 "test", fields=0xa4080, 
    DB=0x9c010 "bench") at msqldb.c:2630
#2  0x524b in msqlProcessQuery () at msql_proc.c:436
#3  0x28a5 in yyparse () at msql_yacc.y:89
#4  0x9cdc in msqlParseQuery (
    inBuf=0x9d040 "insert into test (name,num) values ('item 0', 0) \n", 
    sock=5) at msql_io.c:51
#5  0x231e in main (argc=1, argv=0xf7bfdbcc) at msqld.c:388
(gdb) 

Here's the code:

(gdb) list
1089    }
1090    
1091    #endif
1092    
1093    u_int readKey(keyDef *key)
1094    {
1095            int     numBytes,
1096                    maxRead,
1097                    fd;
1098            register int    rowLen,
(gdb)

In this case, the "key" data appears to be ok, it's as if
the stack is getting trashed or something.  Basically I haven't
got a clue... :-)

Any help, flames, etc would be appreciated... :-)

-Andrew

---

From: "Charles M. Hannum" <[email-masked]>

Yes, that is solid reasoning.  I do wish the man pages for fclose(),
   flush(), and close() were not so confusing.  fflush accepts NULL,
   if the fp is invalid it shoudl return EOF with errno set to EBADF,
   etc.. Nothing about dumping core!

So if the pointer is 0xdeadbeef, it should nicely return EBADF rather
than core dumping, too?

Just how aggressive do you think the C library should be about
validating what you tell it?

---

From: [email-masked] (grantham)

Andrew Gillham:
>    a segmentation fault?
[...]

Charles Hannum:
> ANSI does not require such a special case, and I really
> don't like the idea of masking bugs in applications by kluging up the
> C library.

In this case, Andrew, I'm pretty sure an application would only
call fclose() with NULL if it had lost track of that stream pointer.
As Charles does, I would certainly consider this an error in
the program, not in the C library.  I considered advocating checking
for NULL in fclose, but it occured to me I would actually rather
that my program dump core, as a warning to me that I was closing
a file twice, closing a file that was never open, or somehow not
handling a file correctly in some other way, rather than blindly
continue on.  I would consider that segmentation fault to be a
debugging aid.

		-Brad
-- 
Brad Grantham, [email-masked] >+------+< Happily slaved to NetBSD/Mac68k!
MacBSD : II, IIx, IIcx, IIci, SE/30, IIsi, IIvx -- The list is growing...

---

From: [email-masked] (Andrew Gillham)

The actual code looked like this:
fp = fopen(path, "r");
if (!fp) {
	fclose(fp);
	...
	return(-1);
	}

I guess that's a bug... :-)

So, now that I'm bothering you gurus... why does *this* fail?

#include <stdio.h>

main()
{
char    buf[512 * 1024];
}

Is this some kernel size limitation?  This same code (mSQL)
is trying to use a 'buf[512 * 1024' and this is causing a
segmentation fault.
Is there anyway to fix this? (i.e. config file option)

Thanks!

-Andrew

---

From: [email-masked] (J.T. Conklin)

> don't like the idea of masking bugs in applications by kluging up the

In my experience, an implementation that masks or otherwise makes it 
difficult to detect errors is not a useful "reference platform" for 
writing portable software.  You'll never know you have a problem until 
you are sent to a vendor's porting center half-way across the country.
And you'll be forced to track it down and fix it with the primitive tools 
that are common on pre-release hardware. :-).

One of the most notable examples of this is AT&T's documenters workbench
(troff, etc.).  It was developed on machines that did not trap null
references.  So, it didn't work at all when it was sold (for lots of
$$$'s) to vendors whos machines did trap null references.

	--jtc

---

From: der Mouse <[email-masked]>

>> I would consider that segmentation fault to be a debugging aid.
> [...]

> Is this some kernel size limitation?

Essentially, yes.  The thing is, the kernel has to have some way to
tell the difference between stack growth and wild pointers.

Normally, the stack is grown automatically by the kernel noticing a
memory fault just below the current stack limit, and growing the stack
to encompass the referred-to location.  But obviously there has to be a
limit, or wild pointer references could easily lead to the kernel
trying to make the stack enormous.  The price of this is that when a
program tries to grow the stack by a big step all at once, a
sufficiently big such step will be beyond the limit.  Half a meg seems
to be "sufficiently big" for your kernel.

> This same code (mSQL) is trying to use a 'buf[512 * 1024' and this is
> causing a segmentation fault.

Not that I know of.  You could probably make it one with comparatively
little work - or at least, it probably could be so made; whether _you_
can do it or not I can't say. :-)

But first, I'd recommend seriously thinking about whether you need to
allocate that much memory on the stack.  Do you really need a half-meg
array to be auto?  If not, consider declaring it static, which will
move it to data or bss, and avoid the stack growth problems you're
seeing.

					der Mouse

			    [email-masked]

---

From: der Mouse <[email-masked]>

>>> Just two questions...  Should the following code cause a
>>> segmentation fault?
>>>    fp = NULL;
>>>    fclose(fp);

>> ANSI does not require such a special case, and I really don't like
>> the idea of masking bugs in applications by kluging up the C
>> library.

I'm with you all the way here.

> I considered advocating checking for NULL in fclose, but it occured
> to me I would actually rather that my program dump core, as a warning
> to me that I was closing a file twice, closing a file that was never
> open, or somehow not handling a file correctly in some other way,
> rather than blindly continue on.  I would consider that segmentation
> fault to be a debugging aid.

Me too.  If I might offer a somewhat programmer-friendlier debugging
aid,

	fclose(NULL) called
	Software abort (core dumped)
	% 

might be preferred over

	Segmentation fault (core dumped)
	% 

and having to fire up a debugger to discover where it died.  Something
like this, perhaps:

	fclose(FILE *fp)
	{
	 if (! fp)
	  { write(2,"fclose(NULL) called\r\n",21);
	    kill(getpid(),SIGABRT);
	  }
	 ....
	}

I don't recommend calling abort() because that has a nasty tendency to
try to fclose() everything, which may be the last thing you want in a
case like this.  (At least it does on our Suns.  I haven't looked at
NetBSD abort().)

Of course, carrying this to its logical extreme would have every libc
routine that takes a pointer argument checking for nil pointers.  A
good case could be made that it's excessive, and I don't really see
anything wrong with the current behavior.

					der Mouse

---

From: [email-masked] (Frank van der Linden)

> Is there anyway to fix this? (i.e. config file option)

Yes, this is a resource limit. The default stacksize limit is
512k. Use 'unlimit' in the shell before running the program,
or add some setrlimit() code to the program. The maximum size
is 8Mb, on i386 at least (see /usr/include/machine/vmparam.h).

You _could_ probably put any of the defines in the config file
(options DFLSSIZ=somebignumber, etc), because vmparam.h has
#ifndefs around those defines. But if some user program includes
<machine/vmparam.h> it might get very confused by that, so I don't
think that would be a good idea.

- Frank

---

From: [email-masked] (Christos Zoulas)

>	  { write(2,"fclose(NULL) called\r\n",21);

I assume that after this, we should all fix our c libraries to
be more friendly such as:

int
strcmp(const char* src1, const char* src2)
{
#define MSG(a) write(2, a, sizeof(a) - 1)
	if (src1 == NULL) {
		MSG("You called strcmp with a null first argument.\r\n\
You should not be doing that, or your program will die.\r\n\
This message is intended to help you debug your program.\r\n");
		kill(0, SIGABRT);
	}

	if (src2 == NULL) { 
		MSG("You called strcmp with a null second argument.\r\n\
You should not be doing that, or your program will die.\r\n\
This message is intended to help you debug your program.\r\n");
	    kill(0, SIGABRT);
	}

	....
}

NOT!

The unix library is written with out any argument checking in most places;
it is intended to be fast and small, and not to protect a programmer from
her/his own bugs.

christos

---

From: der Mouse <[email-masked]>

>>	fclose(FILE *fp) { if (! fp)
>>	  } ....  }

> I assume that after this, we should all fix our c libraries to be
> more friendly such as:  [...]

Yes; didn't you read the rest of my note (or was it another message, I
forget)?  I specifically mentioned that carrying this point of view to
its logical extreme would require silliness like this scattered
throughout the library.

					der Mouse

---

From: [email-masked] (Ty Sarna)

> it is intended to be fast and small, and not to protect a programmer from

The example you gave is a tad excessive, but it might be a good idea to
give the system libraries a liberal sprinking of assert()s controlled by
#ifdef DIAGNOSTIC (by analogy with the kernel, and for the same
reasons). Not only would that help programmers catch errors in their
code more quickly, but it might also catch errors in the libraries
themselves.

-- 
Ty Sarna                   "You can lead a gift horse to water but 
[email-masked]          you can't make him look you in the mouth"