powerpc/boot: Delete unneeded .globl _zimage_start
PATCH v2 1/1 powerpc/boot: Delete unneeded .globl _zimage_start
.globl sets the symbol binding to STB_GLOBAL while .weak sets the binding to STB_WEAK. GNU as let .weak override .globl since binutils-gdb 5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd (1996). Clang integrated assembler let the last win but it may error in the future.
Since it is a convention that only one binding directive is used, just delete .globl.
--- arch/powerpc/boot/crt0.S | 3 --- 1 file changed, 3 deletions(-)
arch/powerpc/boot/crt0.S +0 -3
@@ -44,9 +44,6 @@ p_end: .long _end p_pstack: .long _platform_stack_top #endif - .globl _zimage_start - /* Clang appears to require the .weak directive to be after the symbol - * is defined. See https://bugs.llvm.org/show_bug.cgi?id=38921 */ .weak _zimage_start _zimage_start: .globl _zimage_start_lib
Signature · 1 line
2.25.1.696.g5e7596f4ac-goog
Quoted 4 lines · Fangrui Song
.globl sets the symbol binding to
STB_GLOBALwhile .weak sets the binding toSTB_WEAK. GNU as let .weak override .globl since binutils-gdb5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd(1996). Clang integrated assembler let the last win but it may error in the future.
GNU AS works for more than just ELF. The way the assembler language is defined, it is not .weak overriding .globl -- instead, .weak sets a symbol attribute. On an existing symbol (but it creates on if there is none yet).
Clang is buggy if it does not allow valid (and perfectly normal) assembler code like this.
Segher
Quoted 6 lines · Segher Boessenkool
Quoted 4 lines · Fangrui Song
.globl sets the symbol binding to
STB_GLOBALwhile .weak sets the binding toSTB_WEAK. GNU as let .weak override .globl since binutils-gdb5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd(1996). Clang integrated assembler let the last win but it may error in the future.
Quoted 7 lines
GNU AS works for more than just ELF. The way the assembler language is defined, it is not .weak overriding .globl -- instead, .weak sets a symbol attribute. On an existing symbol (but it creates on if there is none yet).
Clang is buggy if it does not allow valid (and perfectly normal) assembler code like this.
https://sourceware.org/pipermail/binutils/2020-March/110399.html
Alan: "I think it is completely fine for you to make the llvm assembler error on inconsistent binding, or the last directive win. Either of those behaviours is logical and good, but you quite possibly will run into a need to fix more user assembly.
I am doing some experiments whether making clang integrated assembler error is feasible.
Quoted 15 lines · Fangrui Song
Quoted 6 lines · Segher Boessenkool
Quoted 4 lines · Fangrui Song
.globl sets the symbol binding to
STB_GLOBALwhile .weak sets the binding toSTB_WEAK. GNU as let .weak override .globl since binutils-gdb5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd(1996). Clang integrated assembler let the last win but it may error in the future.Quoted 7 lines
GNU AS works for more than just ELF. The way the assembler language is defined, it is not .weak overriding .globl -- instead, .weak sets a symbol attribute. On an existing symbol (but it creates on if there is none yet).
Clang is buggy if it does not allow valid (and perfectly normal) assembler code like this.
Quoted 6 lines
https://sourceware.org/pipermail/binutils/2020-March/110399.html
Alan: "I think it is completely fine for you to make the llvm assembler error on inconsistent binding, or the last directive win. Either of those behaviours is logical and good, but you quite possibly will run into a need to fix more user assembly.
This would be fine and consistent behaviour, of course. But it is not appropriate if you want to pretend to be compatible to GNU toolchains.
Which is exactly why you want this kernel patch at all. And the kernel can (in this case) accommodate your buggy assembler, sure, but are you going to "fix" all other programs with this "problem" as well?
Segher
Quoted 18 lines · Segher Boessenkool
Quoted 17 lines · Fangrui Song
Quoted 6 lines · Segher Boessenkool
Quoted 4 lines · Fangrui Song
.globl sets the symbol binding to
STB_GLOBALwhile .weak sets the binding toSTB_WEAK. GNU as let .weak override .globl since binutils-gdb5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd(1996). Clang integrated assembler let the last win but it may error in the future.Quoted 7 lines
GNU AS works for more than just ELF. The way the assembler language is defined, it is not .weak overriding .globl -- instead, .weak sets a symbol attribute. On an existing symbol (but it creates on if there is none yet).
Clang is buggy if it does not allow valid (and perfectly normal) assembler code like this.
https://sourceware.org/pipermail/binutils/2020-March/110399.html
Quoted 7 lines
Quoted 4 lines
Alan: "I think it is completely fine for you to make the llvm assembler error on inconsistent binding, or the last directive win. Either of those behaviours is logical and good, but you quite possibly will run into a need to fix more user assembly.
This would be fine and consistent behaviour, of course. But it is not appropriate if you want to pretend to be compatible to GNU toolchains.
We aim for compatibility with GNU in many aspects to make it easier for people to switch over. However, just because there is a subtle behavior in GNU toolchain does not mean we need to emulate that behavior. With all due respect, there are a large quantity of legacy behaviors we don't want to support. Quite interestingly, many times such behaviors turn out to be not well tested - they are documented by git blame/log.
Building kernel with another mature toolchain is a good way to shake out code that relies on undefined/subtle behaviors. The efforts improve health of the kernel.
It may be a bit more off-topic now. I am more confident on linker/binary utilities side. Not emulating traditional behaviors turns out to be a great success for lld (LLVM linker). We managed to create a linker with 23+k lines of code which is able to build a majority of software. In FreeBSD ports, 32k pieces of software just work, 130+ packages are marked as LLD_UNSAFE, but many should be safe (need developers' testing) as of lld 9.
Quoted 5 lines
Which is exactly why you want this kernel patch at all. And the kernel can (in this case) accommodate your buggy assembler, sure, but are you going to "fix" all other programs with this "problem" as well?
Segher
For this particularly case, A "blanked write privs" binutils maintainer acknowledged clang integrated assembler's behavior. Another "blanked write privs" (but inactive) binutils maintainer does not feel strong about his decision made 24 years ago. With respect, I should mention that our design decisions do not need their approval. That said, we will be careful with the these decisions because the choices may affect several companies and several larger code bases.
This is why I mentioned in my previous message that I want to experiment. I will try out the error on some large code bases. Nick may be able to help on Android side. Additionally, we may get help from FreeBSD folks.
If you subscribe to [email protected], you may find in recent months I am quite active there. I am a very tiny contributor there but I try to communicate clang/LLVM binary utilities/lld's discrepancy (in terms of traditional behaviors) and report issues to binutils. Hope clang's success can give incentive to improve binutils code health as well.
Cheers,
Fangrui
Quoted 3 lines · Fangrui Song
We aim for compatibility with GNU in many aspects to make it easier for people to switch over. However, just because there is a subtle behavior in GNU toolchain does not mean we need to emulate that behavior.
It isn't subtle. It is the explicit documented behaviour.
Quoted 3 lines
With
all due respect, there are a large quantity of legacy behaviors we don't want to support.
And it isn't legacy at all. It is simply the defined semantics.
It is semantics that real code relies on (and has for ages) as well.
Segher
Michael Ellerman
AppliedQuoted 8 lines · Fangrui Song
.globl sets the symbol binding to
STB_GLOBALwhile .weak sets the binding toSTB_WEAK. GNU as let .weak override .globl since binutils-gdb5ca547dc2399a0a5d9f20626d4bf5547c3ccfddd(1996). Clang integrated assembler let the last win but it may error in the future.Since it is a convention that only one binding directive is used, just delete .globl.
Quoted 10 lines
Fixes cd197ffcf10b "[POWERPC] zImage: Cleanup and improve zImage entry point"Fixes ee9d21b3b358 "powerpc/boot: Ensure _zimage_start is a weak symbol"Signed-off-by Fangrui Song <[email protected]>Cc Alan Modra <[email protected]>Cc Joel Stanley <[email protected]>Cc Michael Ellerman <[email protected]>Cc Nick Desaulniers <[email protected]>Cc Segher Boessenkool <[email protected]>
Applied to powerpc next, thanks.
https://git.kernel.org/powerpc/c/968339fad422a58312f67718691b717dac45c399
cheers