[PATCH] D36986: [ELF] Handle assignments outside SECTIONS command separately

Rui Ueyama via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 23 22:09:46 PDT 2017


ruiu added inline comments.


================
Comment at: ELF/LinkerScript.h:78
+  // Whether the command is within SECTIONS command.
+  bool Sections = true;
 };
----------------
phosek wrote:
> ruiu wrote:
> > I think I'd name this `InSections`.
> > 
> > But do you actually need this? This is I believe virtually a flag indicating whether an element is a direct child of the top element or not. But we can distinguish BaseCommands enclosed in an OutputSections from BaseCommands that are top-level elements, no?
> That's what I tried initially, the problem is that `Opt.Commands` would also contain synthetic symbol assignments created by `fabricateDefaultCommands` and those have to be processed  in `assignAddresses` because they modify `.`, so we would need some way to distinguish those commands from other top level symbol assignments. One option would be to create a new subclass e.g. `SyntheticSymbolAssignment` or even `DotAssignment` which would then allow distinguishing the two types of assignments in `assignAddresses`. The other option would be to just add an attribute to `SymbolAssignment`. Any opinion/preference?
I'm not very sure, but creating `DotAssignment` sounds like a good idea, as assignments to `.` are very different from symbol assignments despite their superficial resemblance. It might be worth a try.


================
Comment at: ELF/LinkerScript.h:78
+  // Whether the command is within SECTIONS command.
+  bool Sections = true;
 };
----------------
phosek wrote:
> ruiu wrote:
> > phosek wrote:
> > > ruiu wrote:
> > > > I think I'd name this `InSections`.
> > > > 
> > > > But do you actually need this? This is I believe virtually a flag indicating whether an element is a direct child of the top element or not. But we can distinguish BaseCommands enclosed in an OutputSections from BaseCommands that are top-level elements, no?
> > > That's what I tried initially, the problem is that `Opt.Commands` would also contain synthetic symbol assignments created by `fabricateDefaultCommands` and those have to be processed  in `assignAddresses` because they modify `.`, so we would need some way to distinguish those commands from other top level symbol assignments. One option would be to create a new subclass e.g. `SyntheticSymbolAssignment` or even `DotAssignment` which would then allow distinguishing the two types of assignments in `assignAddresses`. The other option would be to just add an attribute to `SymbolAssignment`. Any opinion/preference?
> > I'm not very sure, but creating `DotAssignment` sounds like a good idea, as assignments to `.` are very different from symbol assignments despite their superficial resemblance. It might be worth a try.
> Turned out it's even more complicated than that, in `SECTIONS { .text : { *(.text) } foo = ABSOLUTE(.) + 0x100; .got  : { *(.got) } }` the `foo = ABSOLUTE(.)` is still going to end up in `Scripts->Opt.Commands`, but we have to process it early because it uses `.`.
> 
> I have added the `InSections` attribute to `BaseCommand` because in LLD, we allow `ASSERT` in input linker scripts, which means it can also refer to predefined symbols. However, neither BFD ld nor gold does allow that. If we followed the same model, we could move `InSections` to `SymbolAssignment`.
We do not aim to extend the linker script that GNU linkers, and we do not guarantee that the current undocumented behavior (lld supports only the documented behavior of GNU linkers). So, I think I'd move `InSections` to `SymbolAssignment`.


Repository:
  rL LLVM

https://reviews.llvm.org/D36986





More information about the llvm-commits mailing list