[ELF] - Do not crash when move location counter backward.

[grimar]

Summary

PR31335 shows that we do that in next case:
SECTIONS { .text 0x2000 : {. = 0x100 ; *(.text) } }

though documentations says that "If . is used inside a section description however, it refers to the byte offset from the start of that section, not an absolute address. " looks does not work as documented in bfd (as mentioned in comments for PR31335).
Until we find out the expected behavior was suggested at least not to 'crash', what we do after trying to generate huge file.

[ruiu]

ELF/LinkerScript.cpp

467–469 ↗

(On Diff #81230)

If Dot overflows towards negative, your expression cannot detect the error because Val would be a very large number. Can you handle this case as well?

[grimar]

ELF/LinkerScript.cpp

467–469 ↗

(On Diff #81230)

I'll update the patch tomorrow for that case.

[grimar]

ELF/LinkerScript.cpp

467–469 ↗

(On Diff #81230)

After reviewing this I think:

1. This patch should be landed separatelly as it fix specific common case which is not related to overflows.
2. For case of overflow I suggest to use something like CheckedType from D25279 in expressions for all Expr we have. That makes overflow handling to be general and avoids point fixes.

[ruiu]

LGTM. I'm not convinced that D25279 is worth its complexity, but this is indeed a small good patch.

[grimar]

In D27712#623084, @ruiu wrote:

LGTM. I'm not convinced that D25279 is worth its complexity

May be. Though I think we should be able to replace uint64_t Dot with "some checked type" Dot,
and that can solve all possible overflow issues once with just a this local change.
It looks convinent to have such overflow detector class, may be we can have one in llvm. Not sure here.

[ruiu]

I think I'm not a big fan of int-ish type because even though they look like int and behave like int, they are different from int at a lot of places. That increases complexity in a subtle way. Currently, you can read our code without too much previous knowledge because we do not use magic C++ provides that much. If we need an overflow check, I want something more explicit. For example, I'd rather use uint128_t or something to avoid overflow during computation and check everything later. But we don't have to do that now.

lld/trunk/ELF/LinkerScript.cpp

void LinkerScript<ELFT>::switchTo(OutputSectionBase *Sec) {
   CurOutSec->setLMAOffset(LMAOffset);
 }
 
 template <class ELFT> void LinkerScript<ELFT>::process(BaseCommand &Base) {
   // This handles the assignments to symbol or to a location counter (.)
   if (auto *AssignCmd = dyn_cast<SymbolAssignment>(&Base)) {
     if (AssignCmd->Name == ".") {
       // Update to location counter means update to section size.
-      Dot = AssignCmd->Expression(Dot);
+      uintX_t Val = AssignCmd->Expression(Dot);
+      if (Val < Dot)
+        error("unable to move location counter backward for: " +
+              CurOutSec->Name);
+      Dot = Val;
       CurOutSec->Size = Dot - CurOutSec->Addr;
       return;
     }
     assignSectionSymbol<ELFT>(AssignCmd, Dot);
     return;
   }
 
   // Handle BYTE(), SHORT(), LONG(), or QUAD().

lld/trunk/test/ELF/linkerscript/locationcountererr.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+# RUN: echo "SECTIONS { .text 0x2000 : {. = 0x10 ; *(.text) } }" > %t.script
+# RUN: not ld.lld %t --script %t.script -o %t1 2>&1 | FileCheck %s
+# CHECK: unable to move location counter backward for: .text
+
+.globl _start
+_start:
+nop