[PATCH] D41403: [ELF] Fix incorrect physical address on self-referencing AT command

[Rafael Avila de Espindola via llvm-commits]

I am not a fan of having to compute alignTo(Dot, Sec->Alignment) twice.

What do you think of the attached patch?

-------------- next part --------------
A non-text attachment was scrubbed...
Name: t.diff
Type: text/x-patch
Size: 3563 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180102/135597a4/attachment.bin>
-------------- next part --------------

Thanks,
Rafael

Erick Reyes via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> erickreyes created this revision.
> erickreyes added a reviewer: lld.
> erickreyes added a project: lld.
> Herald added subscribers: llvm-commits, emaste.
>
> When a section placement (AT) command references the section itself, the physical address of the section in the ELF header was calculated incorrectly due to alignment happening right after the location pointer's value was captured.
>
>
> Repository:
>   rLLD LLVM Linker
>
> https://reviews.llvm.org/D41403
>
> Files:
>   ELF/LinkerScript.cpp
>   test/ELF/linkerscript/at-self-reference.s
>
>
> Index: test/ELF/linkerscript/at-self-reference.s
> ===================================================================
> --- test/ELF/linkerscript/at-self-reference.s
> +++ test/ELF/linkerscript/at-self-reference.s
> @@ -0,0 +1,63 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: echo "SECTIONS { \
> +# RUN:  . = 0x1000; \
> +# RUN:  .aaa : AT(ADDR(.aaa)) { *(.aaa) } \
> +# RUN:  .bbb : AT(ADDR(.bbb)) { *(.bbb) } \
> +# RUN: }" > %t.script
> +# RUN: ld.lld %t --script %t.script -o %t2
> +# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
> +
> +# CHECK:      ProgramHeaders [
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_LOAD (0x1)
> +# CHECK-NEXT:    Offset: 0x1000
> +# CHECK-NEXT:    VirtualAddress: 0x1000
> +# CHECK-NEXT:    PhysicalAddress: 0x1000
> +# CHECK-NEXT:    FileSize: 3
> +# CHECK-NEXT:    MemSize: 3
> +# CHECK-NEXT:    Flags [ (0x5)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_X (0x1)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 4096
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_LOAD (0x1)
> +# CHECK-NEXT:    Offset: 0x1008
> +# CHECK-NEXT:    VirtualAddress: 0x1008
> +# CHECK-NEXT:    PhysicalAddress: 0x1008
> +# CHECK-NEXT:    FileSize: 9
> +# CHECK-NEXT:    MemSize: 9
> +# CHECK-NEXT:    Flags [ (0x5)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_X (0x1)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 4096
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_GNU_STACK (0x6474E551)
> +# CHECK-NEXT:    Offset: 0x0
> +# CHECK-NEXT:    VirtualAddress: 0x0
> +# CHECK-NEXT:    PhysicalAddress: 0x0
> +# CHECK-NEXT:    FileSize: 0
> +# CHECK-NEXT:    MemSize: 0
> +# CHECK-NEXT:    Flags [ (0x6)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_W (0x2)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 0
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:]
> +
> +.global _start
> +_start:
> + nop
> +
> +
> +.section .aaa, "a"
> +.asciz "aa"
> +
> +.section .bbb, "a"
> +.align 8
> +.quad 0
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -663,7 +663,7 @@
>      Dot = Ctx->MemRegionOffset[Ctx->MemRegion];
>  
>    if (Sec->LMAExpr) {
> -    uint64_t D = Dot;
> +    uint64_t D = alignTo(Dot, Sec->Alignment);
>      Ctx->LMAOffset = [=] { return Sec->LMAExpr().getValue() - D; };
>    }
>  
>
>
> Index: test/ELF/linkerscript/at-self-reference.s
> ===================================================================
> --- test/ELF/linkerscript/at-self-reference.s
> +++ test/ELF/linkerscript/at-self-reference.s
> @@ -0,0 +1,63 @@
> +# REQUIRES: x86
> +# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
> +# RUN: echo "SECTIONS { \
> +# RUN:  . = 0x1000; \
> +# RUN:  .aaa : AT(ADDR(.aaa)) { *(.aaa) } \
> +# RUN:  .bbb : AT(ADDR(.bbb)) { *(.bbb) } \
> +# RUN: }" > %t.script
> +# RUN: ld.lld %t --script %t.script -o %t2
> +# RUN: llvm-readobj -program-headers %t2 | FileCheck %s
> +
> +# CHECK:      ProgramHeaders [
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_LOAD (0x1)
> +# CHECK-NEXT:    Offset: 0x1000
> +# CHECK-NEXT:    VirtualAddress: 0x1000
> +# CHECK-NEXT:    PhysicalAddress: 0x1000
> +# CHECK-NEXT:    FileSize: 3
> +# CHECK-NEXT:    MemSize: 3
> +# CHECK-NEXT:    Flags [ (0x5)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_X (0x1)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 4096
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_LOAD (0x1)
> +# CHECK-NEXT:    Offset: 0x1008
> +# CHECK-NEXT:    VirtualAddress: 0x1008
> +# CHECK-NEXT:    PhysicalAddress: 0x1008
> +# CHECK-NEXT:    FileSize: 9
> +# CHECK-NEXT:    MemSize: 9
> +# CHECK-NEXT:    Flags [ (0x5)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_X (0x1)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 4096
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:  ProgramHeader {
> +# CHECK-NEXT:    Type: PT_GNU_STACK (0x6474E551)
> +# CHECK-NEXT:    Offset: 0x0
> +# CHECK-NEXT:    VirtualAddress: 0x0
> +# CHECK-NEXT:    PhysicalAddress: 0x0
> +# CHECK-NEXT:    FileSize: 0
> +# CHECK-NEXT:    MemSize: 0
> +# CHECK-NEXT:    Flags [ (0x6)
> +# CHECK-NEXT:      PF_R (0x4)
> +# CHECK-NEXT:      PF_W (0x2)
> +# CHECK-NEXT:    ]
> +# CHECK-NEXT:    Alignment: 0
> +# CHECK-NEXT:  }
> +# CHECK-NEXT:]
> +
> +.global _start
> +_start:
> + nop
> +
> +
> +.section .aaa, "a"
> +.asciz "aa"
> +
> +.section .bbb, "a"
> +.align 8
> +.quad 0
> Index: ELF/LinkerScript.cpp
> ===================================================================
> --- ELF/LinkerScript.cpp
> +++ ELF/LinkerScript.cpp
> @@ -663,7 +663,7 @@
>      Dot = Ctx->MemRegionOffset[Ctx->MemRegion];
>  
>    if (Sec->LMAExpr) {
> -    uint64_t D = Dot;
> +    uint64_t D = alignTo(Dot, Sec->Alignment);
>      Ctx->LMAOffset = [=] { return Sec->LMAExpr().getValue() - D; };
>    }
>  
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits