[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