Eric Blake | 1 Sep 2008 15:59
Gravatar

Re: next snapshot in preparation for m4 1.4.12


According to Tom G. Christensen on 8/30/2008 7:23 AM:

>> At any rate, I'm also interested in seeing how c-stack behaves if you
>> don't link with libsigsegv (in the earlier snapshot, your build showed
>> that c-stack still attempted stack overflow detection and crashed, causing
>> ./stackovf.test to FAil; my hope is that I've fixed things so that in this
>> snapshot so that c-stack returns ENOTSUPP, letting ./stackovf.test Skip).
>>
> Yes, it seems to do the right thing now.
>
> c_stack_action: Invalid argument
> SKIP: test-c-stack.sh
> c_stack_action: Invalid argument
> SKIP: test-c-stack2.sh

Good.  We've now proven that stack overflow detection is skipped on
platforms where we don't have it ported, rather than causing failures.
It's now only a problem of whether we can easily port things to Irix; and
it's less work if we can just focus on the libsigsegv side of things.

>> Can you include the config.log snippet for that portion of the configure
>> run, so we can see if it was a compile or run failure? 
>>
> configure:6499: checking for working C stack overflow detection
> configure:6591: cc -o conftest -g -I/usr/tgcware/include -L/usr/tgcware/lib
-Wl,-rpath,/usr/tgcware/lib conftest.c  >&5
> cfe: Warning 728: conftest.c, line 62: Long double not supported; double assumed.
>            long double ld;
>         ---^
(Continue reading)

Tom G. Christensen | 1 Sep 2008 20:04

Re: next snapshot in preparation for m4 1.4.12

On Mon, Sep 01, 2008 at 07:59:50AM -0600, Eric Blake wrote:
> According to Tom G. Christensen on 8/30/2008 7:23 AM:
> 
> > configure:6499: checking for working C stack overflow detection
> > configure:6591: cc -o conftest -g -I/usr/tgcware/include -L/usr/tgcware/lib
-Wl,-rpath,/usr/tgcware/lib conftest.c  >&5
> > cfe: Warning 728: conftest.c, line 62: Long double not supported; double assumed.
> >            long double ld;
> >         ---^
> 
> How annoying.  The compiler supports 'long double', per C89 (since long
> double and double are allowed to be the same type), but warns you any time
> you use it.  At any rate, that warning is ignorable (it appears in lots of
> places in your logs).
> 
I figured as much. Adding -woff 728 will silence it but it's easier to
just ignore it.

> > (dbx) t
> >>  0 recurse(p = (nil)) ["/usr/people/tgc/buildpkg/m4/src/conftest.c":96, 0x400bec]
> >    1 recurse(p = 0x7ff0038c = "\001") ["/usr/people/tgc/buildpkg/m4/src/conftest.c":99, 0x400c10]
> >    2 recurse(p = 0x7ff005a4 = "\001") ["/usr/people/tgc/buildpkg/m4/src/conftest.c":99, 0x400c10]
> 
> Hmm, based on the pattern (each frame occupies 536 bytes), I would have
> expected the debugger to report that p = 0x7ff00174 rather than (nil) in
> the final frame before the stack overflow; but that may be a debugger
> anomaly.
>
FWIW gdb has p=0x0 so atleast it's consistent.

(Continue reading)

Eric Blake | 1 Sep 2008 22:48
Gravatar

Re: next snapshot in preparation for m4 1.4.12


According to Tom G. Christensen on 9/1/2008 12:04 PM:
>> I just noticed that the c-stack.m4 file doesn't check the return
>> status from sigaction.  I suspect it worked for you, but just to be sure,
>> could you retry this with this patch to the program in config.log:
>>
>>  <at>  <at>  -85,8 +85,7  <at>  <at>  AC_DEFUN([AC_SYS_XSI_STACK_OVERFLOW_HEURISTIC],
>>            setrlimit (RLIMIT_STACK, &rl);
>>            #endif
>>
>> - -      c_stack_action ();
>> - -      return recurse ("\1");
>> +    return c_stack_action () && recurse ("\1");
>>          }
>>
> I think you hit the nail on the head.

Shoot.  I got the logic backwards.  We want the test to exit with non-zero
status if c_stack_action fails (returns non-zero), or if recurse returns
(which should be impossible).  The conditional should be written with ||,
not &&.  At any rate, it confirms that c_stack_action is succeeding.

So, with my typo fixed, can you once again try those debugging steps, to
see if the breakpoint in overflow_handler trips?

> 
> I can replicate this on Irix 6.2 so probably the same issue there.

