This is an archive of the discontinued LLVM Phabricator instance.

[ORC] Extract hasInitializerSection for testing (NFC)
ClosedPublic

Authored by keith on Dec 5 2022, 10:22 AM.

Diff Detail

Event Timeline

keith created this revision.Dec 5 2022, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 10:22 AM
keith requested review of this revision.Dec 5 2022, 10:22 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 5 2022, 10:22 AM
dblaikie added inline comments.Dec 5 2022, 2:25 PM
llvm/lib/ExecutionEngine/Orc/ObjectFileInterface.cpp
300

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.

llvm/unittests/ExecutionEngine/JITLink/LinkGraphTests.cpp
714–749

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))

keith updated this revision to Diff 481453.Dec 8 2022, 2:49 PM
keith marked an inline comment as done.

Parameterize tests and add all test cases

dblaikie accepted this revision.Dec 8 2022, 6:02 PM

Looks good (:

This revision is now accepted and ready to land.Dec 8 2022, 6:02 PM
keith updated this revision to Diff 482152.Dec 12 2022, 9:00 AM

rebase to run CI

This revision was landed with ongoing or failed builds.Dec 12 2022, 10:25 AM
This revision was automatically updated to reflect the committed changes.

Do you have commit access, or do you need me/someone to commit this for you?

I landed, thanks all!