linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: Treat R_386_PLT32 as R_386_PC32
@ 2021-01-07  0:17 Fangrui Song
  2021-01-07  1:31 ` Nick Desaulniers
  2021-01-07  2:31 ` Nathan Chancellor
  0 siblings, 2 replies; 12+ messages in thread
From: Fangrui Song @ 2021-01-07  0:17 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: linux-kernel, clang-built-linux, Nick Desaulniers, Fangrui Song,
	Arnd Bergmann

This is similar to commit b21ebf2fb4cde1618915a97cc773e287ff49173e "x86:
Treat R_X86_64_PLT32 as R_X86_64_PC32", but for i386.  As far as Linux
kernel is concerned, R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
requirement that the symbol address is significant.
R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
address significance requirement.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
PLT, but R_386_PLT32 is arguably preferable for -fno-pic code as well:
this can avoid a "canonical PLT entry" (st_shndx=0, st_value!=0) if the
symbol turns out to be defined externally. Latest Clang (since
961f31d8ad14c66829991522d73e14b5a96ff6d4) can use R_386_PLT32 for
compiler produced symbols (if we drop -ffreestanding for CONFIG_X86_32,
library call optimization can produce newer declarations) and future GCC
may use R_386_PLT32 as well if the maintainers agree to adopt an option
like -fdirect-access-external-data to avoid "canonical PLT entry"/copy
relocations https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
---
 arch/x86/kernel/module.c | 1 +
 arch/x86/tools/relocs.c  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..5e9a34b5bd74 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 			*location += sym->st_value;
 			break;
 		case R_386_PC32:
+		case R_386_PLT32:
 			/* Add the value, subtract its position */
 			*location += sym->st_value - (uint32_t)location;
 			break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..717e48ca28b6 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
@@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
-- 
2.29.2.729.g45daf8777d-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-07  0:17 [PATCH] x86: Treat R_386_PLT32 as R_386_PC32 Fangrui Song
@ 2021-01-07  1:31 ` Nick Desaulniers
  2021-01-07  2:31 ` Nathan Chancellor
  1 sibling, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2021-01-07  1:31 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	LKML, clang-built-linux, Arnd Bergmann, Arvind Sankar

On Wed, Jan 6, 2021 at 4:17 PM Fangrui Song <maskray@google.com> wrote:
>
> This is similar to commit b21ebf2fb4cde1618915a97cc773e287ff49173e "x86:
> Treat R_X86_64_PLT32 as R_X86_64_PC32", but for i386.  As far as Linux

nit: the format for referring to in tree sha's:

commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as R_X86_64_PC32")

ie. `commit <first 12 chars of sha> ("<oneline from commit message>")

> kernel is concerned, R_386_PLT32 can be treated the same as R_386_PC32.
>
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
> requirement that the symbol address is significant.
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
> address significance requirement.
>
> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
>
> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
> PLT, but R_386_PLT32 is arguably preferable for -fno-pic code as well:
> this can avoid a "canonical PLT entry" (st_shndx=0, st_value!=0) if the
> symbol turns out to be defined externally. Latest Clang (since
> 961f31d8ad14c66829991522d73e14b5a96ff6d4) can use R_386_PLT32 for

Is https://reviews.llvm.org/rG37f0c8df47d84ba311fc9a2c1884935ba8961e84
related? If so, that should be linked; it would be good to say
"clang-12" rather than "Latest Clang" since in some time "Latest
Clang" will lose meaning.

> compiler produced symbols (if we drop -ffreestanding for CONFIG_X86_32,
> library call optimization can produce newer declarations) and future GCC
> may use R_386_PLT32 as well if the maintainers agree to adopt an option
> like -fdirect-access-external-data to avoid "canonical PLT entry"/copy
> relocations https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112

Punctuation for end of sentence.

>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>

This fixes a build failure for me with clang-12 (ie. top of tree),
thanks for the patch.

Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>

I also see R_386_PC32 referenced in scripts/mod/modpost.c and wonder
if we'd need to potentially handle R_386_PLT32 relocation types there
as well? No current build failures, so maybe YAGNI.

> ---
>  arch/x86/kernel/module.c | 1 +
>  arch/x86/tools/relocs.c  | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>                         *location += sym->st_value;
>                         break;
>                 case R_386_PC32:
> +               case R_386_PLT32:
>                         /* Add the value, subtract its position */
>                         *location += sym->st_value - (uint32_t)location;
>                         break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..717e48ca28b6 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
>                  * NONE can be ignored and PC relative relocations don't
>                  * need to be adjusted.
> @@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
>                  * NONE can be ignored and PC relative relocations don't
>                  * need to be adjusted.
> --
> 2.29.2.729.g45daf8777d-goog
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-07  0:17 [PATCH] x86: Treat R_386_PLT32 as R_386_PC32 Fangrui Song
  2021-01-07  1:31 ` Nick Desaulniers
