[PATCH] D41046: [ELF] Make overlapping output sections an error

[Alexander Richardson via Phabricator via llvm-commits]

arichardson marked 10 inline comments as done.
arichardson added inline comments.


================
Comment at: ELF/Writer.cpp:1889-1890
+                                   Predicate ShouldSkip) {
+  auto FirstInteresting = partition(AllSections, ShouldSkip);
+  MutableArrayRef<OutputSection *> InterestingSections(AllSections);
+  InterestingSections = InterestingSections.slice(
----------------
ruiu wrote:
> ruiu wrote:
> > Please do not use `auto` in lld unless the actual type is obvious in a very narrow context (e.g. RHS).
> These variable names are a bit too lengthy and don't match other code in lld. Also this patch uses higher order functions than other code, so it feels different. I'd make this consistent with other code.
> 
> E.g. I'd write this part as
> 
>   std::vector<OutputSection *> Sections;
>   for (OutputSection *Sec : AllSections)
>     if (!ShouldSkip(Sec))
>       Sections.push_back(Sec);
Looking at this code again, the previous version without std::partition was much easier to understand. I've also updated to use for loops instead of copy_if


================
Comment at: ELF/Writer.cpp:1908
+      uint64_t OtherStart = GetStart(Other);
+      if (Start + Sec->Size > OtherStart && Start < OtherStart + Other->Size)
+        errorOrWarn("section " + Sec->Name + " " + Kind +
----------------
ruiu wrote:
> Isn't this condition enough?
> 
>   if (GetStart(Other) < GetStart(Sec) + Sec->Size)
It is now that the sections are sorted. Originally I didn't sort by address so it was required.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D41046