[PATCH] D139347: [ORC] Extract hasInitializerSection for testing (NFC)

Mon Dec 5 14:25:16 PST 2022

dblaikie added inline comments.

================
Comment at: llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp:300
+      return true;
+    if (IsElf && ELFNixPlatform::isInitializerSection(Sec.getName()))
+      return true;
----------------
Aside/@lhames - I /think/ maybe this is meant to be detected by flag rather than name? ( https://docs.oracle.com/cd/E19683-01/816-1386/6m7qcobks/index.html "When creating a dynamic object, the link-editor identifies these arrays with the .dynamic tags DT_PREINIT_ARRAY and DT_PREINIT_ARRAYSZ, DT_INIT_ARRAY and DT_INIT_ARRAYSZ, and DT_FINI_ARRAY and DT_FINI_ARRAYSZ accordingly, so that they may be called by the runtime linker." - so maybe these shouldn't be being detected by name, but by these tags? In any case, that could be fixed here separately from this change in review, but something to keep in mind, perhaps.

================
Comment at: llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp:714-749
+TEST(LinkGraphTest, InitSymbolSections) {
+  {
+    auto Graph =
+        std::make_unique<LinkGraph>("foo", Triple("x86_64-apple-darwin"), 8,
+                                    support::little, getGenericEdgeKindName);
+    Graph->createSection("__DATA,__objc_selrefs",
+                         orc::MemProt::Read | orc::MemProt::Write);
----------------
Could you use data expansion to test this and reduce duplication (& then it might not be too bad to test all the cases, rather than only a sample/exemplar)? Sometihng like the pet thing using `ValuesIn` shown here: https://github.com/google/googletest/blob/main/docs/advanced.md (maybe have a separate Darwin and Linux test (or maybe even have that as a parameter too) and then pairs of section name + true/false for whether it has an initializer))

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139347