[RuntimeDyld] load all sections with ProcessAllSections

[yonghong-song]

Summary

This commit tried to address the following use case.

. bcc (https://github.com/iovisor/bcc) utilizes llvm JIT to
  compile for BTF target.
. with -g, .BTF and .BTF.ext sections (BPF debug info)
  will be generated by LLVM.
. .BTF does not have relocations and .BTF.ext has some
  relocations.
. With ProcessAllSections, .BTF.ext is loaded by JIT dynamic linker
  and is available to application. But .BTF is not loaded.

The bcc application needs both .BTF.ext and .BTF for debugging
purpose, and .BTF is not loaded. This patch addressed this issue
by iterating over all sections and loading any missing
sections, after symbol/relocation processing in loadObjectImpl().

[yonghong-song]

runtime-ld.tar20 KBDownload

I uploaded a test case to demonstrate the use case related to BPF and .BTF section, in case that you want to reproduce in this specific use case context.

@lhames during last October LLVM deverloper conference, I has a brief chat with you about this problem. You mentioned that ProcessAllSections should load all sections.
If some sections are not loaded, maybe there is a bug here.

Please take a look at the patch. It may not be the best implementation, or you could suggest a better interface. FYI, here, in addition to .BTF.ext section, I just
want to load .BTF section as well. An interface to permit user to load selected sections should also work if it is not desirable to really load all of them (e.g., symtab etc.)

Thanks!

[labath]

I only have a very vague memory of making one patch to this part of code, so I don't think I am a suitable reviewer. Lang is the right person for the job.

[yonghong-song]

@labath No problem. Yes, I just searched and find you also contributed patch(es) there in non architecture-dependent way. @lhames is in the review list. Thanks!

[yonghong-song]

Hi, @lhames Pinging, have you got time to take a look at this issue?

[yonghong-song]

Hi, @lhames For this patch, if ProcessAllSections is set to true, all sections will be loaded.
I am not sure whether this is the original intention of not as it says ProcessAllSections, not meaning LoadAllSections.

So I implemented an alternative implementation in D56729. It extends RuntimeDyldELF for BPF target. It will load .BTF section (which is what we want in the bcc or other JIT implementations).

Could you let me know which method you prefer and how we should proceed from here? Thanks!

[lhames]

Hi Yonghong,

ProcessAllSections is kind of subtle. It is up to the user allocator whether the section is actually allocated in the end, but it means that you should call findOrEmit for every section in the object file, regardless of whether or not RuntimeDyld deems it reachable. Its original purpose was as a hack to ensure that clients could get access to debug and other metadata sections that would have otherwise been dropped.

Given that, I think the following check should be removed:

if (RelocatedSection != SE)
  continue;

When ProcessAllSections is set, even non-relocatable sections should be emitted.

Otherwise this looks good to me.

[yonghong-song]

address Lang's comments. Emit all not-yet-loaded sections.

[yonghong-song]

@lhames Thanks for the review. I just addressed your comments to emit all remaining not-yet-emitted sections. Could you take a look?
Also since the approach for this patch is preferred, I have abandoned my another patch D56729. Thanks!

[yonghong-song]

change one comment from C style to C++ style

[yonghong-song]

Hi, @lhames, could you check whether my change addressed your concern or not? Thanks!

[yonghong-song]

@lhames Just ping, did you get a chance looking at the updated patch? Thanks!

[yonghong-song]

@lhames Pinging. Could you take a look whether the modified patch addressed your comments or not? Thanks!

[lhames]

LGTM! Do you have commit access?

[yonghong-song]

Yes, I do. I can follow the instructions at https://llvm.org/docs/Phabricator.html to commit this change. Thanks!

llvm/trunk/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp

for (; I != E;)
         return IOrErr.takeError();
 
     // If there is an attached checker, notify it about the stubs for this
     // section so that they can be verified.
     if (Checker)
       Checker->registerStubMap(Obj.getFileName(), SectionID, Stubs);
   }
 
+  // Process remaining sections
+  if (ProcessAllSections) {
+    LLVM_DEBUG(dbgs() << "Process remaining sections:\n");
+    for (section_iterator SI = Obj.section_begin(), SE = Obj.section_end();
+         SI != SE; ++SI) {
+
+      /* Ignore already loaded sections */
+      if (LocalSections.find(*SI) != LocalSections.end())
+        continue;
+
+      bool IsCode = SI->isText();
+      if (auto SectionIDOrErr =
+              findOrEmitSection(Obj, *SI, IsCode, LocalSections))
+        LLVM_DEBUG(dbgs() << "\tSectionID: " << (*SectionIDOrErr) << "\n");
+      else
+        return SectionIDOrErr.takeError();
+    }
+  }
+
   // Give the subclasses a chance to tie-up any loose ends.
   if (auto Err = finalizeLoad(Obj, LocalSections))
     return std::move(Err);
 
 //   for (auto E : LocalSections)
 //     llvm::dbgs() << "Added: " << E.first.getRawDataRefImpl() << " -> " << E.second << "\n";
 
   return LocalSections;