Misaligned stack on 32-bit s390?

Message ID m3vv46$63c$1@ger.gmane.org
State Committed
Headers

Commit Message

Stefan Liebler Nov. 12, 2014, 3:41 p.m. UTC
  Hi,

i have built glibc from scratch with the patch and get the following 
error while compiling rtld.c:
/tmp/cc3Ata9V.s: Assembler messages:
/tmp/cc3Ata9V.s:111: Error: symbol `.L3' is already defined
/tmp/cc3Ata9V.s:124: Error: symbol `.L4' is already defined

The compiler generated these labels in function 
_dl_initial_error_catch_tsd in order to get the address of the variable 
data via literal pool and got. A compiler with --with-arch=z9-109 uses 
larl-instruction to get the address and does not generate these labels.

Using numbered labels in the inline-assembly in macro RTLD_START (see 
patch) avoids the label collision and the glibc build succeeds
There is no test-suite regression and the stack is adjusted.
The inline-assembly in s390-64/dl-machine.h does not use any label,
thus there we need no change.

Thanks.
Stefan


---
2014-11-12  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* sysdeps/s390/s390-32/dl-machine.h (RTLD_START):
	Use numbered labels in inline assembly.
  

Comments

Siddhesh Poyarekar Nov. 13, 2014, 5:19 a.m. UTC | #1
On Wed, Nov 12, 2014 at 04:41:58PM +0100, Stefan Liebler wrote:
> Hi,
> 
> i have built glibc from scratch with the patch and get the following error
> while compiling rtld.c:
> /tmp/cc3Ata9V.s: Assembler messages:
> /tmp/cc3Ata9V.s:111: Error: symbol `.L3' is already defined
> /tmp/cc3Ata9V.s:124: Error: symbol `.L4' is already defined
> 
> The compiler generated these labels in function _dl_initial_error_catch_tsd
> in order to get the address of the variable data via literal pool and got. A
> compiler with --with-arch=z9-109 uses larl-instruction to get the address
> and does not generate these labels.
> 
> Using numbered labels in the inline-assembly in macro RTLD_START (see patch)
> avoids the label collision and the glibc build succeeds
> There is no test-suite regression and the stack is adjusted.
> The inline-assembly in s390-64/dl-machine.h does not use any label,
> thus there we need no change.
> 
> Thanks.
> Stefan
> 
> 
> ---
> 2014-11-12  Stefan Liebler  <stli@linux.vnet.ibm.com>
> 
> 	* sysdeps/s390/s390-32/dl-machine.h (RTLD_START):
> 	Use numbered labels in inline assembly.

Thanks, this looks good to me.  Technically this should wait for
machine maintainer approval, but IMO it would result in identical code
and if you can somehow prove that, you'd be good to go.

Siddhesh
  
Andreas Krebbel Nov. 13, 2014, 9:46 a.m. UTC | #2
On 11/12/2014 04:41 PM, Stefan Liebler wrote:
> 2014-11-12  Stefan Liebler  <stli@linux.vnet.ibm.com>
> 
> 	* sysdeps/s390/s390-32/dl-machine.h (RTLD_START):
> 	Use numbered labels in inline assembly.
> 

Applied. Thanks!
  

Patch

diff --git a/sysdeps/s390/s390-32/dl-machine.h b/sysdeps/s390/s390-32/dl-machine.h
index 47f5874..6780405 100644
--- a/sysdeps/s390/s390-32/dl-machine.h
+++ b/sysdeps/s390/s390-32/dl-machine.h
@@ -148,7 +148,7 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
 .globl _dl_start_user\n\
 _start:\n\
 	basr  %r13,0\n\
-.L0:    ahi   %r13,.Llit-.L0\n\
+0:      ahi   %r13,.Llit-0b\n\
 	lr    %r2,%r15\n\
 	# Alloc stack frame\n\
 	ahi   %r15,-96\n\
@@ -168,7 +168,7 @@  _dl_start_user:\n\
 	l     %r1,_dl_skip_args@GOT(%r12)\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\
+	je    4f		# 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\
@@ -183,28 +183,28 @@  _dl_start_user:\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\
+1:	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\
+	jne   1b		# Stop after copying the NULL.\n\
 	# envp copy loop:\n\
-.L2:	l     %r7,0(%r5)	# Load a word from the source.\n\
+2:	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\
+	jne   2b		# 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\
+3:	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\
+	jne    3b		# Keep copying if the word count is non-zero.\n\
 	# Adjust _dl_argv\n\
 	la    %r6,100(%r15)\n\
 	l     %r1,_dl_argv@GOT(%r12)\n\
@@ -216,7 +216,7 @@  _dl_start_user:\n\
 	# Call the function to run the initializers.\n\
 	# Load the parameters:\n\
 	# (%r2, %r3, %r4, %r5) = (_dl_loaded, argc, argv, envp)\n\
-.L4:	l     %r2,_rtld_local@GOT(%r12)\n\
+4:	l     %r2,_rtld_local@GOT(%r12)\n\
 	l     %r2,0(%r2)\n\
 	l     %r3,96(%r15)\n\
 	la    %r4,100(%r15)\n\