[PATCH] D75110: Avoid dangling reference when indexing sections

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added a comment.

Drive by code review, no comment on the bug or proposed solution.



================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp:83
 #ifndef NDEBUG
-static void dumpSectionMemory(const SectionEntry &S, StringRef State) {
-  dbgs() << "----- Contents of section " << S.getName() << " " << State
+static void dumpSectionMemory(const SectionList::SectionEntryProxy &S,
+                              StringRef State) {
----------------
How come `SectionEntryProxy` is sometimes passed by reference, but other times passed by value to different functions in this CL?


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldELF.h:105
+    return Sections[SectionID];
+  }
 
----------------
Every time I see `Sections[]` in RuntimeDyldELF.cpp, I think "This should be using `getSection()`."


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/RuntimeDyldMachO.h:134
+    return Sections[SectionID];
+  }
 
----------------
If this is the same between MachO and ELF (and I suspect all other derived classes), can it be a method on a shared base class?


================
Comment at: llvm/lib/ExecutionEngine/RuntimeDyld/Targets/RuntimeDyldELFMips.cpp:33
   if (IsMipsN32ABI) {
-    const SectionEntry &Section = Sections[RE.SectionID];
+    const SectionEntryProxy Section = Sections[RE.SectionID];
     Value = evaluateMIPS64Relocation(Section, RE.Offset, Value, RE.RelType,
----------------
Why not a `&`? Won't this result in a copy-assignment?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75110/new/

https://reviews.llvm.org/D75110