This is an archive of the discontinued LLVM Phabricator instance.

[ifs] Add --exclude flag
ClosedPublic

Authored by abrachet on Feb 16 2022, 11:04 AM.

Details

Summary

Use to remove certain symbols which match the glob pattern. Can be used with --strip-undefined

Diff Detail

Event Timeline

abrachet created this revision.Feb 16 2022, 11:04 AM
abrachet requested review of this revision.Feb 16 2022, 11:04 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 16 2022, 11:04 AM
haowei added inline comments.Feb 16 2022, 11:51 AM
llvm/tools/llvm-ifs/llvm-ifs.cpp
298

I think it is better to put this back into the IFSHandler.cpp. That file is part of the InterfaceStub library so it can be used by other tools in the future.

abrachet updated this revision to Diff 409393.Feb 16 2022, 1:39 PM
abrachet retitled this revision from [ifs] Add --preserve flag for keeping symbols that would be removed by --strip-undefined to [ifs] Add --preserve and --exclude flags.
abrachet edited the summary of this revision. (Show Details)
  • Add --exclude too
  • Move implementation into lib/InterfaceStub
haowei added a subscriber: phosek.

The implementation of the filtering feature LGTM. My concerns is that do we need both 2 flags are needed at the same time. I am not very familiar with the __asan symbols use case so I will ask @mcgrathr and @phosek to comment on this.

Usually when you have these kinds of things they're called include and exclude.
I don't think we have any known use cases for include, just for exclude so it would be sufficient to start with just that feature.

abrachet updated this revision to Diff 409835.Feb 17 2022, 6:43 PM
abrachet retitled this revision from [ifs] Add --preserve and --exclude flags to [ifs] Add --exclude flag.
abrachet edited the summary of this revision. (Show Details)
  • Remove --preserve
  • Make tests more readable by starting and ending with Symbols: and ... respectively, instead of using CHECK-NOT
haowei accepted this revision.Feb 17 2022, 11:30 PM

LGTM.

Please correct the comments of the test file.
Please wait for @mcgrathr 's approval before you land the change.

llvm/test/tools/llvm-ifs/exclude.test
2

--preserve is already removed from this patch

This revision is now accepted and ready to land.Feb 17 2022, 11:30 PM
mcgrathr accepted this revision.Feb 18 2022, 11:11 AM
This revision was landed with ongoing or failed builds.Feb 18 2022, 11:15 AM
This revision was automatically updated to reflect the committed changes.