Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[lld] lld/ELF/Writer.cpp: write directly to outs when output is "-" #72061

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

keithw
Copy link

@keithw keithw commented Nov 12, 2023

This PR lets the LLD ELF driver write directly to lld::outs() (the stdoutOS argument to lld::elf::link()), when the output filename is "-".
This lets the LLD ELF driver be used as a library with its output written to an in-memory buffer.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 12, 2023

@llvm/pr-subscribers-lld-elf

Author: Keith Winstein (keithw)

Changes

This PR lets the LLD ELF driver write directly to outs (skipping the filesystem) when the output filename is "-".
This is a step towards being able to use LLD as a library without using an open syscall.


Full diff: https://github.com/llvm/llvm-project/pull/72061.diff

1 Files Affected:

  • (modified) lld/ELF/Writer.cpp (+7)
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a84e4864ab0e5a5..b5d0c0cafd230dd 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -652,6 +652,13 @@ template <class ELFT> void Writer<ELFT>::run() {
     if (errorCount())
       return;
 
+    if (config->outputFile == "-") {
+      lld::outs() << StringRef((const char *)buffer->getBufferStart(),
+                               buffer->getBufferSize());
+      lld::outs().flush();
+      return;
+    }
+
     if (auto e = buffer->commit())
       fatal("failed to write output '" + buffer->getPath() +
             "': " + toString(std::move(e)));

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 12, 2023

@llvm/pr-subscribers-lld

Author: Keith Winstein (keithw)

Changes

This PR lets the LLD ELF driver write directly to outs (skipping the filesystem) when the output filename is "-".
This is a step towards being able to use LLD as a library without using an open syscall.


Full diff: https://github.com/llvm/llvm-project/pull/72061.diff

1 Files Affected:

  • (modified) lld/ELF/Writer.cpp (+7)
diff --git a/lld/ELF/Writer.cpp b/lld/ELF/Writer.cpp
index a84e4864ab0e5a5..b5d0c0cafd230dd 100644
--- a/lld/ELF/Writer.cpp
+++ b/lld/ELF/Writer.cpp
@@ -652,6 +652,13 @@ template <class ELFT> void Writer<ELFT>::run() {
     if (errorCount())
       return;
 
+    if (config->outputFile == "-") {
+      lld::outs() << StringRef((const char *)buffer->getBufferStart(),
+                               buffer->getBufferSize());
+      lld::outs().flush();
+      return;
+    }
+
     if (auto e = buffer->commit())
       fatal("failed to write output '" + buffer->getPath() +
             "': " + toString(std::move(e)));

Co-authored-by: Keith Winstein <keithw@cs.stanford.edu>
@keithw
Copy link
Author

keithw commented Nov 16, 2023

@smithp35 @MaskRay

Copy link
Member

@MaskRay MaskRay left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ld.lld a.o -o - already writes to stdout. See https://reviews.llvm.org/D56940

@keithw
Copy link
Author

keithw commented Nov 16, 2023

Yeah -- unfortunately that writes to llvm::outs(), i.e. the STDOUT_FILENO file descriptor. This PR makes it write to lld::outs(), the stdoutOS argument to the lld::elf::link() call. We're using this so we can use the LLD ELF driver as a library where the object gets written to an in-memory raw_string_ostream that we pass as stdoutOS.

Maybe there's a cleaner way to do this? I couldn't think of any that didn't involve a much bigger change (e.g. I guess we could think about changing the FileOutputBuffer in lld/ELF/Writer.cpp and changing the signature to lld::elf::link() to pass in an explicit output buffer, but that would require changing all the drivers' link function signatures which seems like overkill...).

@MaskRay
Copy link
Member

MaskRay commented Nov 17, 2023

Ah, thanks for the explanation. The description can mention raw_ostream:

This lets the LLD ELF driver be used as a library with its output written to a raw_ostream.

message calls write to stdoutOS, so you'll need to set disableOutput if you use any feature that calls message.

I think this is still fine but we need a unittest in lld/unittest/ (ninja check-lld-unit)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants