swsusp.S: meaningfull assembly labels

PATCH Pavel Machek 4 participants 9 messages Jun 25, 2004

Share & tools

Pavel Machek

author

2004-06-25 12:00 UTC #0 lore raw

Hi!

This introduces meaningfull labels instead of .L1234, meaning code is readable, kills alignment where unneccessary, and kills TLB flush that was only pure paranoia (and slows it down a lot on emulated systems). Please apply,

Pavel

arch/i386/power/swsusp.S renamed from /arch/i386/power/swsusp.S +12 ~~-15~~

@@ -18,7 +18,7 @@
 ENTRY(do_magic)
 	pushl %ebx
 	cmpl $0,8(%esp)
-	jne .L1450
+	jne resume
 	call do_magic_suspend_1
 	call save_processor_state

@@ -33,21 +33,21 @@
 	pushfl ; popl saved_context_eflags
 
 	call do_magic_suspend_2
-	jmp .L1449
-	.p2align 4,,7
-.L1450:
+	popl %ebx
+	ret
+
+resume:
 	movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
 	movl %ecx,%cr3
 
 	call do_magic_resume_1
 	movl $0,loop
 	cmpl $0,nr_copy_pages
-	je .L1453
-	.p2align 4,,7
-.L1455:
+	je copy_done
+copy_loop:
 	movl $0,loop2
 	.p2align 4,,7
-.L1459:
+copy_one_page:
 	movl pagedir_nosave,%ecx
 	movl loop,%eax
 	movl loop2,%edx

@@ -56,23 +56,21 @@
 	movl (%ecx,%eax),%eax
 	movb (%edx,%eax),%al
 	movb %al,(%edx,%ebx)
-	movl %cr3, %eax;              
-	movl %eax, %cr3;  # flush TLB 
 
 	movl loop2,%eax
 	leal 1(%eax),%edx
 	movl %edx,loop2
 	movl %edx,%eax
 	cmpl $4095,%eax
-	jbe .L1459
+	jbe copy_one_page
 	movl loop,%eax
 	leal 1(%eax),%edx
 	movl %edx,loop
 	movl %edx,%eax
 	cmpl nr_copy_pages,%eax
-	jb .L1455
-	.p2align 4,,7
-.L1453:
+	jb copy_loop
+
+copy_done:
 	movl $__USER_DS,%eax
 
 	movw %ax, %ds

@@ -88,7 +86,6 @@
 	call restore_processor_state
 	pushl saved_context_eflags ; popfl
 	call do_magic_resume_2
-.L1449:
 	popl %ebx
 	ret

Signature · 2 lines

People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Richard B. Johnson

2004-06-25 12:38 UTC #1 lore raw

Quoted 78 lines · Pavel Machek

Hi!

Pavel

arch/i386/power/swsusp.S renamed from /arch/i386/power/swsusp.S +12 ~~-14~~

@@ -18,7 +18,7 @@
 ENTRY(do_magic)
 	pushl %ebx
 	cmpl $0,8(%esp)
-	jne .L1450
+	jne resume
 	call do_magic_suspend_1
 	call save_processor_state

@@ -33,21 +33,21 @@
 	pushfl ; popl saved_context_eflags
 
 	call do_magic_suspend_2
-	jmp .L1449
-	.p2align 4,,7
-.L1450:
+	popl %ebx
+	ret
+
+resume:
 	movl $swsusp_pg_dir-__PAGE_OFFSET,%ecx
 	movl %ecx,%cr3
 
 	call do_magic_resume_1
 	movl $0,loop
 	cmpl $0,nr_copy_pages
-	je .L1453
-	.p2align 4,,7
-.L1455:
+	je copy_done
+copy_loop:
 	movl $0,loop2
 	.p2align 4,,7
-.L1459:
+copy_one_page:
 	movl pagedir_nosave,%ecx
 	movl loop,%eax
 	movl loop2,%edx

@@ -56,23 +56,21 @@
 	movl (%ecx,%eax),%eax
 	movb (%edx,%eax),%al
 	movb %al,(%edx,%ebx)
-	movl %cr3, %eax;
-	movl %eax, %cr3;  # flush TLB
 
 	movl loop2,%eax
 	leal 1(%eax),%edx
 	movl %edx,loop2
 	movl %edx,%eax
 	cmpl $4095,%eax
-	jbe .L1459
+	jbe copy_one_page
 	movl loop,%eax
 	leal 1(%eax),%edx
 	movl %edx,loop
 	movl %edx,%eax
 	cmpl nr_copy_pages,%eax
-	jb .L1455
-	.p2align 4,,7
-.L1453:
+	jb copy_loop
+
+copy_done:
 	movl $__USER_DS,%eax
 
 	movw %ax, %ds

arch/i386/power/swsusp.S

Quoted 9 lines

arch/i386/power/swsusp.S renamed from /arch/i386/power/swsusp.S +0 -2

@@ -88,7 +86,6 @@
 	call restore_processor_state
 	pushl saved_context_eflags ; popfl
 	call do_magic_resume_2
