Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

LLD silently misinterprets the LMA in linkerscripts #163

Closed
andrewrk opened this issue Mar 2, 2020 · 7 comments
Closed

LLD silently misinterprets the LMA in linkerscripts #163

andrewrk opened this issue Mar 2, 2020 · 7 comments

Comments

@andrewrk
Copy link
Member

andrewrk commented Mar 2, 2020

Downstream issue: ziglang/zig#4595
Corresponding Bugzilla Issue

Content from downstream issue reproduced here (courtesy of @LemonBoy):

ASM file:

.section ".text","ax"
.global _start

.arm
.align 4
_start:
    b _start

Linker script:

OUTPUT_ARCH(arm)
ENTRY(_start)

PHDRS {
	p0  PT_LOAD FLAGS(7);
	p1  PT_LOAD FLAGS(7);
}

MEMORY {
    m0  : ORIGIN = 0x1000, LENGTH = 0x1000
    m1  : ORIGIN = 0x8000, LENGTH = 0x1000
}

SECTIONS
{
    .s0 :
    {
        *(.s0)
    } >m1 AT>m0 :p0

    .s1 :
    {
        KEEP (*(.text))
        KEEP (*(.data))
        KEEP (*(.bss))
    } >m0 :p1
}

Linked using the following command line: ld.lld-10 -T /tmp/script.ld /tmp/foo.o -o /tmp/foo
Version: 1:10~++20200228052623+4c6e5899859-1~exp1~20200228163220.107 (from the LLVM's APT repo)

The problem lies in how the physical address of the program headers is calculated, the value computed by lld is wildly wrong:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0x00001000 0xffffa000 0x00004 0x00004 RWE 0x1000

On the other hand gnu ld produces the correct result:

  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x000074 0x00000000 0x00000000 0x00000 0x00000 RWE 0x10000
  LOAD           0x001000 0x00001000 0x00001000 0x00004 0x00004 RWE 0x10000

btw thanks for allowing GitHub issues. It's much nicer than bugzilla

@andrewrk
Copy link
Member Author

andrewrk commented Mar 2, 2020

Note, because bugzilla is unresponsive:

<smeenai> andrewrk: I'm trying to add CCs but Bugzilla is being unresponsive :/
<smeenai> But you should CC Rui Ueyama, George Rimar, and Fangrui Song on that when you can

@smeenai
Copy link
Collaborator

smeenai commented Mar 2, 2020

CC @rui314 @GeorgiiR @MaskRay

@smithp35
Copy link
Collaborator

smithp35 commented Mar 2, 2020

I've commented on the PR, my reading from LLVM-DEV was that there wasn't consensus on using github issues yet. My interpretation of the version is that this is on lld-10. I think that this may have been fixed on master with https://reviews.llvm.org/D74297

@LemonBoy
Copy link
Contributor

LemonBoy commented Mar 2, 2020

My interpretation of the version is that this is on lld-10.

Yes, lld 10 built on the 28th of February from commit 4c6e5899859.

I think that this may have been fixed on master with https://reviews.llvm.org/D74297

Interesting, I guess this patch didn't land in time before the LLVM-10 branch was created.

@MaskRay
Copy link
Member

MaskRay commented Mar 2, 2020

Fixed by https://reviews.llvm.org/D74297

Program Headers:  
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0x00001000 0x00001000 0x00004 0x00004 RWE 0x1000
                                 
 Section to Segment mapping:                                                                                                          
  Segment Sections...                                                                                                                 
   00     .s1                

There are still differences: ld.lld does not create the empty PT_LOAD, but it should not matter.

If .s0 is not empty:

# lld
Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x001000 0x00008000 0x00001000 0x00001 0x00001 RWE 0x1000
  LOAD           0x001010 0x00001010 0x00001010 0x00004 0x00004 RWE 0x1000
# GNU ld
Program Headers:
  Type           Offset   VirtAddr   PhysAddr   FileSiz MemSiz  Flg Align
  LOAD           0x008000 0x00008000 0x00001000 0x00001 0x00001 RWE 0x10000
  LOAD           0x011010 0x00001010 0x00001010 0x00004 0x00004 RWE 0x10000

There should not be a different at runtime, but lld's p_offset fields save space.

@MaskRay MaskRay closed this as completed Mar 2, 2020
@LemonBoy
Copy link
Contributor

LemonBoy commented Mar 2, 2020

Fixed by https://reviews.llvm.org/D74297

Can/Will this be backported to LLVM-10?

@zmodem
Copy link
Collaborator

zmodem commented Mar 4, 2020

I've commented on the PR, my reading from LLVM-DEV was that there wasn't consensus on using github issues yet. My interpretation of the version is that this is on lld-10. I think that this may have been fixed on master with https://reviews.llvm.org/D74297

For reference, the PR is https://bugs.llvm.org/show_bug.cgi?id=45078

Fixed by https://reviews.llvm.org/D74297

Can/Will this be backported to LLVM-10?

It's probably too late for 10.0.0, but it may be a candidate for 10.0.1.

am11 pushed a commit to am11/llvm-project that referenced this issue Mar 29, 2022
This caught my attention as I was looking at the ObjWriter. LLVM no longer emits a `LC_VERSION_MIN_MACOSX` load command unless we explicitly set a version. I don't see a difference in `llvm-objdump -macho -x foo.o` with/without these lines (I didn't bother myself to boot into macOS to run `otool`).
trevor-m pushed a commit to trevor-m/llvm-project that referenced this issue Apr 20, 2023
Summary: This fixes issue llvm#163, among other improvements to go-to-definition.

Reviewers: sammccall

Subscribers: jkorous, mgrang, arphaman, kadircet, usaxena95, cfe-commits

Tags: #clang

Differential Revision: https://reviews.llvm.org/D69237
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants