[ELF] - Do not handle --defsym as regular linkerscript assignment.

[grimar]

Summary

With following code and script:

.global foo
foo:

VER1 { global: foo ; local: * ; } ;

ld.bfd does not allow to redefine symbol:

++ ld.bfd --defsym=foo=2 --version-script version.txt -shared test.o -o test.so
test.o: In function `foo':
(.text+0x0): multiple definition of `foo'

So semantic of --defsym is like a symbol declaration.
LLD allowed that, patch fixes it.

[grimar]

• Fixed last minute mistype.

[grimar]

• Rebased, updated.

[ruiu]

I agree that we shouldn't introduce a new semantics just for reporting an error for a corner use case which looks like we don't need to mimic the exact behavior of the bfd linker.

[grimar]

Abandoning basing on discussion about this.

ELF/LinkerScript.h

struct SymbolAssignment : BaseCommand {
 
   // The RHS of an expression.
   Expr Expression;
 
   // Command attributes for PROVIDE, HIDDEN and PROVIDE_HIDDEN.
   bool Provide = false;
   bool Hidden = false;
 
+  // Flag shows if this command is -defsym assignment.
+  bool IsDefsym = false;
+
   // Holds file name and line number for error reporting.
   std::string Location;
 };
 
 // Linker scripts allow additional constraints to be put on ouput sections.
 // If an output section is marked as ONLY_IF_RO, the section is created
 // only if its input sections are read-only. Likewise, an output section
 // with ONLY_IF_RW is created if all input sections are RW.

ELF/LinkerScript.cpp

void LinkerScript::addSymbol(SymbolAssignment *Cmd) {
   // it is a referenced undefined symbol.
   Symbol *B = Symtab->find(Cmd->Name);
   if (Cmd->Provide && (!B || B->isDefined()))
     return;
 
   // Define a symbol.
   Symbol *Sym;
   uint8_t Visibility = Cmd->Hidden ? STV_HIDDEN : STV_DEFAULT;
-  std::tie(Sym, std::ignore) = Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
-                                              /*CanOmitFromDynSym*/ false,
-                                              /*File*/ nullptr);
   ExprValue Value = Cmd->Expression();
   SectionBase *Sec = Value.isAbsolute() ? nullptr : Value.Sec;
 
   // When this function is called, section addresses have not been
   // fixed yet. So, we may or may not know the value of the RHS
   // expression.
   //
   // For example, if an expression is `x = 42`, we know x is always 42.
   // However, if an expression is `x = .`, there's no way to know its
   // value at the moment.
   //
   // We want to set symbol values early if we can. This allows us to
   // use symbols as variables in linker scripts. Doing so allows us to
   // write expressions like this: `alignment = 16; . = ALIGN(., alignment)`.
   uint64_t SymValue = Value.Sec ? 0 : Value.getValue();
 
+  // Regular linker script assignment may be used to update existent symbol
+  // value, but --defsym assignment should always create new symbol and report
+  // duplicate symbol definition error if symbol already exists.
+  if (Cmd->IsDefsym) {
+    Sym = Symtab->addRegular(Cmd->Name, Visibility, STT_NOTYPE, SymValue,
+                             /*Size=*/0, STB_GLOBAL, Sec,
+                             /*File=*/nullptr);
+  } else {
+    std::tie(Sym, std::ignore) =
+        Symtab->insert(Cmd->Name, /*Type*/ 0, Visibility,
+                       /*CanOmitFromDynSym*/ false,
+                       /*File*/ nullptr);
     replaceSymbol<Defined>(Sym, nullptr, Cmd->Name, STB_GLOBAL, Visibility,
                            STT_NOTYPE, SymValue, 0, Sec);
+  }
   Cmd->Sym = cast<Defined>(Sym);
 }
 
 // This function is called from assignAddresses, while we are
 // fixing the output section addresses. This function is supposed
 // to set the final value for a given symbol assignment.
 void LinkerScript::assignSymbol(SymbolAssignment *Cmd, bool InSec) {
   if (Cmd->Name == ".") {

ELF/ScriptParser.cpp

void ScriptParser::readLinkerScript() {
   }
 }
 
 void ScriptParser::readDefsym(StringRef Name) {
   Expr E = readExpr();
   if (!atEOF())
     setError("EOF expected, but got " + next());
   SymbolAssignment *Cmd = make<SymbolAssignment>(Name, E, getCurrentLocation());
+  Cmd->IsDefsym = true;
   Script->SectionCommands.push_back(Cmd);
 }
 
 void ScriptParser::addFile(StringRef S) {
   if (IsUnderSysroot && S.startswith("/")) {
     SmallString<128> PathData;
     StringRef Path = (Config->Sysroot + S).toStringRef(PathData);
     if (sys::fs::exists(Path)) {

test/ELF/linkerscript/symbol-multiple-defs.s

+# REQUIRES: x86
+# RUN: llvm-mc -filetype=obj -triple=x86_64-unknown-linux %s -o %t
+
+# RUN: echo "foo = 0x3;" > %t.script
+# RUN: ld.lld -o %t1 --script %t.script %t
+# RUN: llvm-objdump -t %t1 | FileCheck %s --check-prefix=SCRIPT
+# SCRIPT: 0000000000000003 *ABS* 00000000 foo
+
+# RUN: not ld.lld --defsym=foo=0x3 -o %t2 %t 2>&1 \
+# RUN:  | FileCheck %s --check-prefix=ERR
+# ERR: duplicate symbol: foo
+
+.global foo
+foo: