[PATCH] D153652: [Support] Don't set "all_exe" mode by default for file written by llvm::writeToOutput

Mon Jun 26 05:05:35 PDT 2023

hokein added a comment.

Thanks for the comment.

> unit test which checks llvm::writeOutput may be added to raw_ostream_test.cpp. Let`s this test check the case when "all_exec" is not set. i.e. that after default usage of llvm::writeOutput the resulting file does not have "all_exec", which is exact purpose of this patch.

OK, added one.

================
Comment at: llvm/test/tools/llvm-dwarfutil/ELF/X86/file-permissions.test:1
+# RUN: yaml2obj %p/Inputs/common.yaml -o %t.o
+# RUN: chmod a+x %t.o
----------------
jhenderson wrote:
> hokein wrote:
> > jhenderson wrote:
> > > It might make sense to reuse the test code from llvm-objcopy's mirror-permissions tests. See my other inline comment too.
> > by reusing the code, I think you mean calling the `llvm-dwardfutil` tool in the `llvm-objcopy` lit test, I'm not sure it is a good idea, calling a different tool in the `llvm/test/tools/llvm-objcopy/mirror-permissions-*.test` seems like violating the layer.
> I meant that you could copy and adapt the existing llvm-objcopy test to create a new llvm-dwarfutil test (in the llvm-dwarfutil test folder).
I see, adjusted the test per comment.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D153652