@ 2021-01-07  2:31 ` Nathan Chancellor
  2021-01-07 18:55   ` [PATCH v2] " Fangrui Song
  1 sibling, 1 reply; 12+ messages in thread
From: Nathan Chancellor @ 2021-01-07  2:31 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, linux-kernel,
	clang-built-linux, Nick Desaulniers, Arnd Bergmann

On Wed, Jan 06, 2021 at 04:17:39PM -0800, Fangrui Song wrote:
> This is similar to commit b21ebf2fb4cde1618915a97cc773e287ff49173e "x86:
> Treat R_X86_64_PLT32 as R_X86_64_PC32", but for i386.  As far as Linux
> kernel is concerned, R_386_PLT32 can be treated the same as R_386_PC32.
> 
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
> requirement that the symbol address is significant.
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
> address significance requirement.
> 
> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
> 
> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
> PLT, but R_386_PLT32 is arguably preferable for -fno-pic code as well:
> this can avoid a "canonical PLT entry" (st_shndx=0, st_value!=0) if the
> symbol turns out to be defined externally. Latest Clang (since
> 961f31d8ad14c66829991522d73e14b5a96ff6d4) can use R_386_PLT32 for
> compiler produced symbols (if we drop -ffreestanding for CONFIG_X86_32,
> library call optimization can produce newer declarations) and future GCC
> may use R_386_PLT32 as well if the maintainers agree to adopt an option
> like -fdirect-access-external-data to avoid "canonical PLT entry"/copy
> relocations https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112
> 
> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>

I agree with Nick's comments about the commit message. With those
addressed:

Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

> ---
>  arch/x86/kernel/module.c | 1 +
>  arch/x86/tools/relocs.c  | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>  			*location += sym->st_value;
>  			break;
>  		case R_386_PC32:
> +		case R_386_PLT32:
>  			/* Add the value, subtract its position */
>  			*location += sym->st_value - (uint32_t)location;
>  			break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..717e48ca28b6 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>  	case R_386_PC32:
>  	case R_386_PC16:
>  	case R_386_PC8:
> +	case R_386_PLT32:
>  		/*
>  		 * NONE can be ignored and PC relative relocations don't
>  		 * need to be adjusted.
> @@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>  	case R_386_PC32:
>  	case R_386_PC16:
>  	case R_386_PC8:
> +	case R_386_PLT32:
>  		/*
>  		 * NONE can be ignored and PC relative relocations don't
>  		 * need to be adjusted.
> -- 
> 2.29.2.729.g45daf8777d-goog
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v2] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-07  2:31 ` Nathan Chancellor
@ 2021-01-07 18:55   ` Fangrui Song
  2021-01-14 22:48     ` [PATCH v3] " Fangrui Song
  0 siblings, 1 reply; 12+ messages in thread
From: Fangrui Song @ 2021-01-07 18:55 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: linux-kernel, clang-built-linux, Fangrui Song, Arnd Bergmann,
	Nick Desaulniers, Nathan Chancellor

This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
requirement that the symbol address is significant.
R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
address significance requirement.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
PLT, but R_386_PLT32 is arguably preferable for -fno-pic code as well:
this can avoid a "canonical PLT entry" (st_shndx=0, st_value!=0) if the
symbol turns out to be defined externally.

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/961f31d8ad14c66829991522d73e14b5a96ff6d4)
can emit R_386_PLT32 for compiler produced symbols (if we drop
-ffreestanding for CONFIG_X86_32, library call optimization can produce
newer declarations) and future GCC -fno-pic may emit R_386_PLT32 as well
if an option like -fno-direct-access-external-data is adopted to avoid
canonical PLT entry/copy relocations.

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Link: https://github.com/llvm/llvm-project/commit/961f31d8ad14c66829991522d73e14b5a96ff6d4
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=98112
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

---
Change in v2:
* Improve commit message
---
 arch/x86/kernel/module.c | 1 +
 arch/x86/tools/relocs.c  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..5e9a34b5bd74 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 			*location += sym->st_value;
 			break;
 		case R_386_PC32:
+		case R_386_PLT32:
 			/* Add the value, subtract its position */
 			*location += sym->st_value - (uint32_t)location;
 			break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..717e48ca28b6 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
@@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
-- 
2.29.2.729.g45daf8777d-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH v3] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-07 18:55   ` [PATCH v2] " Fangrui Song
@ 2021-01-14 22:48     ` Fangrui Song
  2021-01-22  8:49       ` Fāng-ruì Sòng
  2021-01-25 14:23       ` Borislav Petkov
  0 siblings, 2 replies; 12+ messages in thread
