Re: [PATCH]: Use RAX_LP/RDX_LP on SAVE_PTR in sysdeps/x86_64/strtok.S

[Roland McGrath <roland at hack dot frob dot com>]

> This test is very sensitive to stack layout.  -O1 with GCC 4.6 will
> create a stack layout such that the test fails on x32 with a broken
> strtok_r implementation.

That is so fragile that I don't think this is a good thing to rely on in
the test suite.  Perhaps it would be better to write an explicit wrapper
function in assembly that ensures the stack situation that tickles the bug.

> Since strtok_r will update line, we can't use const.

Oops, good point.

> >> - ? ? movq %rdx, %rax
> >> ? ? ? movq %rdx, %r9 ? ? ? ? ?/* Save value - see def. of SAVE_PTR. ?*/
> >
> > Might as well make SAVE_PTR %R9_LP and use 'mov %RDX_LP, SAVE_PTR' here.
> >
> 
> We can't since SAVE_PTR is a memory location, not a register.

Oh, right.  It can still be %RDX_LP, which I see you've done.

> +/* Test strtok_r

It's worthwhile to say it's specifically a regression test for BZ #14229.

> +  return strcmp (tok, "udf");

It's probably better to use != 0 so the failing return value is always 1
even if strcmp returns some arbitrary positive or negative value.


The fix itself looks fine to me now, as do most of the nits about the test.
But I think we need a different method to ensure the test continues to test
the problem scenario, that is less fragile than assuming the compiler
happens to produce a certain stack layout.


Thanks,
Roland