That's what I've been figuring too - if we can get Irix 5.3 to work, then
all newer Irix will probably fall into place.
(Continue reading)

Eric Blake | 2 Sep 2008 05:30
Gravatar

Re: diversion failure [was: snapshot in preparation for m4 1.4.12]


According to Eric Blake on 8/19/2008 6:29 AM:
> According to Alain Guibert on 8/19/2008 4:40 AM:
>> Sure. There is a segfault on the test for large precisions, on
>> "%.4000d". On libc5 this format works well until "%.1020d", but
>> segfaults for "%.1021d" and above.
> 
> Bruno, this looks like a vasnprintf bug, since configure accurately
> determined that:
> 
>> | checking whether printf supports large precisions... no
> 
> I'm not sure where to look in vasnprintf to try fixing this, although it
> seems like it might be as simple as using NEED_PRINTF_UNBOUNDED_PRECISION
> in more places.  Do you have suggestions?

In looking at this further, it looks like line 4179 of vasnprintf.c should
_not_ count precision as a prefix if prec_ourselves is nonzero.  Without
the patch to vasnprintf.c, the added test in test-vasnprintf-posix dumped
core when I used

$ gl_cv_func_printf_sizes_c99=no gl_cv_func_printf_infinite=no
gl_cv_func_printf_directive_a=no gl_cv_func_printf_directive_f=no
gl_cv_func_printf_flag_zero=no gl_cv_func_printf_precision=no
./gnulib-tool --with-tests --test vasnprintf-posix

on cygwin.  I traced this to an attempt to call snprintf(buf, 12, "%d%n",
4000, 1234567, &count).

OK to apply?  Are there other files in lib or tests that need matching
(Continue reading)

Eric Blake | 2 Sep 2008 05:39
Gravatar

Re: next snapshot in preparation for m4 1.4.12


According to Alain Guibert on 8/28/2008 8:59 AM:
> Hello Eric,

Hi Alain,

> 
> | 132.diversions fails

I think I finally nailed this, but am waiting for Bruno's response.