From: Fangrui Song @ 2021-01-14 22:48 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: linux-kernel, clang-built-linux, Fangrui Song, Arnd Bergmann,
	Nick Desaulniers, Nathan Chancellor

This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
requirement that the symbol address is significant.
R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
address significance requirement.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
PLT.

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations as
well to avoid a canonical PLT entry (st_shndx=0, st_value!=0) if the
symbol turns out to be defined externally. GCC/GNU as will likely keep
using R_386_PC32 because (1) the ABI is legacy (2) the change will drop
a GNU ld non-default visibility ifunc for shared objects.
https://sourceware.org/bugzilla/show_bug.cgi?id=27169

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

---
Change in v2:
* Improve commit message
---
Change in v3:
* Change the GCC link to the more relevant GNU as link.
* Fix the relevant llvm-project commit id.
---
 arch/x86/kernel/module.c | 1 +
 arch/x86/tools/relocs.c  | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..5e9a34b5bd74 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 			*location += sym->st_value;
 			break;
 		case R_386_PC32:
+		case R_386_PLT32:
 			/* Add the value, subtract its position */
 			*location += sym->st_value - (uint32_t)location;
 			break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..717e48ca28b6 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
@@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
 		 * NONE can be ignored and PC relative relocations don't
 		 * need to be adjusted.
-- 
2.30.0.296.g2bfb1c46d8-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-14 22:48     ` [PATCH v3] " Fangrui Song
@ 2021-01-22  8:49       ` Fāng-ruì Sòng
  2021-01-25 14:23       ` Borislav Petkov
  1 sibling, 0 replies; 12+ messages in thread
From: Fāng-ruì Sòng @ 2021-01-22  8:49 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, X86 ML
  Cc: LKML, clang-built-linux, Arnd Bergmann, Nick Desaulniers,
	Nathan Chancellor

On Thu, Jan 14, 2021 at 2:48 PM Fangrui Song <maskray@google.com> wrote:
>
> This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
> R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
> R_386_PLT32 can be treated the same as R_386_PC32.
>
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
> requirement that the symbol address is significant.
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
> address significance requirement.
>
> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
>
> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
> PLT.
>
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
> can emit R_386_PLT32 for compiler generated function declarations as
> well to avoid a canonical PLT entry (st_shndx=0, st_value!=0) if the
> symbol turns out to be defined externally. GCC/GNU as will likely keep
> using R_386_PC32 because (1) the ABI is legacy (2) the change will drop
> a GNU ld non-default visibility ifunc for shared objects.
> https://sourceware.org/bugzilla/show_bug.cgi?id=27169
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> ---
> Change in v2:
> * Improve commit message
> ---
> Change in v3:
> * Change the GCC link to the more relevant GNU as link.
> * Fix the relevant llvm-project commit id.
> ---
>  arch/x86/kernel/module.c | 1 +
>  arch/x86/tools/relocs.c  | 2 ++
>  2 files changed, 3 insertions(+)
>
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>                         *location += sym->st_value;
>                         break;
>                 case R_386_PC32:
> +               case R_386_PLT32:
>                         /* Add the value, subtract its position */
>                         *location += sym->st_value - (uint32_t)location;
>                         break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..717e48ca28b6 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
>                  * NONE can be ignored and PC relative relocations don't
>                  * need to be adjusted.
> @@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
>                  * NONE can be ignored and PC relative relocations don't
>                  * need to be adjusted.
> --
> 2.30.0.296.g2bfb1c46d8-goog
>

Ping:)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-14 22:48     ` [PATCH v3] " Fangrui Song
  2021-01-22  8:49       ` Fāng-ruì Sòng
@ 2021-01-25 14:23       ` Borislav Petkov
  2021-01-25 17:29         ` Fangrui Song
  1 sibling, 1 reply; 12+ messages in thread
From: Borislav Petkov @ 2021-01-25 14:23 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	clang-built-linux, Arnd Bergmann, Nick Desaulniers,
	Nathan Chancellor, Michael Matz

It's a good thing I have a toolchain guy who can explain to me what you
guys are doing because you need to start writing those commit messages
for !toolchain developers.

On Thu, Jan 14, 2021 at 02:48:19PM -0800, Fangrui Song wrote:
> This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
> R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
> R_386_PLT32 can be treated the same as R_386_PC32.
> 
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
> requirement that the symbol address is significant.
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
> address significance requirement.

I was told what "significant" means in that context and while it is
clear to you, I'm pretty sure it is not clear to kernel developers who
haven't looked at toolchains in depth. So please elaborate.

> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.

Also, please explain in short why LLVM is generating R_X86_64_PLT32
relocs now? I.e., is it the same reason as why binutils does that?

I.e., mentioning the big picture of things would help as to why you're
doing this.

> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
> PLT.

Convention in general or convention for LLVM?

> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
> can emit R_386_PLT32 for compiler generated function declarations as
> well to avoid a canonical PLT entry (st_shndx=0, st_value!=0) if the
> symbol turns out to be defined externally. GCC/GNU as will likely keep
> using R_386_PC32 because (1) the ABI is legacy (2) the change will drop
> a GNU ld non-default visibility ifunc for shared objects.
> https://sourceware.org/bugzilla/show_bug.cgi?id=27169

Not sure how useful this paragraph is for kernel developers...

> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
> 
> ---
> Change in v2:
> * Improve commit message
> ---
> Change in v3:
> * Change the GCC link to the more relevant GNU as link.
> * Fix the relevant llvm-project commit id.
> ---
>  arch/x86/kernel/module.c | 1 +
>  arch/x86/tools/relocs.c  | 2 ++
>  2 files changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>  			*location += sym->st_value;
>  			break;
>  		case R_386_PC32:
> +		case R_386_PLT32:
>  			/* Add the value, subtract its position */
>  			*location += sym->st_value - (uint32_t)location;
>  			break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..717e48ca28b6 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>  	case R_386_PC32:
>  	case R_386_PC16:
>  	case R_386_PC8:
> +	case R_386_PLT32:
>  		/*
>  		 * NONE can be ignored and PC relative relocations don't
>  		 * need to be adjusted.

That comment might need adjustment.

> @@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>  	case R_386_PC32:
>  	case R_386_PC16:
>  	case R_386_PC8:
> +	case R_386_PLT32:
>  		/*
>  		 * NONE can be ignored and PC relative relocations don't
>  		 * need to be adjusted.

Ditto.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v3] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-25 14:23       ` Borislav Petkov
@ 2021-01-25 17:29         ` Fangrui Song
  2021-01-27 20:56           ` [PATCH v4] " Fangrui Song
  0 siblings, 1 reply; 12+ messages in thread
