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

[George Rimar via Phabricator via llvm-commits]

grimar accepted this revision.
grimar added a comment.

LGTM with a nit.



================
Comment at: llvm/lib/InterfaceStub/ELFObjHandler.cpp:561
+    Stream << FileReadError;
+    Stream << " when trying to open `" << FilePath << "` for writing";
+    consumeError(std::move(FileReadError));
----------------
haowei wrote:
> grimar wrote:
> > haowei wrote:
> > > grimar wrote:
> > > > You don't have a test for this I think.
> > > Emmm, I tried locally on a linux machine and it seems to be difficult to put it in test case.
> > > I have to create a file with same filename, change ownership and the permission so elfabi cannot modify it. And run llvm-elfabi to write output to this file. Is there any good way to trigger error from FileOutputBuffer?
> > > 
> > Perhaps I am missing something (I am not sure what the problem is).
> > We have a few tests in LLVM (e.g see LLD tests, like lld\test\COFF\thinlto-emit-imports.ll) that
> > do something very close to what you describe I think: they create a file, change permission and run tool on it:
> > 
> > ```
> > ; RUN: rm -f %t3.obj.imports
> > ; RUN: touch %t3.obj.imports
> > ; RUN: chmod 400 %t3.obj.imports
> > ; RUN: not lld-link -entry:main -thinlto-index-only \
> > ; RUN:     -thinlto-emit-imports-files %t1.obj %t2.obj %t3.obj \
> > ; RUN:     -out:%t4.exe 2>&1 | FileCheck %s --check-prefix=ERR
> > ; ERR: cannot open {{.*}}3.obj.imports: {{P|p}}ermission denied
> > ```
> > 
> > Can you do something like that?
> I added "fail-file-write.test".
> I thought it is discouraged to use rm, chmod commands since they are not cross-platforms.
> 
> Do I have to add anything to prevent this test from running on Windows? 
> Do I have to add anything to prevent this test from running on Windows?

I believe - no. `touch` and `rm` are often used in tests. `chmod` is a bit more rare, but
all of them are parts of `GnuWin32` and works fine under windows.


================
Comment at: llvm/test/tools/llvm-elfabi/fail-file-write.test:7
+# RUN: chmod 777 %t.TestDir
+# RUN: rm -rf %t.TestDir.
+
----------------
The last line has an excessive full stop at the end. I am not sure you need this line though.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61767