> 
> | test-stdint.c:252: two `l's in integer constant

Was this just a warning (ie. it still compiled) or a hard error?  That
test is already filtered on other known broken preprocessors; maybe we add
gcc 2.7 and earlier to the list of preprocessors that should skip that test.

> 
> | c_stack_action: Invalid argument
> | SKIP: test-c-stack.sh
> 
> | c_stack_action: Invalid argument
> | SKIP: test-c-stack2.sh

Did you try with libsigsegv 2.6?

> 
> | test-frexpl.c:104: assertion failed
> | FAIL: test-frexpl
(Continue reading)

Bruno Haible | 2 Sep 2008 12:50

Re: diversion failure [was: snapshot in preparation for m4 1.4.12]

Eric Blake wrote:
> In looking at this further, it looks like line 4179 of vasnprintf.c should
> _not_ count precision as a prefix if prec_ourselves is nonzero.

Yes. Now that you say it, it's obvious that the code in line 4179 should be
in sync with the code in line 4077.

> Without the patch to vasnprintf.c, the added test in test-vasnprintf-posix
> dumped core when I used
> 
> $ gl_cv_func_printf_sizes_c99=no gl_cv_func_printf_infinite=no
> gl_cv_func_printf_directive_a=no gl_cv_func_printf_directive_f=no
> gl_cv_func_printf_flag_zero=no gl_cv_func_printf_precision=no
> ./gnulib-tool --with-tests --test vasnprintf-posix
> 
> on cygwin.  I traced this to an attempt to call snprintf(buf, 12, "%d%n",
> 4000, 1234567, &count).
> 
> OK to apply?  Are there other files in lib or tests that need matching
> modifications?

Yes, please apply. 4 test files should be modified in sync:

$ grep -rl 4000d tests/
tests/test-snprintf-posix.h
tests/test-sprintf-posix.h
tests/test-vasnprintf-posix.c
tests/test-vasprintf-posix.c

Thanks!!
(Continue reading)

Eric Blake | 2 Sep 2008 14:15
Gravatar

Re: diversion failure [was: snapshot in preparation for m4 1.4.12]


According to Bruno Haible on 9/2/2008 4:50 AM:
>>
>> OK to apply?  Are there other files in lib or tests that need matching
>> modifications?
> 
> Yes, please apply. 4 test files should be modified in sync:
> 
> $ grep -rl 4000d tests/
> tests/test-snprintf-posix.h
> tests/test-sprintf-posix.h
> tests/test-vasnprintf-posix.c
> tests/test-vasprintf-posix.c

Done, with all four tests updated.

--
Don't work too hard, make some time for fun as well!

Eric Blake             ebb9 <at> byu.net
Tom G. Christensen | 2 Sep 2008 17:28

Re: next snapshot in preparation for m4 1.4.12

On Mon, Sep 01, 2008 at 02:48:20PM -0600, Eric Blake wrote:
> Shoot.  I got the logic backwards.  We want the test to exit with non-zero
> status if c_stack_action fails (returns non-zero), or if recurse returns
> (which should be impossible).  The conditional should be written with ||,
> not &&.  At any rate, it confirms that c_stack_action is succeeding.
> 
Oh well. Ofcourse the config.log program crashes with this typo fixed.

> So, with my typo fixed, can you once again try those debugging steps, to
> see if the breakpoint in overflow_handler trips?
> 
It does not.

[tgc <at> fortuna tests]$ gdb test-c-stack
GNU gdb 4.17.gnat.3.11p
Copyright 1998 Free Software Foundation, Inc.
GDB is free software, covered by the GNU General Public License, and you are
welcome to change it and/or distribute copies of it under certain conditions.
Type "show copying" to see the conditions.
There is absolutely no warranty for GDB.  Type "show warranty" for details.
This GDB was configured as "mips-sgi-irix5.3"...
(gdb) b overflow_handler
Breakpoint 1 at 0x4015e8: file c-stack.c, line 184.
(gdb) run
Starting program: /usr/people/tgc/buildpkg/m4/src/m4-1.4.11.42-864d/tests/test-c-stack

Program received signal SIGSEGV, Segmentation fault.
0x400d70 in recurse (p=0x0) at test-c-stack.c:46
46      {
(gdb) step
(Continue reading)

Eric Blake | 2 Sep 2008 21:20
Gravatar

Re: next snapshot in preparation for m4 1.4.12


According to Tom G. Christensen on 9/2/2008 9:28 AM:
>> So, with my typo fixed, can you once again try those debugging steps, to
>> see if the breakpoint in overflow_handler trips?
>>
> It does not.

Which means I think we have isolated the bug to the recurse function.
Gnulib's version must be doing something that actually manages to recurse
with p == 0 in the final stack frame, then segv's dereferencing NULL
rather than triggering stack overflow:

static long
recurse (char *p)
{
  char array[500];
  array[0] = 1;
  return *p + recurse (array);
}

Whereas libsigsegv's version goes to greater lengths to try to force a
pure stack overflow:

volatile int *
recurse_1 (int n, volatile int *p)
{
  if (n < INT_MAX)
    *recurse_1 (n + 1, p) += n;
  return p;
}
(Continue reading)

Tom G. Christensen | 2 Sep 2008 22:17

Re: next snapshot in preparation for m4 1.4.12

On Tue, Sep 02, 2008 at 01:20:43PM -0600, Eric Blake wrote:
> I wonder if switching to libsigsegv's version will solve it.  In the
> meantime, I would be curious to see the disassembly of recurse, to see if
> anything obvious appears as to why p==0 rather than a valid stack address
> before the final stack frame.  In gdb, you can get this with 'disas recurse'.
>
(gdb) run
Starting program:
/usr/people/tgc/buildpkg/m4/src/m4-1.4.11.42-864d/tests/test-c-stack

Program received signal SIGSEGV, Segmentation fault.
0x400d70 in recurse (p=0x0) at test-c-stack.c:46
46      {
(gdb) disas recurse
Dump of assembler code for function recurse:
0x400d60 <recurse>:     lui     $gp,0xfc0
0x400d64 <recurse+4>:   addiu   $gp,$gp,29504
0x400d68 <recurse+8>:   addu    $gp,$gp,$t9
0x400d6c <recurse+12>:  addiu   $sp,$sp,-536
0x400d70 <recurse+16>:  sw      $ra,28($sp)
0x400d74 <recurse+20>:  sw      $gp,24($sp)
0x400d78 <recurse+24>:  sw      $a0,536($sp)
0x400d7c <recurse+28>:  li      $t6,1
0x400d80 <recurse+32>:  addiu   $t7,$sp,36
0x400d84 <recurse+36>:  sb      $t6,0($t7)
0x400d88 <recurse+40>:  lw      $t9,-32748($gp)
0x400d8c <recurse+44>:  addiu   $a0,$sp,36
0x400d90 <recurse+48>:  addiu   $t9,$t9,3424
0x400d94 <recurse+52>:  jalr    $t9
0x400d98 <recurse+56>:  nop
(Continue reading)


Gmane