From: Fangrui Song @ 2021-01-25 17:29 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar, x86, linux-kernel,
	clang-built-linux, Arnd Bergmann, Nick Desaulniers,
	Nathan Chancellor, Michael Matz


On 2021-01-25, Borislav Petkov wrote:
>It's a good thing I have a toolchain guy who can explain to me what you
>guys are doing because you need to start writing those commit messages
>for !toolchain developers.

How about this following message? I'll answer your questions in line as
well. Explaining everything in the message will be quite long...  If
someone is interested, I have put every possibly related matter in
https://maskray.me/blog/2021-01-09-copy-relocations-canonical-plt-entries-and-protected


This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which can
only be used by branches. If the referenced symbol is defined
externally, a PLT will be used.
R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
used by address taking operations and branches. If the referenced symbol
is defined externally, a copy relocation/canonical PLT entry will be
created in the executable.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
This avoids copy relocations/canonical PLT entries.

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries are
possible ABI issues but GCC/GNU as will likely keep the status quo
because (1) the ABI is legacy (2) the change will drop a GNU ld
diagnostic for non-default visibility ifunc in shared objects.
https://sourceware.org/bugzilla/show_bug.cgi?id=27169

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations,
because preventing canonical PLT entries is weighed over the rare ifunc
diagnostic.

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>


>On Thu, Jan 14, 2021 at 02:48:19PM -0800, Fangrui Song wrote:
>> This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
>> R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
>> R_386_PLT32 can be treated the same as R_386_PC32.
>>
>> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types with the
>> requirement that the symbol address is significant.
>> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types without the
>> address significance requirement.
>
>I was told what "significant" means in that context and while it is
>clear to you, I'm pretty sure it is not clear to kernel developers who
>haven't looked at toolchains in depth. So please elaborate.

Expanded "significant" to more words. See above.

>> On x86-64, there is no PIC vs non-PIC PLT distinction and an
>> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
>> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
>
>Also, please explain in short why LLVM is generating R_X86_64_PLT32
>relocs now? I.e., is it the same reason as why binutils does that?
>
>I.e., mentioning the big picture of things would help as to why you're
>doing this.

It has been explained. The LLVM change was in 2018, roughly the same
time when GNU as emitted R_X86_64_PLT32. I think it does not need
extended explanation because of the separate canonical PLT entries
paragraph.

>> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
>> convention is to use R_386_PC32 for non-PIC PLT and R_386_PLT32 for PIC
>> PLT.
>
>Convention in general or convention for LLVM?

Changed to "GCC/GNU as convention".

