[PATCH] D113825: [llvm-readobj][XCOFF] dump auxiliary symbols.

Esme Yi via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 23 04:11:13 PST 2021


Esme added inline comments.


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols-invalid.test:6
+
+# CASE1: llvm-readobj: warning: '[[FILE]]': the C_EXT/C_WEAKEXT/C_HIDEXT symbol, which is not for a function, should have only 1 auxiliary entry, i.e. the CSECT auxiliary entry 
+
----------------
jhenderson wrote:
> How about "a non-function C_EXT/C_WEAKEXT/C_HIDEXT symbol should have only..."?
> 
> You probably should have this test case for each of the three different types. If viable, I'd also dynamically change the warning to explicitly state the type of the symbol (i.e. something like "a non-function C_EXT symbol ..." and "a non-function C_WEAKEXT symbol ..." etc).
> 
> Finally, ideally you'd incldue the symbol's name if possible in this warning, since it's not necessarily clear out-of-context which symbol this warning refers to. Applies to each of the other warnings too.
Thanks!


================
Comment at: llvm/test/tools/llvm-readobj/XCOFF/symbols-invalid.test:32
+
+## This case tests the raw data output ability when a file auxiliary entry does
+## not have the matching auxiliary type.
----------------
jhenderson wrote:
> Why specifically the file auxiliary entry and not others?
This is a case from the deleted file llvm/test/tools/llvm-readobj/XCOFF/file-aux-wrong64.test


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113825/new/

https://reviews.llvm.org/D113825



More information about the llvm-commits mailing list