[PATCH] D61767: [llvm-elfabi] Emit ELF header and string table section

[George Rimar via Phabricator via llvm-commits]

grimar added inline comments.


================
Comment at: llvm/test/tools/llvm-elfabi/fail-file-write.test:16
+
+# ERR: Permission denied when trying to open
----------------
haowei wrote:
> grimar wrote:
> > haowei wrote:
> > > jhenderson wrote:
> > > > haowei wrote:
> > > > > jhenderson wrote:
> > > > > > jhenderson wrote:
> > > > > > > Please check the full error here, rather than a partial one. Additionally, I would be suspicious that the error message will be different depending on whether the host OS is Linux or Windows. Please could you check? If you don't have access to both systems, let me know, and I can test the one you can't.
> > > > > > Looking at the pre-merge checks this test is failing on Windows as-is.
> > > > > I have a windows workstation but it does not have development environment set up for llvm. I will see if I can set it up tomorrow. For now, let me test it on bots first.
> > > > Did you get anywhere with resolving this? The test is still failing on the pre-merge bot, as noted near the top of this page.
> > > I finally get my windows workstation to build llvm successfully. So the reason this test failed on Windows is that, the `chmod` seems to be a no-op on windows. It didn't change the permission of the %t.TestDir at all. The failure at `not llvm-elfabi...` did not happen so the test failed.
> > > 
> > > I don't have a good workaround for this so I disabled the test under Windows for now. Do you have any good suggestion for testing this failure under Windows?
> > I guess you should be able to change the permission for the file, if not for directory.
> > 
> > Our LLD test (ELF\lto\thinlto-cant-write-index.ll) has the following lines:
> > 
> > ```
> > ; Ensure lld generates error if unable to write to index files
> > ; RUN: rm -f %t2.o.thinlto.bc
> > ; RUN: touch %t2.o.thinlto.bc
> > ; RUN: chmod u-w %t2.o.thinlto.bc
> > ; RUN: not ld.lld --plugin-opt=thinlto-index-only -shared %t1.o %t2.o -o /dev/null 2>&1 | FileCheck %s
> > ; RUN: chmod u+w %t2.o.thinlto.bc
> > ; CHECK: cannot open {{.*}}2.o.thinlto.bc: {{P|p}}ermission denied
> > 
> > ```
> > 
> > This test is not disabled for windows amd I believe that the last line has `{{P|p}}` because the message
> > produced is different on windows/*nix.
> > 
> > There are other LLD tests that are using chmod on files and are not disabled on windows.
> If I only change the file permission, it won't work under linux. Apparently the file write function used in elfabi will remove the file first and then recreate it. If the directory contains the file has wx permission, the file can be removed regardless of the file's own permission bits. That is why in this test, I changed the permission of the directory, not the file.
I see. Perhaps it is OK to keep it unsupported at least for now.
(may be someone else have ideas, I don't have any currently).

I am not sure how your way of doing it works well though.
Usually in tests the "system-windows" is commonly used:

```
UNSUPPORTED: system-windows
```

I also observe "windows-gnu", "windows-msvc" and just "windows" versions but,
don't see "-windows-" anywhere.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61767