[PATCH] D54621: [ELF] - Do not remove empty sections referenced in LOADADDR/ADDR commands.

[Peter Smith via Phabricator via llvm-commits]

peter.smith added a comment.

I think that this change is worth making as it is a reasonable to expect ADDR and LOADADDR to return a valid result even when empty. I note that with wildcards the author may not know ahead of time whether the OutputSection will be empty or not so this doesn't just affect people intentionally using empty OutputSections.

I've take another look and have made some suggestions to make it easier to understand.



================
Comment at: ELF/LinkerScript.cpp:832
 
   // We do not want to remove sections that reference symbols in address and
   // other expressions. We add script symbols as undefined, and want to ensure
----------------
I think it might be best to keep Sec.ExpressionsUseSymbols and introduce a new bool variable called something like UsedInExpression and set that whenever ADDR and LOADADDR uses the name. Even though the effect (keep section) is the same, I think the reason to keep it is different and it gets lost in the comment. Perhaps something like:

```
// We do not want to remove OutputSections with expressions that reference symbols even if the OutputSection is empty. We want to ensure that the expressions can be evaluated and report an error if they cannot.
if (Sec.ExpressionsUseSymbols)
  return false;

// OutputSections may be referenced by name in ADDR and LOADADDR expressions, as an empty Section can has a valid VMA and LMA we keep the OutputSection to maintain the integrity of the other Expression.
if (Sec.UsedInExpression)
  return false;  
```



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

https://reviews.llvm.org/D54621