Re: Another note on that optimiser thingy

[Greg Harvey <Greg.Harvey@thezone.net>]

Tel <telford@eng.uts.edu.au> writes:

> Well it's funny that everyone is talking about bugs caused
> by the optimiser -- I just found where I was being bitten by
> it too, probably bitten for some time without realising exactly
> where the problem was coming from. The error:
> 
> insert_test.scm:38:5: In procedure point+ in expression (point+ (random-error-point) (list # # ...)):
> insert_test.scm:38:5: Wrong type argument in position 1: (e{@e{@-01 0.0050657 0.0244)
> 
> 
> Doesn't explain much except that you can see something that looks like
> memory corruption and it is happening to what should be a floating point
> value. What finally turned out to be the cause:
> 
> 
> {
> 	int c;
> 	long pos = 0;
> 	SCM result;
> 
> 	SCM tok_buf = scm_makstr( 100L, 0 );
> 
> 	while( 1 )
> 	{
> 		switch( c = scm_getc( port ))
> 		{
> 			/* lots of cases removed */
> 			default:
> 			{
> 				char *p = SCM_CHARS( tok_buf );
> 
> 				scm_ungetc( c, port );
> 				result = scm_istring2number( p, pos, 10L );
> 				if( SCM_BOOL_F != result ) return( result );
> 				return( SCM_CAR( scm_intern( p, pos )));
> 			}
> 		}
> 	}
> }
> 
> 
> As you can see, *p takes the internal memory of tok_buf which is
> then not referred to before the next return() statement.
> This leaves the optimiser free to throw away tok_buff and scm_istring2number
> can sometimes activate garbage collection.
> 
> The quick solution is to put a volatile in the declaration of
> tok_buf, another (better?) solution is to avoid scm_istring2number()
> and scm_intern(), preferring something that works directly on the
> SCM value. Thus, the following works without the volatile declaration:
> 
> 	default:
> 	{
> 		char *p = SCM_CHARS( tok_buf );
> 		SCM sub = scm_make_shared_substring( tok_buf, SCM_MAKINUM( 0 ),
> 							 SCM_MAKINUM( pos ));
> 
> 		scm_ungetc( c, port );
> 		result = scm_string_to_number( sub, SCM_UNDEFINED );
> 		if( SCM_BOOL_F != result ) return( result );
> 		return( scm_string_to_symbol( sub ));
> 	}
> 
> The reason is that even when tok_buf is thrown away, sub keeps the
> memory usable and sub hangs in there right till the end. The implications
> of this are logical enough once you have gone through it a few times but
> it is going to become a big trap to newbies unless it gets well documented.
> 
> As an aside, it is interesting to note that the libguile code for
> scm_make_shared_substring() uses the optimiser-defeating trick for
> exactly the same reason, so using this function is a way of passing
> the buck. On the other hand scm_string_to_symbol() does NOT use the
> optimiser protection nor does it declare anything volatile.
> Apparently, the only reason that it works is because function arguments
> tend to be on the stack (OK, I'm on 386-linux system, they are ALWAYS
> on the stack) but this is not an absolute truth -- I'll give the UltraPenguin
> a try sometime and see if I can break it.

Actually, it can happen in the x86 as well, since the newer gcc
variants do have an option of passing arguments in registers (though
enabling it breaks damn near everything ;). I think a good
explaination in the docs of the problem, what triggers it, and how to
avoid it in code is probably the best solution. System functions
should also be very robust in this situation, since correctness is
worth the cost of a memory reference or two.
 
> Something for the TODO list I guess is to draw a line around which
> functions can get you into trouble (e.g. SCM_CHARS(), SCM_ROCHARS(), etc)
> and maybe start thinking about the sort of cases those functions have
> been used in.
> 
> One big one is probably SMOB code that takes a SCM value which is
> the SMOB and first thing it does is extract a pointer to a C struct.
> >From there on in, all the work is done with the C struct and the
> SCM pointer is probably left untouched in 9/10 cases. Most of these
> are safe because the SCM value is a function argument so it's on the
> stack. Better than that, most SMOBs aren't created just to be used
> once and destroyed so something else usually points to them.

-- 
Greg