[LLD][COMMON] Fix Incorrect Stream in LLD ErrorHandler

[blackhole12]

Summary

LLD's ErrorHandler accepts a stream parameter that output may be redirected to. This can be used by programs to capture log output or redirect it somewhere else. For some reason, message() does not use this parameter, and simply outputs directly to std::cout no matter what. This patch ensure it uses the stream parameter like all the other error handling functions.

[ruiu]

I think this is intended because what message() prints out is not an error message (if it were, you should have used error()) instead.

If you had a specific reason to make this change, could you share it? We may be able to help you fix your problem.

[blackhole12]

I am calling LLD from within another command line application, and I only want to display output from LLD if there is an actual error, or the user has enabled verbose logging, but several places in LLD use message() during the linking process even if it succeeds, which will output directly to stdout with no way for me to portably redirect it (unless I use freopen, which permanently redirects it to a file, forever, and cannot be restored afterwards). If there needs to be a separate stream for non-error messages, then there should be a way to pass that into the link() function as well, instead of bypassing everything and outputting directly to stdout.

[ruiu]

In D70287#1746897, @blackhole12 wrote:

I am calling LLD from within another command line application, and I only want to display output from LLD if there is an actual error, or the user has enabled verbose logging, but several places in LLD use message() during the linking process even if it succeeds, which will output directly to stdout with no way for me to portably redirect it (unless I use freopen, which permanently redirects it to a file, forever, and cannot be restored afterwards). If there needs to be a separate stream for non-error messages, then there should be a way to pass that into the link() function as well, instead of bypassing everything and outputting directly to stdout.

I guess you are using lld as a library -- you call lld::elf::link from your program, right?

If so, it sounds like we probably should add a new parameter to lld::elf::link to specify stdout just like what we are doing for stderr.

[blackhole12]

Yes, I am calling lld::elf::link from my program, which statically links LLD as a library. I can create a change that adds a diag stream to all the linker calls, but this would touch quite a lot of files. It may be more appropriate to close this change and to create a new change for adding a new stream.

Unfortunately, I can't submit any additional patches. I have another patch for LLD that fixes a cleanup error, but https://reviews.llvm.org/differential/diff/create/ returns Unhandled Exception ("Exception") no matter how many times I log out, clear the cache, or use another browser. I tried using the command-line arcanist tool, but that doesn't seem to like my patch either. I've tried asking on IRC, but nobody seems able to help and I have no idea who to contact about this issue.

I can either replace this patch with one that adds an additional diagnostics stream, or I can wait until I figure out a way to submit new patches here.

[ruiu]

I don't know what is going on with your Phabricator account, but I can help you solve your problem. I'll make a patch to add a new parameter to the link() functions so that you can pass not only stderr but also stdout to the functions.

[grimar]

In D70287#1746923, @blackhole12 wrote:

Unfortunately, I can't submit any additional patches. I have another patch for LLD that fixes a cleanup error, but https://reviews.llvm.org/differential/diff/create/ returns Unhandled Exception ("Exception") no matter how many times I log out, clear the cache, or use another browser. I tried using the command-line arcanist tool, but that doesn't seem to like my patch either. I've tried asking on IRC, but nobody seems able to help and I have no idea who to contact about this issue.

I think I saw the similar issue earlier when my patch had no new line at EOF. At the same time sometimes you can still can see patches on phab that are posted with the same issue,
but I'd recheck if you have such things in the diff posted anyways.

[blackhole12]

Addressed by https://reviews.llvm.org/D70292

lld/Common/ErrorHandler.cpp

 
 void ErrorHandler::message(const Twine &msg) {
   std::lock_guard<std::mutex> lock(mu);
-  outs() << msg << "\n";
-  outs().flush();
+  *errorOS << msg << "\n";
+  errorOS->flush();
 }
 
 void ErrorHandler::warn(const Twine &msg) {