[PATCH] [lld] [LinkerScript] Implement semantics for simple sections mappings

[Shankar Kalpathi Easwaran]

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:168
@@ -163,2 +167,3 @@
     Sections,
+    SortedGroup,
     SymbolAssignment,
----------------
rafaelauler wrote:
> shankarke wrote:
> > Could you represent all the different kinds of functions that can operate on the input sections as Functions instead of InputSectionKinds ?
> I'm afraid I'm not on the same page, which functions are you mentioning?
Thinking about this again, Its ok.

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:1122
@@ +1121,3 @@
+  struct SectionKey {
+    StringRef archiveName;
+    StringRef memberName;
----------------
rafaelauler wrote:
> shankarke wrote:
> > can you rename this to archiveNameOrPath
> I was under the impression that, from the linker script perspective, we should always work with file names instead of paths. For example, to make my test case work correctly, I need to drop the path that comes from the File object and use only the file name. Only then I can match it with a linker script rule because they omit the full path. Can a linker script specify a full path to an archive?
I think so, you can specify a full path to the archive too. why not ?

================
Comment at: include/lld/ReaderWriter/LinkerScript.h:1130
@@ +1129,3 @@
+  /// number (caching results).
+  struct SectionKeyHash {
+    int64_t operator()(const SectionKey &k) const {
----------------
rafaelauler wrote:
> ruiu wrote:
> > Instead of defining a class (struct) implementing an operator=(const SectionKey &), you can simply define of a function (int64_t f(const SectionKey &)) and pass it to std::unordered_map.
> > 
> > You want to move this below private:.
> I don't understand how caching here would be error prone. I'm simply implementing the design that was suggested from the previous thread of D7915:
> 
> * Make DefaultLayout ask Sema about the order of two sections by section name (or, more precisely, section "key") and do not expose "ids".
> 
> * Since these questions are frequently asked for the same input section, we cache the result.
> 
> Which users do you refer to? You mean, the TargetLayout?
Yes, TargetLayout.

================
Comment at: lib/ReaderWriter/ELF/DefaultLayout.h:350
@@ +349,3 @@
+/// override some of the functionalities.
+template <class ELFT> class ScriptLayout : public DefaultLayout<ELFT> {
+public:
----------------
rafaelauler wrote:
> ruiu wrote:
> > Merge this with DefaultLayout and remove this class.
> No problem, but first can we get a consensus on whether do we need ScriptLayout or not? I'm asking this because I only moved the code out of DefaultLayout due to Shankar's solicitation.
DefaultLayout was designed as a class to handle generic ELF Layout when not having linker scripts. Some of the linker script usecases are not even relevant for the DefaultLayout. Conditionally executing things in the DefaultLayout if we have a linker script IMO is a bad design. 

================
Comment at: lib/ReaderWriter/ELF/SegmentChunks.h:437
@@ -402,1 +436,3 @@
+        if (auto expr = dyn_cast<ExpressionChunk<ELFT>>(section))
+          fileOffset += expr->virtualAddr() - lastVirtualAddress;
       // Align fileoffset to the alignment of the section.
----------------
rafaelauler wrote:
> shankarke wrote:
> > why would you do this ?
> This is used to keep in sync the file image with the memory image.
Do you have a specific test for this ? I would like to see that to understand here.

http://reviews.llvm.org/D8157

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/