>> clang-12 -fno-pic (since
>> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
>> can emit R_386_PLT32 for compiler generated function declarations as
>> well to avoid a canonical PLT entry (st_shndx=0, st_value!=0) if the
>> symbol turns out to be defined externally. GCC/GNU as will likely keep
>> using R_386_PC32 because (1) the ABI is legacy (2) the change will drop
>> a GNU ld non-default visibility ifunc for shared objects.
>> https://sourceware.org/bugzilla/show_bug.cgi?id=27169
>
>Not sure how useful this paragraph is for kernel developers...

Reorganize it a bit...

>> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
>> Reported-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Fangrui Song <maskray@google.com>
>> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
>> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
>> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>>
>> ---
>> Change in v2:
>> * Improve commit message
>> ---
>> Change in v3:
>> * Change the GCC link to the more relevant GNU as link.
>> * Fix the relevant llvm-project commit id.
>> ---
>>  arch/x86/kernel/module.c | 1 +
>>  arch/x86/tools/relocs.c  | 2 ++
>>  2 files changed, 3 insertions(+)
>>
>> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
>> index 34b153cbd4ac..5e9a34b5bd74 100644
>> --- a/arch/x86/kernel/module.c
>> +++ b/arch/x86/kernel/module.c
>> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>>  			*location += sym->st_value;
>>  			break;
>>  		case R_386_PC32:
>> +		case R_386_PLT32:
>>  			/* Add the value, subtract its position */
>>  			*location += sym->st_value - (uint32_t)location;
>>  			break;
>> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
>> index ce7188cbdae5..717e48ca28b6 100644
>> --- a/arch/x86/tools/relocs.c
>> +++ b/arch/x86/tools/relocs.c
>> @@ -867,6 +867,7 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>>  	case R_386_PC32:
>>  	case R_386_PC16:
>>  	case R_386_PC8:
>> +	case R_386_PLT32:
>>  		/*
>>  		 * NONE can be ignored and PC relative relocations don't
>>  		 * need to be adjusted.
>
>That comment might need adjustment.
>
>> @@ -910,6 +911,7 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>>  	case R_386_PC32:
>>  	case R_386_PC16:
>>  	case R_386_PC8:
>> +	case R_386_PLT32:
>>  		/*
>>  		 * NONE can be ignored and PC relative relocations don't
>>  		 * need to be adjusted.
>
>Ditto.
>
>-- 
>Regards/Gruss,
>    Boris.
>
>https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH v4] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-25 17:29         ` Fangrui Song
@ 2021-01-27 20:56           ` Fangrui Song
  2021-01-27 22:37             ` Nick Desaulniers
                               ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Fangrui Song @ 2021-01-27 20:56 UTC (permalink / raw)
  To: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86
  Cc: linux-kernel, clang-built-linux, Michael Matz, Fangrui Song,
	Arnd Bergmann, Nick Desaulniers, Nathan Chancellor

This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
R_386_PLT32 can be treated the same as R_386_PC32.

R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which can
only be used by branches. If the referenced symbol is defined
externally, a PLT will be used.
R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
used by address taking operations and branches. If the referenced symbol
is defined externally, a copy relocation/canonical PLT entry will be
created in the executable.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
This avoids canonical PLT entries (st_shndx=0, st_value!=0).

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries are
possible ABI issues but GCC/GNU as will likely keep the status quo
because (1) the ABI is legacy (2) the change will drop a GNU ld
diagnostic for non-default visibility ifunc in shared objects.
https://sourceware.org/bugzilla/show_bug.cgi?id=27169

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations,
because preventing canonical PLT entries is weighed over the rare ifunc
diagnostic.

Link: https://github.com/ClangBuiltLinux/linux/issues/1210
Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>

---
Change in v2:
* Improve commit message
---
Change in v3:
* Change the GCC link to the more relevant GNU as link.
* Fix the relevant llvm-project commit.
---
Change in v4:
* Improve comments and commit message
---
 arch/x86/kernel/module.c |  1 +
 arch/x86/tools/relocs.c  | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153cbd4ac..5e9a34b5bd74 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 			*location += sym->st_value;
 			break;
 		case R_386_PC32:
+		case R_386_PLT32:
 			/* Add the value, subtract its position */
 			*location += sym->st_value - (uint32_t)location;
 			break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..1c3a1962cade 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,9 +867,11 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
-		 * NONE can be ignored and PC relative relocations don't
-		 * need to be adjusted.
+		 * NONE can be ignored and PC relative relocations don't need
+		 * to be adjusted. Because sym must be defined, R_386_PLT32 can
+		 * be treated the same way as R_386_PC32.
 		 */
 		break;
 
@@ -910,9 +912,11 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
-		 * NONE can be ignored and PC relative relocations don't
-		 * need to be adjusted.
+		 * NONE can be ignored and PC relative relocations don't need
+		 * to be adjusted. Because sym must be defined, R_386_PLT32 can
+		 * be treated the same way as R_386_PC32.
 		 */
 		break;
 
-- 
2.30.0.280.ga3ce27912f-goog


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-27 20:56           ` [PATCH v4] " Fangrui Song
@ 2021-01-27 22:37             ` Nick Desaulniers
  2021-01-28  1:10             ` Sedat Dilek
  2021-01-28 11:59             ` [tip: x86/build] x86/build: Treat R_386_PLT32 relocation " tip-bot2 for Fangrui Song
  2 siblings, 0 replies; 12+ messages in thread
From: Nick Desaulniers @ 2021-01-27 22:37 UTC (permalink / raw)
  To: Fangrui Song, Borislav Petkov
  Cc: Thomas Gleixner, Ingo Molnar,
	maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT),
	LKML, clang-built-linux, Michael Matz, Arnd Bergmann,
	Nathan Chancellor

