[PATCH] D39493: [ELF] Fix DT_MIPS_LOCAL_GOTNO value when using linker scripts to change section sizes

[Rafael Avila de Espindola via llvm-commits]

LGTM.

I had to run dos2unix on the patch, so please be careful not to commit
CRLF lines.

I agree that the check for mips is unavoidable in here.

Thanks an sorry for taking so long to look at it.

Cheers,
Rafael

James Henderson via Phabricator <reviews at reviews.llvm.org> writes:

> jhenderson updated this revision to Diff 122217.
> jhenderson marked an inline comment as done.
> jhenderson added a comment.
>
> Thanks @atanasyan. I've fixed that.
>
>
> https://reviews.llvm.org/D39493
>
> Files:
>   ELF/SyntheticSections.cpp
>   test/ELF/mips-got-script.s
>
>
> Index: test/ELF/mips-got-script.s
> ===================================================================
> --- test/ELF/mips-got-script.s
> +++ test/ELF/mips-got-script.s
> @@ -0,0 +1,47 @@
> +# Check number of got entries is adjusted for linker script-added space.
> +
> +# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
> +# RUN: echo "SECTIONS { .data : { *(.data.1); . += 0x10000; *(.data.2) } }" > %t.script
> +# RUN: ld.lld %t.o -shared -o %t.so -T %t.script
> +# RUN: llvm-readobj -mips-plt-got -dynamic-table %t.so | FileCheck %s
> +
> +# REQUIRES: mips
> +
> +# CHECK: 0x7000000A MIPS_LOCAL_GOTNO 5
> +#                                    ^-- 2 * header + 3 local entries
> +# CHECK:      Local entries [
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32744
> +# CHECK-NEXT:     Initial: 0x0
> +#                          ^-- loc1
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32740
> +# CHECK-NEXT:     Initial: 0x10000
> +#                          ^-- loc2
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32736
> +# CHECK-NEXT:     Initial: 0x20000
> +#                          ^-- redundant
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]
> +
> +  .text
> +  .globl  foo
> +foo:
> +  lw      $t0, %got(loc1)($gp)
> +  addi    $t0, $t0, %lo(loc1)
> +  lw      $t0, %got(loc2)($gp)
> +  addi    $t0, $t0, %lo(loc2)
> +
> +  .section .data.1,"aw",%progbits
> +loc1:
> +  .word 0
> +
> +  .section .data.2,"aw",%progbits
> +loc2:
> +  .word 0
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1127,7 +1127,11 @@
>      add({DT_MIPS_FLAGS, RHF_NOTPOT});
>      add({DT_MIPS_BASE_ADDRESS, Target->getImageBase()});
>      add({DT_MIPS_SYMTABNO, InX::DynSymTab->getNumSymbols()});
> -    add({DT_MIPS_LOCAL_GOTNO, InX::MipsGot->getLocalEntriesNum()});
> +
> +    // The number of local got entries has not yet been finalized. This value
> +    // will be set in writeTo().
> +    add({DT_MIPS_LOCAL_GOTNO, uint64_t(0)});
> +
>      if (const Symbol *B = InX::MipsGot->getFirstGlobalEntry())
>        add({DT_MIPS_GOTSYM, B->DynsymIndex});
>      else
> @@ -1162,7 +1166,10 @@
>        P->d_un.d_ptr = E.Sym->getVA();
>        break;
>      case Entry::PlainInt:
> -      P->d_un.d_val = E.Val;
> +      if (Config->EMachine == EM_MIPS && E.Tag == DT_MIPS_LOCAL_GOTNO)
> +        P->d_un.d_val = InX::MipsGot->getLocalEntriesNum();
> +      else
> +        P->d_un.d_val = E.Val;
>        break;
>      }
>      ++P;
>
>
> Index: test/ELF/mips-got-script.s
> ===================================================================
> --- test/ELF/mips-got-script.s
> +++ test/ELF/mips-got-script.s
> @@ -0,0 +1,47 @@
> +# Check number of got entries is adjusted for linker script-added space.
> +
> +# RUN: llvm-mc -filetype=obj -triple=mips-unknown-linux %s -o %t.o
> +# RUN: echo "SECTIONS { .data : { *(.data.1); . += 0x10000; *(.data.2) } }" > %t.script
> +# RUN: ld.lld %t.o -shared -o %t.so -T %t.script
> +# RUN: llvm-readobj -mips-plt-got -dynamic-table %t.so | FileCheck %s
> +
> +# REQUIRES: mips
> +
> +# CHECK: 0x7000000A MIPS_LOCAL_GOTNO 5
> +#                                    ^-- 2 * header + 3 local entries
> +# CHECK:      Local entries [
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32744
> +# CHECK-NEXT:     Initial: 0x0
> +#                          ^-- loc1
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32740
> +# CHECK-NEXT:     Initial: 0x10000
> +#                          ^-- loc2
> +# CHECK-NEXT:   }
> +# CHECK-NEXT:   Entry {
> +# CHECK-NEXT:     Address:
> +# CHECK-NEXT:     Access: -32736
> +# CHECK-NEXT:     Initial: 0x20000
> +#                          ^-- redundant
> +# CHECK-NEXT:   }
> +# CHECK-NEXT: ]
> +
> +  .text
> +  .globl  foo
> +foo:
> +  lw      $t0, %got(loc1)($gp)
> +  addi    $t0, $t0, %lo(loc1)
> +  lw      $t0, %got(loc2)($gp)
> +  addi    $t0, $t0, %lo(loc2)
> +
> +  .section .data.1,"aw",%progbits
> +loc1:
> +  .word 0
> +
> +  .section .data.2,"aw",%progbits
> +loc2:
> +  .word 0
> Index: ELF/SyntheticSections.cpp
> ===================================================================
> --- ELF/SyntheticSections.cpp
> +++ ELF/SyntheticSections.cpp
> @@ -1127,7 +1127,11 @@
>      add({DT_MIPS_FLAGS, RHF_NOTPOT});
>      add({DT_MIPS_BASE_ADDRESS, Target->getImageBase()});
>      add({DT_MIPS_SYMTABNO, InX::DynSymTab->getNumSymbols()});
> -    add({DT_MIPS_LOCAL_GOTNO, InX::MipsGot->getLocalEntriesNum()});
> +
> +    // The number of local got entries has not yet been finalized. This value
> +    // will be set in writeTo().
> +    add({DT_MIPS_LOCAL_GOTNO, uint64_t(0)});
> +
>      if (const Symbol *B = InX::MipsGot->getFirstGlobalEntry())
>        add({DT_MIPS_GOTSYM, B->DynsymIndex});
>      else
> @@ -1162,7 +1166,10 @@
>        P->d_un.d_ptr = E.Sym->getVA();
>        break;
>      case Entry::PlainInt:
> -      P->d_un.d_val = E.Val;
> +      if (Config->EMachine == EM_MIPS && E.Tag == DT_MIPS_LOCAL_GOTNO)
> +        P->d_un.d_val = InX::MipsGot->getLocalEntriesNum();
> +      else
> +        P->d_un.d_val = E.Val;
>        break;
>      }
>      ++P;