[PATCH] D76995: [ELF] Propagate LMA offset to sections with neither AT() nor AT>

[Peter Smith via Phabricator via llvm-commits]

psmith added a comment.

I think there may be one case from ld that we aren't handling, and I've made a few suggestions for the documentation, otherwise looks like a good step in the right direction.

Apologies in advance for being slow to respond to reviews this week.



================
Comment at: lld/ELF/LinkerScript.cpp:866
+    ctx->lmaOffset = 0;
 
+  // Propagate ctx->lmaOffset to the first "non-header" section.
----------------
At a glance, it looks like we might not handle this particular case, is this intentional?
- If the section has a specific VMA address, then this is used as the LMA address as well.
I think that this is where there is an addrExpr but no lmaExpr we should set ctx->lmaOffset = 0;


================
Comment at: lld/docs/ELF/linker_script.rst:64
+  means the default region is used. Note, GNU ld propagates the previous LMA
+  memory region when ``address`` is not specified. The LMA is set to the
+  current location of the memory region aligned to the section alignment. In
----------------
```
The lack of ``AT>lma_region``
  means the default region is used. Note, GNU ld propagates the previous LMA
  memory region when ``address`` is not specified.
```
My understanding of the sentence is that when there is no AT > lma_region LLD does not behave like GNU ld. LLD always uses the default region, whereas GNU ld uses the previous OutputSections lma_region when address is not specified. If I'm right then is this an intentional change or a current limitation? I think we should be explicit, whichever way it is. For example:

```
If the OutputSection has no ``AT>lma_region`` then LLD will always use the default memory region. Note that this is in contrast with GNU ld which will propagate the lma_region of the previous OutputSection when ``address`` is not specified. For a script that is compatible with
GNU ld and LLD you must set ``AT>lma_region`` explicitly and not rely on propagation.


================
Comment at: lld/docs/ELF/linker_script.rst:67
+  the absence of a PHDRS command, if the LMA region is different from the
+  previous one, this section will start a new PT_LOAD segment.
+
----------------
```
In the absence of a PHDRS command, if the LMA region is different from the previous one, this section will start a new PT_LOAD segment.
```
Suggest an alternative:

```
If the linker script does not have a PHDRS command then if the ``lma_region`` for an OutputSection is different to the ``lma_region`` for the previous OutputSection a new loadable segment will be generated.
```


================
Comment at: lld/docs/ELF/linker_script.rst:70
+The two keywords cannot be specified at the same time. If neither ``AT(lma)``
+nor ``AT>lma_region`` is specified:
+
----------------
Suggest a new line or paragraph before If neither ``AT(lma)`` nor ``AT>lma_region`` is specified:


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76995/new/

https://reviews.llvm.org/D76995