[PATCH] D27276: [ELF] Allow defined symbols to be assigned from linker script

[Meador Inge via Phabricator via llvm-commits]

meadori added inline comments.


================
Comment at: ELF/LinkerScript.cpp:111-118
+  if (!B || B->isUndefined()) {
+    if (Cmd->Expression.IsAbsolute())
+      Cmd->Sym = addRegular<ELFT>(Cmd);
+    else
+      Cmd->Sym = addSynthetic<ELFT>(Cmd);
+  } else {
+    Cmd->Sym = B;
----------------
ruiu wrote:
> meadori wrote:
> > ruiu wrote:
> > > I don't think you need to check if B exists. You can add a new symbol unconditionally.
> > If you do it unconditionally, then duplicate symbols might be created, e.g. `foo` is defined in an input object *and* the linker script.
> That shouldn't happen because of the architecture of LLD. Did you actually try that?
I did, and several tests in the test suite fail.  Consider making unconditional with the following diff against my current patch:

```
-  if (!B || B->isUndefined()) {
-    if (Cmd->Expression.IsAbsolute())
-      Cmd->Sym = addRegular<ELFT>(Cmd);
-    else
-      Cmd->Sym = addSynthetic<ELFT>(Cmd);
-  } else {
-    Cmd->Sym = B;
-  }
+  if (Cmd->Expression.IsAbsolute())
+    Cmd->Sym = addRegular<ELFT>(Cmd);
+  else
+    Cmd->Sym = addSynthetic<ELFT>(Cmd);
```

Then:

```
$ ./bin/llvm-nm test.o 
0000000000000001 A foo
$ cat test.ld
foo = 12;
$ ./bin/ld.lld test.o -T test.ld
./bin/ld.lld: error: duplicate symbol 'foo' in test.o and (internal)
```


https://reviews.llvm.org/D27276