On Wed, Jan 27, 2021 at 12:56 PM Fangrui Song <maskray@google.com> wrote:
>
> This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
> R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
> R_386_PLT32 can be treated the same as R_386_PC32.
>
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which can
> only be used by branches. If the referenced symbol is defined
> externally, a PLT will be used.
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
> used by address taking operations and branches. If the referenced symbol
> is defined externally, a copy relocation/canonical PLT entry will be
> created in the executable.
>
> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
> This avoids canonical PLT entries (st_shndx=0, st_value!=0).
>
> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
> R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries are
> possible ABI issues but GCC/GNU as will likely keep the status quo
> because (1) the ABI is legacy (2) the change will drop a GNU ld
> diagnostic for non-default visibility ifunc in shared objects.
> https://sourceware.org/bugzilla/show_bug.cgi?id=27169
>
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
> can emit R_386_PLT32 for compiler generated function declarations,
> because preventing canonical PLT entries is weighed over the rare ifunc
> diagnostic.

Boris, my CI is red since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6
landed (Dec 5) for i386.  If you need a shorter (or less toolchain
verbiage) commit message, please consider simply:

```
This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
R_X86_64_PC32"), but for i386.  From that commit message:
  As far as the Linux kernel is concerned, R_386_PLT32 can be
  treated the same as R_386_PC32.

clang-12 -fno-pic (since
https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
can emit R_386_PLT32 for compiler generated function declarations.
```

It would help me abuse <strikethrough>alcohol</strikethrough>coffee
less to have one less fire.

>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>
> ---
> Change in v2:
> * Improve commit message
> ---
> Change in v3:
> * Change the GCC link to the more relevant GNU as link.
> * Fix the relevant llvm-project commit.
> ---
> Change in v4:
> * Improve comments and commit message
> ---
>  arch/x86/kernel/module.c |  1 +
>  arch/x86/tools/relocs.c  | 12 ++++++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>                         *location += sym->st_value;
>                         break;
>                 case R_386_PC32:
> +               case R_386_PLT32:
>                         /* Add the value, subtract its position */
>                         *location += sym->st_value - (uint32_t)location;
>                         break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..1c3a1962cade 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,9 +867,11 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
> -                * NONE can be ignored and PC relative relocations don't
> -                * need to be adjusted.
> +                * NONE can be ignored and PC relative relocations don't need
> +                * to be adjusted. Because sym must be defined, R_386_PLT32 can
> +                * be treated the same way as R_386_PC32.
>                  */
>                 break;
>
> @@ -910,9 +912,11 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
> -                * NONE can be ignored and PC relative relocations don't
> -                * need to be adjusted.
> +                * NONE can be ignored and PC relative relocations don't need
> +                * to be adjusted. Because sym must be defined, R_386_PLT32 can
> +                * be treated the same way as R_386_PC32.
>                  */
>                 break;
>
> --
> 2.30.0.280.ga3ce27912f-goog
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH v4] x86: Treat R_386_PLT32 as R_386_PC32
  2021-01-27 20:56           ` [PATCH v4] " Fangrui Song
  2021-01-27 22:37             ` Nick Desaulniers
@ 2021-01-28  1:10             ` Sedat Dilek
  2021-01-28 11:59             ` [tip: x86/build] x86/build: Treat R_386_PLT32 relocation " tip-bot2 for Fangrui Song
  2 siblings, 0 replies; 12+ messages in thread
From: Sedat Dilek @ 2021-01-28  1:10 UTC (permalink / raw)
  To: Fangrui Song
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, x86, linux-kernel,
	Clang-Built-Linux ML, Michael Matz, Arnd Bergmann,
	Nick Desaulniers, Nathan Chancellor