-.L1449:
 	popl %ebx
 	ret
 
 
--

NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!

Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.

Daniel Jacobowitz

2004-06-25 15:20 UTC #2 lore raw

Quoted 6 lines · Richard B. Johnson

NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!

Er, no. They'll show up in the object file. That doesn't mean they're global; static symbols also show up in the object file.

Signature · 1 line

Daniel Jacobowitz

Richard B. Johnson

2004-06-25 16:03 UTC #3 lore raw

Quoted 8 lines · Daniel Jacobowitz

Quoted 6 lines · Richard B. Johnson
NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!

Quoted 5 lines

Er, no. They'll show up in the object file. That doesn't mean they're global; static symbols also show up in the object file.
--
Daniel Jacobowitz

I got caught on these, thinking that they weren't globals when I made a assembly files that used, not only named labels but also definitions like:

BUF = 0x08
LEN = 0x0c

code: movl BUF(%esp), %ebx

	movl	LEN(%esp), %ecx

This was done in several files. When linked, I got 'duplicate synbol' errors. These symbols, while not 'globals' in the sense that 'C' code can link with them are global definitions that are seen by the linker and cause duplicate symbol errors. I went through the gas documentation, trying to find how to prevent these private symbols from being written to the object file and there is no command-line mechanism. You just have to start every name with .L to keep them local.

When I used labels like 'code' above, even though not declared '.global', the labels also showed up as duplicate symbols when linking the resulting object files.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.

Daniel Jacobowitz

2004-06-25 18:49 UTC #4 lore raw

Quoted 23 lines · Richard B. Johnson

Quoted 8 lines · Daniel Jacobowitz
Quoted 6 lines · Richard B. Johnson
NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!
Quoted 5 lines
Er, no. They'll show up in the object file. That doesn't mean they're global; static symbols also show up in the object file.
--
Daniel Jacobowitz
I got caught on these, thinking that they weren't globals when I made a assembly files that used, not only named labels but also definitions like:
BUF = 0x08
LEN = 0x0c

Quoted 8 lines

code: movl BUF(%esp), %ebx
	movl	LEN(%esp), %ecx
This was done in several files. When linked, I got 'duplicate synbol' errors. These symbols, while not 'globals' in the sense that 'C' code can link with them are global definitions that are seen by the linker and cause duplicate symbol errors. I went through

Those are what gas calls absolute symbols.

Quoted 8 lines

the gas documentation, trying to find how to prevent these private symbols from being written to the object file and there is no command-line mechanism. You just have to start every name with .L to keep them local.
When I used labels like 'code' above, even though not declared '.global', the labels also showed up as duplicate symbols when linking the resulting object files.

Does this only happen in your world? Or were you using a broken non-GNU linker?

drow@nevyn:~% cat a.s
BUF = 0x1
code:

        mov %eax, 0

drow@nevyn:~% as -o a.o a.s
drow@nevyn:~% as -o b.o a.s
drow@nevyn:~% ld -o a a.o b.o
ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074 drow@nevyn:~% nm a.o
00000001 a BUF
00000000 t code
drow@nevyn:~% nm a
00000001 a BUF
00000001 a BUF
08049084 A __bss_start
08049084 A _edata
08049084 A _end
08048074 t code
0804807c t code

LD won't even warn about duplicate absolute symbols unless they have different values.

Signature · 1 line

Daniel Jacobowitz

Richard B. Johnson

2004-06-25 19:30 UTC #5 lore raw

Quoted 40 lines · Daniel Jacobowitz

Quoted 23 lines · Richard B. Johnson
Quoted 8 lines · Daniel Jacobowitz
Quoted 6 lines · Richard B. Johnson
NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!
Quoted 5 lines
Er, no. They'll show up in the object file. That doesn't mean they're global; static symbols also show up in the object file.
--
Daniel Jacobowitz
I got caught on these, thinking that they weren't globals when I made a assembly files that used, not only named labels but also definitions like:
BUF = 0x08
LEN = 0x0c
Quoted 8 lines
code: movl BUF(%esp), %ebx
	movl	LEN(%esp), %ecx
This was done in several files. When linked, I got 'duplicate synbol' errors. These symbols, while not 'globals' in the sense that 'C' code can link with them are global definitions that are seen by the linker and cause duplicate symbol errors. I went through
Those are what gas calls absolute symbols.
Quoted 5 lines
the gas documentation, trying to find how to prevent these private symbols from being written to the object file and there is no command-line mechanism. You just have to start every name with .L to keep them local.

Quoted 7 lines

Quoted 3 lines
When I used labels like 'code' above, even though not declared '.global', the labels also showed up as duplicate symbols when linking the resulting object files.
Does this only happen in your world? Or were you using a broken non-GNU linker?

It is the GNU linker

Quoted 20 lines

drow@nevyn:~% cat a.s
BUF = 0x1
code:
        mov %eax, 0
