[PATCH] D42471: [ARM] Fix lld crash introduced by r321154

Igor Kudrin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Jan 24 06:49:52 PST 2018


ikudrin added a comment.

Thank you for the patch. I agree with Peter, the fix should be accomplished with a test.

In https://reviews.llvm.org/D42471#986379, @vit9696 wrote:

> I could provide an example, but it is kind of obvious.


The simpler, the better. Such tests help us avoid regressions in the future.



================
Comment at: ELF/SyntheticSections.cpp:2588
 bool ARMExidxSentinelSection::empty() const {
-  OutputSection *OS = getParent();
-  for (auto *B : OS->SectionCommands)
-    if (auto *ISD = dyn_cast<InputSectionDescription>(B))
-      for (auto *S : ISD->Sections)
-        if (!isa<ARMExidxSentinelSection>(S))
-          return false;
+  if (OutputSection *OS = getParent()) {
+    for (auto *B : OS->SectionCommands)
----------------
Use early return here. It's better than increasing the level of nesting.


================
Comment at: ELF/Writer.cpp:1312
     OutputSection *OS = SS->getParent();
-    if (!SS->empty() || !OS)
+    if (!OS || !SS->empty())
       continue;
----------------
I think this change is independent. It's more about style, so it should be applied separately.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D42471





More information about the llvm-commits mailing list