On Wed, Jan 27, 2021 at 9:56 PM 'Fangrui Song' via Clang Built Linux
<clang-built-linux@googlegroups.com> wrote:
>
> This is similar to commit b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as
> R_X86_64_PC32"), but for i386.  As far as Linux kernel is concerned,
> R_386_PLT32 can be treated the same as R_386_PC32.
>
> R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which can
> only be used by branches. If the referenced symbol is defined
> externally, a PLT will be used.
> R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
> used by address taking operations and branches. If the referenced symbol
> is defined externally, a copy relocation/canonical PLT entry will be
> created in the executable.
>
> On x86-64, there is no PIC vs non-PIC PLT distinction and an
> R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
> `call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
> This avoids canonical PLT entries (st_shndx=0, st_value!=0).
>
> On i386, there are 2 types of PLTs, PIC and non-PIC. Currently the
> GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
> R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries are
> possible ABI issues but GCC/GNU as will likely keep the status quo
> because (1) the ABI is legacy (2) the change will drop a GNU ld
> diagnostic for non-default visibility ifunc in shared objects.
> https://sourceware.org/bugzilla/show_bug.cgi?id=27169
>
> clang-12 -fno-pic (since
> https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6)
> can emit R_386_PLT32 for compiler generated function declarations,
> because preventing canonical PLT entries is weighed over the rare ifunc
> diagnostic.
>
> Link: https://github.com/ClangBuiltLinux/linux/issues/1210
> Reported-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Fangrui Song <maskray@google.com>
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
> Tested-by: Nick Desaulniers <ndesaulniers@google.com>
> Tested-by: Nathan Chancellor <natechancellor@gmail.com>
>

Tested-by: Sedat Dilek <sedat.dilek@gmail.com> # v3

- Sedat -

> ---
> Change in v2:
> * Improve commit message
> ---
> Change in v3:
> * Change the GCC link to the more relevant GNU as link.
> * Fix the relevant llvm-project commit.
> ---
> Change in v4:
> * Improve comments and commit message
> ---
>  arch/x86/kernel/module.c |  1 +
>  arch/x86/tools/relocs.c  | 12 ++++++++----
>  2 files changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
> index 34b153cbd4ac..5e9a34b5bd74 100644
> --- a/arch/x86/kernel/module.c
> +++ b/arch/x86/kernel/module.c
> @@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
>                         *location += sym->st_value;
>                         break;
>                 case R_386_PC32:
> +               case R_386_PLT32:
>                         /* Add the value, subtract its position */
>                         *location += sym->st_value - (uint32_t)location;
>                         break;
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..1c3a1962cade 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -867,9 +867,11 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
> -                * NONE can be ignored and PC relative relocations don't
> -                * need to be adjusted.
> +                * NONE can be ignored and PC relative relocations don't need
> +                * to be adjusted. Because sym must be defined, R_386_PLT32 can
> +                * be treated the same way as R_386_PC32.
>                  */
>                 break;
>
> @@ -910,9 +912,11 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
>         case R_386_PC32:
>         case R_386_PC16:
>         case R_386_PC8:
> +       case R_386_PLT32:
>                 /*
> -                * NONE can be ignored and PC relative relocations don't
> -                * need to be adjusted.
> +                * NONE can be ignored and PC relative relocations don't need
> +                * to be adjusted. Because sym must be defined, R_386_PLT32 can
> +                * be treated the same way as R_386_PC32.
>                  */
>                 break;
>
> --
> 2.30.0.280.ga3ce27912f-goog
>
> --
> You received this message because you are subscribed to the Google Groups "Clang Built Linux" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to clang-built-linux+unsubscribe@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/clang-built-linux/20210127205600.1227437-1-maskray%40google.com.

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [tip: x86/build] x86/build: Treat R_386_PLT32 relocation as R_386_PC32
  2021-01-27 20:56           ` [PATCH v4] " Fangrui Song
  2021-01-27 22:37             ` Nick Desaulniers
  2021-01-28  1:10             ` Sedat Dilek