drow@nevyn:~% as -o a.o a.s
drow@nevyn:~% as -o b.o a.s
drow@nevyn:~% ld -o a a.o b.o
ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074 drow@nevyn:~% nm a.o
00000001 a BUF
00000000 t code
drow@nevyn:~% nm a
00000001 a BUF
00000001 a BUF
08049084 A __bss_start
08049084 A _edata
08049084 A _end
08048074 t code
0804807c t code

Quoted 3 lines

LD won't even warn about duplicate absolute symbols unless they have different values.

But they ARE different values. They will probably always be different values, and they end up with the same name, that ld sees and complains.

Quoted 3 lines

--
Daniel Jacobowitz

Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.

Patrick Mochel

2004-06-25 19:39 UTC #6 lore raw

Quoted 21 lines

Quoted 21 lines
drow@nevyn:~% cat a.s
BUF = 0x1
code:
        mov %eax, 0
drow@nevyn:~% as -o a.o a.s
drow@nevyn:~% as -o b.o a.s
drow@nevyn:~% ld -o a a.o b.o
ld: warning: cannot find entry symbol _start; defaulting to 0000000008048074 drow@nevyn:~% nm a.o
00000001 a BUF
00000000 t code
drow@nevyn:~% nm a
00000001 a BUF
00000001 a BUF
08049084 A __bss_start
08049084 A _edata
08049084 A _end
08048074 t code
0804807c t code

Quoted 6 lines

Quoted 3 lines
LD won't even warn about duplicate absolute symbols unless they have different values.
But they ARE different values. They will probably always be different values, and they end up with the same name, that ld sees and complains.

No, it doesn't complain, that's the point. According to nm(1):

For each symbol, nm shows:

The symbol value, in the radix selected by options
(see below), or hexadecimal by default.

The symbol type. At least the following types are
used; others are, as well, depending on the object
file format. If lowercase, the symbol is local; if
uppercase, the symbol is global (external).

Symbols in a source file that are static are tagged with a 't' as well. Read System.map for more proof of this.

The patch is fine, especially since I think it's based on something I've posted in the past. :) The one thing I'd always done differently, though, was to prefix the labels with '.', since I thought that was what kept it local. But, after I RTFM'd, I find that's not even necessary.

Pat

Pavel Machek

author

2004-06-25 16:14 UTC #7 lore raw

In reply to Richard B. Johnson

Hi!

Quoted 18 lines

Quoted 18 lines
This introduces meaningfull labels instead of .L1234, meaning code is readable, kills alignment where unneccessary, and kills TLB flush that was only pure paranoia (and slows it down a lot on emulated systems). Please apply,
Pavel
arch/i386/power/swsusp.S renamed from /arch/i386/power/swsusp.S +1 -1
@@ -18,7 +18,7 @@
 ENTRY(do_magic)
 	pushl %ebx
 	cmpl $0,8(%esp)
-	jne .L1450
+	jne resume
 	call do_magic_suspend_1
 	call save_processor_state
 

...

Quoted 6 lines

NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!

Are you sure? I thought theare not visible from other moduless unless "ENTRY()" is used. See for example entry.S.
Pavel

Signature · 2 lines

People were complaining that M$ turns users into beta-testers...
...jr ghea gurz vagb qrirybcref, naq gurl frrz gb yvxr vg gung jnl!

Richard B. Johnson

2004-06-25 17:24 UTC #8 lore raw

Quoted 28 lines · Pavel Machek

Hi!
Quoted 18 lines
Quoted 18 lines
This introduces meaningfull labels instead of .L1234, meaning code is readable, kills alignment where unneccessary, and kills TLB flush that was only pure paranoia (and slows it down a lot on emulated systems). Please apply,
Pavel
arch/i386/power/swsusp.S renamed from /arch/i386/power/swsusp.S +1 -1
@@ -18,7 +18,7 @@
 ENTRY(do_magic)
 	pushl %ebx
 	cmpl $0,8(%esp)
-	jne .L1450
+	jne resume
 	call do_magic_suspend_1
 	call save_processor_state
 
...
Quoted 6 lines
NO! You just made those labels public! The LOCAL symbols need to begin with ".L". Now, if you have a 'copy_loop' in another module, linked with this, anywhere in the kernel, they will share the same address -- not what you expected, I'm sure! The assembler has some strange rules you need to understand. Use nm and you will find that your new labels are in the object file!

Quoted 3 lines

Are you sure? I thought theare not visible from other moduless unless "ENTRY()" is used. See for example entry.S.
Pavel

Well if you find a way to make those symbols disappear from the object files without using ".L", please let me know. I spent some considerable time beautifying multiple assembly-language files on a recent project, only to find that it would fail to link because of the "multiple symbol definition" errors. I had to change everything back.

You can make labels like ".Lresume:" remain visible only in the file. They don't need to be numbers. It may even be possible to use labels like ".L_resume:", I haven't tried. Basically, if they show up using nm, they can (read will) give you problems.

Cheers,
Dick Johnson
Penguin : Linux version 2.4.26 on an i686 machine (5570.56 BogoMips).
Note 96.31% of all statistics are fiction.