This is an archive of the discontinued LLVM Phabricator instance.

[ELF] - Fix crash relative to SHF_LINK_ORDER sections.
ClosedPublic

Authored by grimar on Mar 7 2018, 2:30 AM.

Details

Summary

Our code assumes all input sections in an output SHF_LINK_ORDER section has SHF_LINK_ORDER flag.
We do not check that and that can cause a crash.
That happens because we call
std::stable_sort(Sections.begin(), Sections.end(), compareByFilePosition);,
where compareByFilePosition does not expect to see null when calls getLinkOrderDep.
Given that we never faced it before, I suggest to error out that case.

The same might happen when sections refer to non-regular sections.
Test cases demonstrate the issues.

Diff Detail

Repository
rLLD LLVM Linker

Event Timeline

grimar created this revision.Mar 7 2018, 2:30 AM
grimar planned changes to this revision.Mar 7 2018, 2:37 AM
grimar updated this revision to Diff 137355.Mar 7 2018, 3:38 AM
  • Implemented in a different way.

George Rimar via Phabricator via llvm-commits
<llvm-commits@lists.llvm.org> writes:

Index: ELF/OutputSections.cpp

  • ELF/OutputSections.cpp

+++ ELF/OutputSections.cpp
@@ -100,7 +100,8 @@

  Flags = IS->Flags;
} else {
  // Otherwise, check if new type or flags are compatible with existing ones.
  • if ((Flags & (SHF_ALLOC | SHF_TLS)) != (IS->Flags & (SHF_ALLOC | SHF_TLS)))

+ if ((Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)) !=
+ (IS->Flags & (SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER)))

Please move SHF_ALLOC | SHF_TLS | SHF_LINK_ORDER to a mask variable.

LGTM with that.

Cheers,
Rafael

This revision was not accepted when it landed; it landed in state Needs Review.Mar 8 2018, 7:10 AM
This revision was automatically updated to reflect the committed changes.