[PATCH] D130221: [ORC] Fix macho section name typo

Lang Hames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Nov 21 19:50:01 PST 2022


lhames added a comment.

Sorry -- I'm not getting Phab pings for some reason. I'll have to look into that.

> @lhames - is there some way this functionality should be observable in that unit test? Or covered in some other way?



> I was attempting to write a test where I added one of these sections to the link graph, and then looked up the init symbol that should be added, any tips for how to test this would be appreciated!

I think there's an easy test with dubious value, and a valuable but awkward/ test:

Easy but dubious: create LinkGraphs that contain each of the init sections (but none of the others), create materialization units for each of those graphs, then assert that each MU has an init symbol. The problem is that the list you use to create the graphs would usually be a copy/paste of the original, which wouldn't have found this bug. I'm not sure how much value there is in that.

Valuable but awkward: Develop an API for creating `LinkGraph`s directly from text representations (Obj2LinkGraph?) and then write an extra regression test tool for the ORC runtime (or add functionality to `llvm-jitlink`) to verify that a graph containing an objc selrefs function behaves as expected at runtime if you call a contained selector. Since constructing graphs via API calls is rare at the moment I'm not sure this is worth the effort either.

Non-testing solution: Replace this with MachOPlatform::isInitializerSection (which is easily testable as an ORC runtime regression test) -- no function, no need for a test. ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130221



More information about the llvm-commits mailing list