[PATCH] D158004: llvm-nm ignore the Import symbol file for the --export-symbol option.

[James Henderson via Phabricator via llvm-commits]

jhenderson added a comment.

In D158004#4616402 <https://reviews.llvm.org/D158004#4616402>, @MaskRay wrote:

> In D158004#4616388 <https://reviews.llvm.org/D158004#4616388>, @jhenderson wrote:
>
>> In D158004#4616384 <https://reviews.llvm.org/D158004#4616384>, @MaskRay wrote:
>>
>>> Changing `llvm::identify_magic` to recognize all files starting with `#!` as `aix_linker_import_file` is not correct. Many can well be used as shebang for shell or other scripts.
>>>
>>>   case '#':
>>>     if (Magic[1] == '!')
>>>       return file_magic::aix_linker_import_file;
>>>     break;
>>
>> How would you suggest that @DiggerLin proceeds then? The impact of misidentifying a file with a shebang as an AIX import file is that tools will no longer report "invalid object file" or something to that effect for such files, but will otherwise do nothing with them, so it seems relatively harmless to me.
>
> Sorry, I have analyzed the AIX use cases... I just noticed this patch and am concerned that `identify_magic` mischaracterizing files with shebang as AIX-specific would likely cause beyond-negligible negative impacts...
> `identify_magic` in LLVMBinaryFormat is somewhat commonly used outside of llvm-project.

Could you elaborate please what these "beyond-negligible negative impacts" are? I've highlighted the one behaviour impact that I'm aware of, which I think we can ignore as not really an issue.

It seems to me like you're blocking this patch because the AIX import file magic happens to clash with some general text file contents. How else can such a file be identified?

@DiggerLin/@hubert.reinterpretcast et al, what is preventing the change to clang to not call llvm-nm with import files?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158004