RE: "[PATCH] take 2: cc1/f771 array overrun - x86 Floating Point"

[Donn Terry <donnte at microsoft dot com>]

With respect to making this always fatal: I would have
except that I expected that I'd antagonize more people
than I wanted to.  It's fine with me.

With respect to style: I'm also working in an environment
that expressly prohibits two of the most common elements
of Gnu style: space after function name and { on a separate
line after an if, so forgive me if I miss on that occasionally.
Another environment requires the use of () whenever naming a
function in text.  I suspect I'm not alone here.  I'll do the
best I can, but without a single set of habits, it's hard
to see them.

Anyway, thanks.

Donn

> -----Original Message-----
> From: Jeffrey A Law [mailto:law@cygnus.com]
> Sent: Friday, July 07, 2000 6:36 AM
> To: Donn Terry
> Cc: 'gcc-bugs@gcc.gnu.org'
> Subject: Re: "[PATCH] take 2: cc1/f771 array overrun - x86 Floating
> Point" 
> 
> 
> 
>   In message 
> <309F4FC4705DC844987051A517E9E39BF5BCB5@red-pt-02.redmond.corp.mic
> rosoft.com>you write:
>   > This is take 2, differing from the prior version by the use of
>   > ENABLE_CHECKING and an abort call, per Jeff's suggestion.
>   > The code continues to fail soft when checking is disabled, since 
>   > there appears to be no downside to it, and the possible SIGSEGV 
>   > is avoided.
> I don't like the code that fails soft -- that just encourages 
> people to
> ignore the problem by hacking around the real bug and I can envision
> ways to make your fail-soft fail in other strange and unusual ways.
> 
> I'd rather put the entire check in an ENABLE_CHECKING block, 
> which also has
> the benefit of formatting correctly :-)  Also note that 
> ENABLE_CHECKING
> is on by default in the development trees, so it's likely people will
> start triggering this failure immediately -- I doubt the bug 
> will survive
> very long.
> 
> Again, I think the way to fix this is to have 
> init_insn_lengths be called
> again after reg-stack has finished.
> 
> You always need a space before the open paren for an argument 
> list, you
> missed it in a couple places in your patch.
> 
> We don't refer to function names in comments with "()" -- just use the
> name of the function.  Actually, since we're putting in a 
> generic sanity
> test, I think the comment is better worded as
> 
> /* This can be caused by bugs elsewhere in the compiler if 
> new insns are 
>    created after init_insn_lengths is called.  */
> 
> Here's the patch I'm checking in.
> 
> Index: final.c
> ===================================================================
> RCS file: /cvs/gcc/egcs/gcc/final.c,v
> retrieving revision 1.134
> diff -c -3 -p -r1.134 final.c
> *** final.c	2000/06/13 16:06:26	1.134
> --- final.c	2000/07/07 13:31:59
> *************** final (first, file, optimize, prescan)
> *** 2014,2019 ****
> --- 2014,2025 ----
>     for (insn = NEXT_INSN (first); insn;)
>       {
>   #ifdef HAVE_ATTR_length
> + #ifdef ENABLE_CHECKING
> +       /* This can be triggered by bugs elsewhere in the compiler if
> + 	 new insns are created after init_insn_lengths is called.  */
> +       if (INSN_UID (insn) >= insn_lengths_max_uid)
> + 	abort ();
> + #endif
>         insn_current_address = insn_addresses[INSN_UID (insn)];
>   #endif
>         insn = final_scan_insn (insn, file, optimize, prescan, 0);
> 
>