[PATCH] D82251: llvm-nm: Implement --special-syms.
Fangrui Song via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jun 19 21:39:36 PDT 2020
MaskRay accepted this revision.
MaskRay added a comment.
This revision is now accepted and ready to land.
LGTM, with a nit about tests. Please give some time for other reviewers to respond.
================
Comment at: llvm/test/tools/llvm-nm/AArch64/special-syms.test:2
# RUN: yaml2obj %s -o %t
-# Test --special-syms flag. Currently this flag is a no-op, so outputs with and without
-# this flag should be identical. GNU nm doesn't show ARM and AArch64 special symbols
-# without --special-syms, so this test is to be changed when/if we decide to implement
-# GNU nm-like behavior in llvm-nm
+# Test --special-syms flag.
----------------
Consider using `## ` for comments and moving to the top.
(Most newer binary utility tests use `## ` and some reviewers find the marker makes comments stand out.)
================
Comment at: llvm/tools/llvm-nm/llvm-nm.cpp:745
+ return false;
+ return Name[0] == '$';
+}
----------------
In BFD the logic is more complex:
* `bfd/cpu-arm.c:bfd_is_arm_special_symbol_name`
* `bfd/cpu-aarch64.c:bfd_is_aarch64_special_symbol_name`
I think the simplified rule should be fine.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82251/new/
https://reviews.llvm.org/D82251
More information about the llvm-commits
mailing list