Re: Misaligned stack on 32-bit s390?

[Andreas Krebbel <krebbel at linux dot vnet dot ibm dot com>]

On 11/11/2014 10:49 AM, Carlos O'Donell wrote:
> On 11/11/2014 04:28 AM, Richard Henderson wrote:
>> On 11/11/2014 06:31 AM, Carlos O'Donell wrote:
>>> Any clever ideas on how to fix this without copying up a large
>>> portion of the stack?
>>
>> Nope, because other targets do in fact have to do just that.
> 
> Right.
> 
>> I'm actually surprised that almost all of them don't.  I suppose
>> that just depends on how the ABI is set up to pass parameters to
>> the user _start...
> 
> Yes, on hppa the ABI was such that I didn't depend on the layout,
> but s390 does, thus I think it is non-trivial.
> 
> The argv might be trivial to copy, but the size of envp is unknown,
> and _start from the application side in start.S expects argv and
> envp to be right up against eachother AFAICT. Worse it wants to
> scan past them in order to find auxv. The saving grace is that
> the auxv scan parses an arbitrary number of zeros between envp
> and auxv. So I can move the gap to be between envp and auxv.
> 
>> Fortunately, s390 has a block copy instruction, so the move
>> should be trivial to implement.
> 
> For argv only. What instruction is the block copy? Are you
> talking about lm/stm?
> 
> The most naive fix I have working is as follows (I've handed
> off to Siddhesh to have a look since I'm out of time this
> evening/morning).
> 
> diff --git a/sysdeps/s390/s390-32/dl-machine.h b/sysdeps/s390/s390-32/dl-machine.h
> index c56185c..b189552 100644
> --- a/sysdeps/s390/s390-32/dl-machine.h
> +++ b/sysdeps/s390/s390-32/dl-machine.h
> @@ -166,18 +166,47 @@ _dl_start_user:\n\
>  	# See if we were run as a command with the executable file\n\
>  	# name as an extra leading argument.\n\
>  	l     %r1,_dl_skip_args@GOT12(0,%r12)\n\
> -	l     %r1,0(%r1)          # load _dl_skip_args\n\
> +	l     %r1,0(%r1)	# load _dl_skip_args\n\
> +	ltr   %r1,%r1\n\
> +	je    .L4		# Skip the arg adjustment if there were none.\n\
>  	# Get the original argument count.\n\
>  	l     %r0,96(%r15)\n\
>  	# Subtract _dl_skip_args from it.\n\
>  	sr    %r0,%r1\n\
> -	# Adjust the stack pointer to skip _dl_skip_args words.\n\
> -	sll   %r1,2\n\
> -	ar    %r15,%r1\n\
> -	# Set the back chain to zero again\n\
> -	xc    0(4,%r15),0(%r15)\n\
>  	# Store back the modified argument count.\n\
>  	st    %r0,96(%r15)\n\
> +	# Copy argv and envp forward to account for skipped argv entries.\n\
> +	# We skipped at least one argument or we would not get here.\n\
> +	la    %r6,100(%r15)	# Destination pointer i.e. &argv[0]\n\
> +	lr    %r5,%r6\n\
> +	lr    %r0,%r1\n\
> +	sll   %r0,2\n\		# Number of skipped bytes.\n\
> +	ar    %r5,%r0		# Source pointer = Dest + Skipped args.\n\
> +	# argv copy loop:\n\
> +.L1:	l     %r7,0(%r5)	# Load a word from the source.\n\
> +	st    %r7,0(%r6)	# Store the word in the destination.\n\
> +	ahi   %r5,4\n\
> +	ahi   %r6,4\n\
> +	ltr   %r7,%r7\n\
> +	jne   .L1		# Stop after copying the NULL.\n\
> +	# envp copy loop:\n\
> +.L2:	l     %r7,0(%r5)	# Load a word from the source.\n\
> +	st    %r7,0(%r6)	# Store the word in the destination.\n\
> +	ahi   %r5,4\n\
> +	ahi   %r6,4\n\
> +	ltr   %r7,%r7\n\
> +	jne   .L2		# Stop after copying the NULL.\n\
> +	# Now we have to zero out the envp entries after NULL to allow\n\
> +	# start.S to properly find auxv by skipping zeroes.\n\
> +	# zero out loop:\n\
> +	lhi   %r7,0\n\
> +.L3:	st    %r7,0(%r6)	# Store zero.\n\
> +	ahi   %r6,4		# Advance dest pointer.\n\
> +	ahi   %r1,-1		# Subtract one from the word count.\n\
> +	ltr   %r1,%r1\n\
> +	jne    .L3		# Keep copying if the word count is non-zero.\n\
> +	# Set the back chain to zero again\n\
> +.L4:	xc    0(4,%r15),0(%r15)\n\
>  	# The special initializer gets called with the stack just\n\
>  	# as the application's entry point will see it; it can\n\
>  	# switch stacks if it moves these contents over.\n\

The patch is ok to apply with the xc removed. Thanks!

A minor improvement might be to make use of an index reg in the loops. Perhaps something like this
(untested):

 +	# argv copy loop:\n\
 +	lhi   %r8,0
 +.L1:	l     %r7,0(%r8,%r5)	# Load a word from the source.\n\
 +	st    %r7,0(%r8,%r6)	# Store the word in the destination.\n\
 +	ahi   %r8,4\n\
 +	ltr   %r7,%r7\n\
 +	jne   .L1		# Stop after copying the NULL.\n\
 +	# envp copy loop:\n\
 +.L2:	l     %r7,0(%r8,%r5)	# Load a word from the source.\n\
 +	st    %r7,0(%r8,%r6)	# Store the word in the destination.\n\
 +	ahi   %r8,4\n\
 +	ltr   %r7,%r7\n\
 +	jne   .L2		# Stop after copying the NULL.\n\
 +	# Now we have to zero out the envp entries after NULL to allow\n\
 +	# start.S to properly find auxv by skipping zeroes.\n\
 +	# zero out loop:\n\
 +	lhi   %r7,0\n\
 +.L3:	st    %r7,0(%r8,%r6)	# Store zero.\n\
 +	ahi   %r8,4		# Advance dest pointer.\n\
 +	ahi   %r1,-1		# Subtract one from the word count.\n\
 +	ltr   %r1,%r1\n\
 +	jne    .L3		# Keep copying if the word count is non-zero.\n\
 +	# Set the back chain to zero again\n\
 +.L4:

Bye,

-Andreas-