llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.12k stars 12.01k forks source link

LLD: in-file position computation goes wrong and end up with huge value. #65159

Open UmeshKalappa0 opened 1 year ago

UmeshKalappa0 commented 1 year ago

Hi ,

the testcase has the issue https://godbolt.org/z/j5baEPWfc like


- $ cat test.ld 
- ENTRY(_start)
- SECTIONS
- {
-   .text (VMA_START) : AT(VMA_START + LMA_START)
-    {
-     *(.text)
-    }
-   . += LMA_START;
-   _TEXTCORE_LOAD_ADDR = .;
-   .text.cpu :
-   AT(_TEXTCORE_LOAD_ADDR)
-   {
-     *(.text_cpu)
-   }
-   . = . -LMA_START;
-   .sbss :
-   {
-     *(.sbss)
-   }
-   PROVIDE (wrs_kernel_bss_start = ADDR(.sbss)) ;
- }
$llvm-mc -filetype=obj  -triple=x86_64 test.s -o test.o
$ld.lld -m elf_x86_64 --no-relax -X -N --eh-frame-hdr     -e _start --defsym ENTRY_SYMBOL=_start  --defsym VMA_START=0xffffffff90008000 --defsym LMA_START=0x0 --defsym LMA_START=0x70000000   -Ttext 00008000 -defsym DATA_ALIGN=1 test.o -T ./test.ld

**ld.lld: error: output file too large: 18446744071830504616 bytes**
section sizes:
.text 13
.text.cpu 0
.sbss 0
.data 4
.comment 19
.symtab 192
.shstrtab 64
.strtab 77

and we investigate the issue and found that the below code at