@ 2021-01-28 11:59             ` tip-bot2 for Fangrui Song
  2 siblings, 0 replies; 12+ messages in thread
From: tip-bot2 for Fangrui Song @ 2021-01-28 11:59 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Arnd Bergmann, Fangrui Song, Borislav Petkov, Nick Desaulniers,
	Nathan Chancellor, Sedat Dilek, x86, linux-kernel

The following commit has been merged into the x86/build branch of tip:

Commit-ID:     bb73d07148c405c293e576b40af37737faf23a6a
Gitweb:        https://git.kernel.org/tip/bb73d07148c405c293e576b40af37737faf23a6a
Author:        Fangrui Song <maskray@google.com>
AuthorDate:    Wed, 27 Jan 2021 12:56:00 -08:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Thu, 28 Jan 2021 12:24:06 +01:00

x86/build: Treat R_386_PLT32 relocation as R_386_PC32

This is similar to commit

  b21ebf2fb4cd ("x86: Treat R_X86_64_PLT32 as R_X86_64_PC32")

but for i386. As far as the kernel is concerned, R_386_PLT32 can be
treated the same as R_386_PC32.

R_386_PLT32/R_X86_64_PLT32 are PC-relative relocation types which
can only be used by branches. If the referenced symbol is defined
externally, a PLT will be used.

R_386_PC32/R_X86_64_PC32 are PC-relative relocation types which can be
used by address taking operations and branches. If the referenced symbol
is defined externally, a copy relocation/canonical PLT entry will be
created in the executable.

On x86-64, there is no PIC vs non-PIC PLT distinction and an
R_X86_64_PLT32 relocation is produced for both `call/jmp foo` and
`call/jmp foo@PLT` with newer (2018) GNU as/LLVM integrated assembler.
This avoids canonical PLT entries (st_shndx=0, st_value!=0).

On i386, there are 2 types of PLTs, PIC and non-PIC. Currently,
the GCC/GNU as convention is to use R_386_PC32 for non-PIC PLT and
R_386_PLT32 for PIC PLT. Copy relocations/canonical PLT entries
are possible ABI issues but GCC/GNU as will likely keep the status
quo because (1) the ABI is legacy (2) the change will drop a GNU
ld diagnostic for non-default visibility ifunc in shared objects.

clang-12 -fno-pic (since [1]) can emit R_386_PLT32 for compiler
generated function declarations, because preventing canonical PLT
entries is weighed over the rare ifunc diagnostic.

Further info for the more interested:

  https://github.com/ClangBuiltLinux/linux/issues/1210
  https://sourceware.org/bugzilla/show_bug.cgi?id=27169
  https://github.com/llvm/llvm-project/commit/a084c0388e2a59b9556f2de0083333232da3f1d6 [1]

 [ bp: Massage commit message. ]

Reported-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Fangrui Song <maskray@google.com>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
Reviewed-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Nick Desaulniers <ndesaulniers@google.com>
Tested-by: Nathan Chancellor <natechancellor@gmail.com>
Tested-by: Sedat Dilek <sedat.dilek@gmail.com>
Link: https://lkml.kernel.org/r/20210127205600.1227437-1-maskray@google.com
---
 arch/x86/kernel/module.c |  1 +
 arch/x86/tools/relocs.c  | 12 ++++++++----
 2 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index 34b153c..5e9a34b 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -114,6 +114,7 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 			*location += sym->st_value;
 			break;
 		case R_386_PC32:
+		case R_386_PLT32:
 			/* Add the value, subtract its position */
 			*location += sym->st_value - (uint32_t)location;
 			break;
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188c..1c3a196 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -867,9 +867,11 @@ static int do_reloc32(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
-		 * NONE can be ignored and PC relative relocations don't
-		 * need to be adjusted.
+		 * NONE can be ignored and PC relative relocations don't need
+		 * to be adjusted. Because sym must be defined, R_386_PLT32 can
+		 * be treated the same way as R_386_PC32.
 		 */
 		break;
 
@@ -910,9 +912,11 @@ static int do_reloc_real(struct section *sec, Elf_Rel *rel, Elf_Sym *sym,
 	case R_386_PC32:
 	case R_386_PC16:
 	case R_386_PC8:
+	case R_386_PLT32:
 		/*
-		 * NONE can be ignored and PC relative relocations don't
-		 * need to be adjusted.
+		 * NONE can be ignored and PC relative relocations don't need
+		 * to be adjusted. Because sym must be defined, R_386_PLT32 can
+		 * be treated the same way as R_386_PC32.
 		 */
 		break;
 

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2021-01-28 12:00 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-07  0:17 [PATCH] x86: Treat R_386_PLT32 as R_386_PC32 Fangrui Song
2021-01-07  1:31 ` Nick Desaulniers
2021-01-07  2:31 ` Nathan Chancellor
2021-01-07 18:55   ` [PATCH v2] " Fangrui Song
2021-01-14 22:48     ` [PATCH v3] " Fangrui Song
2021-01-22  8:49       ` Fāng-ruì Sòng
2021-01-25 14:23       ` Borislav Petkov
2021-01-25 17:29         ` Fangrui Song
2021-01-27 20:56           ` [PATCH v4] " Fangrui Song
2021-01-27 22:37             ` Nick Desaulniers
2021-01-28  1:10             ` Sedat Dilek
2021-01-28 11:59             ` [tip: x86/build] x86/build: Treat R_386_PLT32 relocation " tip-bot2 for Fangrui Song

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).