This is an archive of the discontinued LLVM Phabricator instance.

[llvm-nm] Display defined weak STT_GNU_IFUNC symbols as 'i'
ClosedPublic

Authored by MaskRay on Jan 26 2021, 10:58 AM.

Details

Summary

This patch makes the behavior match GNU nm.
Note: undefined STT_GNU_IFUNC symbols use 'U'.

Diff Detail

Event Timeline

MaskRay created this revision.Jan 26 2021, 10:58 AM
MaskRay requested review of this revision.Jan 26 2021, 10:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 26 2021, 10:58 AM
MaskRay updated this revision to Diff 319416.EditedJan 26 2021, 2:59 PM
MaskRay added a subscriber: lhames.

Display Mach-O WeakRef as 'U'

Mach-O is poorly tested. CC @lhames in case he wants to add Mach-O tests:)

$ echo '__attribute__((weak)) void ref();  __attribute__((weak)) void def() { ref(); };' > test.c && clang -c test.c
$ nm test.o
0000000000000000 T _def
                 U _ref
grimar added inline comments.Jan 27 2021, 12:22 AM
llvm/tools/llvm-nm/llvm-nm.cpp
1150–1154

I'd reorder. The isa<MachOObjectFile>(Obj) condition looks a bit too burried.

if (Symflags & object::SymbolRef::SF_Undefined) {
  if (isa<MachOObjectFile>(Obj) || !(Symflags & object::SymbolRef::SF_Weak))
    return 'U';
  return isObject(Obj, I) ? 'v' : 'w';
}
1155

No need to have curly bracers here.

1156

The same comment about reordering applies here.
I think checking the object type first is cleaner.

if (!isa<MachOObjectFile>(Obj) && (Symflags & object::SymbolRef::SF_Weak))
MaskRay updated this revision to Diff 319486.Jan 27 2021, 12:41 AM
MaskRay marked 3 inline comments as done.

comments

Frankly, the llvm-nm code in this area feels like a bit of a mess, but this is at least an improvement. Just one test suggestion. Otherwise, this looks good.

llvm/test/tools/llvm-nm/ifunc.test
35–37

Maybe worth a weak undefined case too.

MaskRay updated this revision to Diff 319598.Jan 27 2021, 9:22 AM

Add undef weak

MaskRay marked an inline comment as done.Jan 27 2021, 9:22 AM
This revision is now accepted and ready to land.Jan 28 2021, 12:10 AM
This revision was landed with ongoing or failed builds.Jan 28 2021, 9:46 AM
This revision was automatically updated to reflect the committed changes.