ELF/Writer.cpp
@@ -2519,8 +2519,10 @@ static uint64_t computeFileOffset(OutputSection *os, uint64_t off) {

   // If two sections share the same PT_LOAD the file offset is calculated
   // using this formula: Off2 = Off1 + (VA2 - VA1).
 OutputSection *first = os->ptLoad->firstSec;
 return first->offset + os->addr - first->addr;
and 
>>this formula: Off2 = Off1 + (VA2 - VA1).  can end up with huge value like stated in the above error  for VA1 > VA2.

So we recommend the changes like

diff --git a/ELF/Writer.cpp b/ELF/Writer.cpp
index 6fbee17..d76184d 100644
--- a/ELF/Writer.cpp
+++ b/ELF/Writer.cpp
@@ -2519,8 +2519,10 @@ static uint64_t computeFileOffset(OutputSection *os, uint64_t off) {

   // If two sections share the same PT_LOAD the file offset is calculated
   // using this formula: Off2 = Off1 + (VA2 - VA1).
-  OutputSection *first = os->ptLoad->firstSec;
-  return first->offset + os->addr - first->addr;
+    OutputSection *first = os->ptLoad->firstSec;
+    if (os->addr > first->addr)
+      return first->offset + os->addr - first->addr;
+    return off;
 }
llvmbot commented 1 year ago

@llvm/issue-subscribers-lld-elf

smithp35 commented 1 year ago

Can you use the lld --reproduce= to make a full reproducer. The existing example has constants that are not defined like VMA_START. It may be possible to reproduce by fiddling with numbers but would certainly save time with them provided.

Will need to work out what has gone wrong before jumping to a fix. It may be that assigning the output sections to the same PT_LOAD was the wrong thing to do as the location counter can't go backwards within a single PT_LOAD.

UmeshKalappa0 commented 1 year ago

@smithp35

Can you use the lld --reproduce= to make a full reproducer. The existing example has constants that are not defined like VMA_START. test.zip

the command define these constants like $ld.lld -m elf_x86_64 --no-relax -X -N --eh-frame-hdr -e _start --defsym ENTRY_SYMBOL=_start --defsym VMA_START=0xffffffff90008000 --defsym LMA_START=0x0 --defsym LMA_START=0x70000000 -Ttext 00008000 -defsym DATA_ALIGN=1 test.o -T ./test.ld

It may be that assigning the output sections to the same PT_LOAD was the wrong thing to do as the location counter can't go backwards within a single PT_LOAD.

it possible sharing same PT_LOAD may be wrong here .

Thank you @smithp35 for the quick thought and attached test.zip like asked. [ Testcase : test.zip ]

UmeshKalappa0 commented 1 year ago

@smithp35 and @MaskRay ,further investigating issue and consider the below case like

llvm-mc -filetype=obj -triple=x86_64 test.s -o test.o

test.s

.text .globl main .type main, @function main: ret

.globl main1 .section .text.cpu,"ax",@progbits .type main1, @function main1: ret

.globl main2 .type main2, @function main2: ret

linker command used

ld.lld -m elf_x86_64 --no-relax -X -N --eh-frame-hdr -e main --defsym ENTRY_SYMBOL=main --defsym VMA_START=0xffffffff90008000 --defsym LMA_START=0x0 --defsym LMA_START=0x70000000 -Ttext 00008000 -defsym DATA_ALIGN=1 test.o -T ./test.ld

$objdump -S a.out

0000000000008000 : 8000: c3 retq

Disassembly of section .text.cpu:

0000000070008001 main1: 70008001: c3 retq

0000000070008002 main2: 70008002: c3 retq

$ readelf -l a.out

Program Headers: Type Offset VirtAddr PhysAddr FileSiz MemSiz Flags Align LOAD 0x00000000000000e8 0x0000000000008000 0x0000000000008000 0x0000000000000001 0x0000000000000001 RWE 0x4 LOAD 0x00000000000000e9 0x0000000070008001 0x0000000070008001 0x0000000000000002 0x0000000000000002 RWE 0x1 GNU_STACK 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 0x0000000000000000 RW 0x0

main2 and main1 share the same PT_LOAD header and its same semantics like gnu ld ..

so like mentioned

So we recommend the changes like

make sense and would like know your suggestions or comments on the sharing the same PT_LOAD segment in this case?

Testcase : updateTest.zip

smithp35 commented 1 year ago

I've only had a brief time to play around with the example. I think part of the problem might be that in

ENTRY(_start)
SECTIONS
{
  .text (VMA_START) : AT(VMA_START + LMA_START)
   {
    *(.text)
   }
  . += LMA_START;
  _TEXTCORE_LOAD_ADDR = .;
  .text.cpu :
  AT(_TEXTCORE_LOAD_ADDR)
  {
    *(.text_cpu)
  }
  . = . -LMA_START;
  .sbss :
  {
    *(.sbss)
  }
  PROVIDE (wrs_kernel_bss_start = ADDR(.sbss)) ;
}

The .sbss and .text_cpu are empty which may mean that LLD doesn't create a new program header. If, like I think you have done with your second example, added at least one input section in .text_cpu or .sbss then the 2nd program header is generated.

It is possible that GNU ld is smarter about not counting empty output sections. If I try my example with .text_cpu and .sbss both GNU ld and LLD generate two program headers.

One observation for your example is that the -ttext is completely overwriting the (VMA_START) : AT(VMA_START + LMA_START) and the LMA_START = 0x0 is overridden by LMA_START = 0x70000000

Essentially what I think we have is:

ENTRY(_start)
SECTIONS
{
  .text 0x8000
   {
    *(.text)
   }
  . += 0x70000000;
  _TEXTCORE_LOAD_ADDR = .;
  .text.cpu :
  AT(_TEXTCORE_LOAD_ADDR)
  {
    *(.text_cpu)
  }
  . = . -LMA_START;
  .sbss :
  {
    *(.sbss)
  }
  PROVIDE (wrs_kernel_bss_start = ADDR(.sbss)) ;
}

We'll need to look through the code to work out what the best fix is. It would definitely be an error if within the same program segment VA goes backwards, and the linker had to write content. However if the section is 0 sized so there is nothing there to write it may be OK to keep the same file offset. Would probably want a fatal error message for a non-0 sized section.

Am a bit busy with the day job at the moment, so may take some time to get to this though.

andcarminati commented 10 months ago

Hi, just sharing some interesting point. I did a fast investigation, and I think that the problem starts when we create PT_LOAD for the osections without isections. In this case I tried the following change (without any error):

diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a84e4864ab0e..da1a31b89aef 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -2416,7 +2416,7 @@ SmallVector<PhdrEntry *, 0> Writer<ELFT>::createPhdrs(Partition &part) {
   relRo->p_align = 1;

   for (OutputSection *sec : outputSections) {
-    if (!needsPtLoad(sec))
+    if (!needsPtLoad(sec) || !sec->hasInputSections)
       continue;

     // Normally, sections in partitions other than the current partition are

And I got:

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x0000b0 0x0000000000008000 0x0000000000008000 0x000014 0x000014 RWE 0x10
  GNU_STACK      0x000000 0x0000000000000000 0x0000000000000000 0x000000 0x000000 RW  0x0

 Section to Segment mapping:
  Segment Sections...
   00     .text .sbss .data 
   01     
   None   .text.cpu .comment .symtab .shstrtab .strtab 

There are no relocations in this file.

Symbol table '.symtab' contains 8 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND 
     1: 0000000000008000    13 FUNC    GLOBAL DEFAULT     1 _start
     2: 0000000000008010     4 OBJECT  GLOBAL DEFAULT     4 data
     3: ffffffff90008000     0 NOTYPE  GLOBAL DEFAULT   ABS VMA_START
     4: 0000000070000000     0 NOTYPE  GLOBAL DEFAULT   ABS LMA_START
     5: 000000007000800d     0 NOTYPE  GLOBAL DEFAULT     1 _TEXTCORE_LOAD_ADDR
     6: 0000000000008000     0 FUNC    GLOBAL DEFAULT     1 ENTRY_SYMBOL
     7: 0000000000000001     0 NOTYPE  GLOBAL DEFAULT   ABS DATA_ALIGN

Just as a comparison, with GNU Ld:

Program Headers:
  Type           Offset   VirtAddr           PhysAddr           FileSiz  MemSiz   Flg Align
  LOAD           0x000080 0x0000000000008000 0x0000000000008000 0x000014 0x000014 RWE 0x10

 Section to Segment mapping:
  Segment Sections...
   00     .text .data 
   None   .symtab .strtab .shstrtab 

There are no relocations in this file.

Symbol table '.symtab' contains 8 entries:
   Num:    Value          Size Type    Bind   Vis       Ndx Name
     0: 0000000000000000     0 NOTYPE  LOCAL  DEFAULT   UND 
     1: 0000000070000000     0 NOTYPE  GLOBAL DEFAULT   ABS LMA_START
     2: 0000000000008000     0 FUNC    GLOBAL DEFAULT     1 ENTRY_SYMBOL
     3: 0000000070008014     0 NOTYPE  GLOBAL DEFAULT     2 _TEXTCORE_LOAD_ADDR
     4: 0000000000000001     0 NOTYPE  GLOBAL DEFAULT   ABS DATA_ALIGN
     5: 0000000000008000    13 FUNC    GLOBAL DEFAULT     1 _start
     6: 0000000000008010     4 OBJECT  GLOBAL DEFAULT     2 data
     7: ffffffff90008000     0 NOTYPE  GLOBAL DEFAULT   ABS VMA_START

I'm not fully convinced because of the .sbss section, but this can indicate one possible direction for a solution.