[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea marked an inline comment as done. aganea added inline comments. Comment at: lld/Common/ErrorHandler.cpp:78 } - _exit(val); + llvm::sys::Process::Exit(val); } rnk wrote: > This appears to have regressed shutdown. `sys::Process::Exit` calls `exit`, > not `_exit`, so now atexit destructors are run. That's unintended, right? Yes, the fix is in D88348! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
rnk added inline comments. Comment at: lld/Common/ErrorHandler.cpp:78 } - _exit(val); + llvm::sys::Process::Exit(val); } This appears to have regressed shutdown. `sys::Process::Exit` calls `exit`, not `_exit`, so now atexit destructors are run. That's unintended, right? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
ayrivera added inline comments.
Comment at: lld/ELF/Driver.cpp:895
const char *argv[] = {config->progName.data(), opt.data()};
+ cl::ResetAllOptionOccurrences();
if (cl::ParseCommandLineOptions(2, argv, "", ))
aganea wrote:
> ayrivera wrote:
> > MaskRay wrote:
> > > ayrivera wrote:
> > > > Hi,
> > > >
> > > > I built locally lld and came across an issue. The ELF driver isn't
> > > > recognizing multiple -mllvm options. It only process the last -mllvm
> > > > option that was entered in the command line. For example, I can add
> > > > -mllvm -print-after-all -debug-pass=Arguments and it only prints the
> > > > information from -debug-pass=Arguments, and not the IR and the debug
> > > > data. If I swap the order, then the IR is printed and not the debug
> > > > information.
> > > >
> > > > I noticed that this change set added a call to
> > > > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The
> > > > function parseClangOption is called inside the loop that process the
> > > > -mllvm options. If I comment this line, then everything works correctly
> > > > and I can pass multiple -mllvm options through the command.
> > > >
> > > > Is this an intended behavior or an actual issue? Thanks for your time.
> > > Hi,
> > >
> > > > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
> > >
> > > You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm
> > > -debug-pass=Arguments`. And with the option, I see the print-after-all
> > > dump.
> > Hi,
> >
> >
> > > You need two -mllvm, i.e. -mllvm -print-after-all -mllvm
> > > -debug-pass=Arguments. And with the option, I see the print-after-all
> > > dump.
> >
> >
> > Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm
> > and still got the issue.
> > Is this an intended behavior or an actual issue?
> At first sight, this is an actual issue. Normally,
> `cl::ResetAllOptionOccurrences()` should be called only once per compiler or
> linker instance, to reset the internal counters for the `cl::opt`s. I would
> move it somewhere at the begining of `readConfigs()`. Can you try that see if
> that solves your issue?
Hi @aganea ,
This patch https://reviews.llvm.org/D88461 , posted by @MaskRay , works. It
does what you suggested. Thanks for the quick reply.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea added inline comments.
Comment at: lld/ELF/Driver.cpp:895
const char *argv[] = {config->progName.data(), opt.data()};
+ cl::ResetAllOptionOccurrences();
if (cl::ParseCommandLineOptions(2, argv, "", ))
ayrivera wrote:
> MaskRay wrote:
> > ayrivera wrote:
> > > Hi,
> > >
> > > I built locally lld and came across an issue. The ELF driver isn't
> > > recognizing multiple -mllvm options. It only process the last -mllvm
> > > option that was entered in the command line. For example, I can add
> > > -mllvm -print-after-all -debug-pass=Arguments and it only prints the
> > > information from -debug-pass=Arguments, and not the IR and the debug
> > > data. If I swap the order, then the IR is printed and not the debug
> > > information.
> > >
> > > I noticed that this change set added a call to
> > > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The
> > > function parseClangOption is called inside the loop that process the
> > > -mllvm options. If I comment this line, then everything works correctly
> > > and I can pass multiple -mllvm options through the command.
> > >
> > > Is this an intended behavior or an actual issue? Thanks for your time.
> > Hi,
> >
> > > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
> >
> > You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm
> > -debug-pass=Arguments`. And with the option, I see the print-after-all dump.
> Hi,
>
>
> > You need two -mllvm, i.e. -mllvm -print-after-all -mllvm
> > -debug-pass=Arguments. And with the option, I see the print-after-all dump.
>
>
> Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm and
> still got the issue.
> Is this an intended behavior or an actual issue?
At first sight, this is an actual issue. Normally,
`cl::ResetAllOptionOccurrences()` should be called only once per compiler or
linker instance, to reset the internal counters for the `cl::opt`s. I would
move it somewhere at the begining of `readConfigs()`. Can you try that see if
that solves your issue?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
ayrivera added inline comments.
Comment at: lld/ELF/Driver.cpp:895
const char *argv[] = {config->progName.data(), opt.data()};
+ cl::ResetAllOptionOccurrences();
if (cl::ParseCommandLineOptions(2, argv, "", ))
MaskRay wrote:
> ayrivera wrote:
> > Hi,
> >
> > I built locally lld and came across an issue. The ELF driver isn't
> > recognizing multiple -mllvm options. It only process the last -mllvm option
> > that was entered in the command line. For example, I can add -mllvm
> > -print-after-all -debug-pass=Arguments and it only prints the information
> > from -debug-pass=Arguments, and not the IR and the debug data. If I swap
> > the order, then the IR is printed and not the debug information.
> >
> > I noticed that this change set added a call to
> > cl:ResetAllOptionOccurrences() (line 895) inside parseClangOption. The
> > function parseClangOption is called inside the loop that process the -mllvm
> > options. If I comment this line, then everything works correctly and I can
> > pass multiple -mllvm options through the command.
> >
> > Is this an intended behavior or an actual issue? Thanks for your time.
> Hi,
>
> > For example, I can add -mllvm -print-after-all -debug-pass=Arguments
>
> You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm
> -debug-pass=Arguments`. And with the option, I see the print-after-all dump.
Hi,
> You need two -mllvm, i.e. -mllvm -print-after-all -mllvm
> -debug-pass=Arguments. And with the option, I see the print-after-all dump.
Thanks for the quick reply. Sorry, this is a typo. I'm using both -mllvm and
still got the issue.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
MaskRay added inline comments.
Comment at: lld/ELF/Driver.cpp:895
const char *argv[] = {config->progName.data(), opt.data()};
+ cl::ResetAllOptionOccurrences();
if (cl::ParseCommandLineOptions(2, argv, "", ))
ayrivera wrote:
> Hi,
>
> I built locally lld and came across an issue. The ELF driver isn't
> recognizing multiple -mllvm options. It only process the last -mllvm option
> that was entered in the command line. For example, I can add -mllvm
> -print-after-all -debug-pass=Arguments and it only prints the information
> from -debug-pass=Arguments, and not the IR and the debug data. If I swap the
> order, then the IR is printed and not the debug information.
>
> I noticed that this change set added a call to cl:ResetAllOptionOccurrences()
> (line 895) inside parseClangOption. The function parseClangOption is called
> inside the loop that process the -mllvm options. If I comment this line, then
> everything works correctly and I can pass multiple -mllvm options through the
> command.
>
> Is this an intended behavior or an actual issue? Thanks for your time.
Hi,
> For example, I can add -mllvm -print-after-all -debug-pass=Arguments
You need two -mllvm, i.e. `-mllvm -print-after-all -mllvm
-debug-pass=Arguments`. And with the option, I see the print-after-all dump.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
ayrivera added inline comments.
Comment at: lld/ELF/Driver.cpp:895
const char *argv[] = {config->progName.data(), opt.data()};
+ cl::ResetAllOptionOccurrences();
if (cl::ParseCommandLineOptions(2, argv, "", ))
Hi,
I built locally lld and came across an issue. The ELF driver isn't recognizing
multiple -mllvm options. It only process the last -mllvm option that was
entered in the command line. For example, I can add -mllvm -print-after-all
-debug-pass=Arguments and it only prints the information from
-debug-pass=Arguments, and not the IR and the debug data. If I swap the order,
then the IR is printed and not the debug information.
I noticed that this change set added a call to cl:ResetAllOptionOccurrences()
(line 895) inside parseClangOption. The function parseClangOption is called
inside the loop that process the -mllvm options. If I comment this line, then
everything works correctly and I can pass multiple -mllvm options through the
command.
Is this an intended behavior or an actual issue? Thanks for your time.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf2efb5742cc9: [LLD][COFF] Cover usage of LLD-as-a-library in
tests (authored by aganea).
Changed prior to commit:
https://reviews.llvm.org/D70378?vs=287952=294129#toc
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
Files:
lld/COFF/Driver.cpp
lld/COFF/Writer.cpp
lld/COFF/Writer.h
lld/Common/ErrorHandler.cpp
lld/ELF/Driver.cpp
lld/MachO/Driver.cpp
lld/include/lld/Common/Driver.h
lld/include/lld/Common/ErrorHandler.h
lld/lib/Driver/DarwinLdDriver.cpp
lld/test/COFF/lit.local.cfg
lld/tools/lld/lld.cpp
lld/wasm/Driver.cpp
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -85,6 +85,8 @@
lld::stdoutOS =
lld::stderrOS =
+ errorHandler().cleanupCallback = []() { freeArena(); };
+
errorHandler().logName = args::getFilenameWithoutExe(args[0]);
errorHandler().errorLimitExceededMsg =
"too many errors emitted, stopping now (use "
@@ -103,7 +105,6 @@
if (canExitEarly)
exitLld(errorCount() ? 1 : 0);
- freeArena();
return !errorCount();
}
@@ -776,6 +777,7 @@
v.push_back("wasm-ld (LLVM option parsing)");
for (auto *arg : args.filtered(OPT_mllvm))
v.push_back(arg->getValue());
+ cl::ResetAllOptionOccurrences();
cl::ParseCommandLineOptions(v.size(), v.data());
errorHandler().errorLimit = args::getInteger(args, OPT_error_limit, 20);
Index: lld/tools/lld/lld.cpp
===
--- lld/tools/lld/lld.cpp
+++ lld/tools/lld/lld.cpp
@@ -26,6 +26,7 @@
//===--===//
#include "lld/Common/Driver.h"
+#include "lld/Common/ErrorHandler.h"
#include "lld/Common/Memory.h"
#include "llvm/ADT/STLExtras.h"
#include "llvm/ADT/SmallVector.h"
@@ -33,12 +34,19 @@
#include "llvm/ADT/Triple.h"
#include "llvm/ADT/Twine.h"
#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/CrashRecoveryContext.h"
#include "llvm/Support/Host.h"
#include "llvm/Support/InitLLVM.h"
#include "llvm/Support/Path.h"
#include "llvm/Support/PluginLoader.h"
+#include "llvm/Support/Signals.h"
#include
+#if !defined(_MSC_VER) && !defined(__MINGW32__)
+#include // for raise
+#include // for _exit
+#endif
+
using namespace lld;
using namespace llvm;
using namespace llvm::sys;
@@ -133,36 +141,103 @@
return parseProgname(arg0);
}
-// If this function returns true, lld calls _exit() so that it quickly
-// exits without invoking destructors of globally allocated objects.
-//
-// We don't want to do that if we are running tests though, because
-// doing that breaks leak sanitizer. So, lit sets this environment variable,
-// and we use it to detect whether we are running tests or not.
-static bool canExitEarly() { return StringRef(getenv("LLD_IN_TEST")) != "1"; }
-
/// Universal linker main(). This linker emulates the gnu, darwin, or
/// windows linker based on the argv[0] or -flavor option.
-int main(int argc, const char **argv) {
- InitLLVM x(argc, argv);
-
+static int lldMain(int argc, const char **argv, llvm::raw_ostream ,
+ llvm::raw_ostream , bool exitEarly = true) {
std::vector args(argv, argv + argc);
switch (parseFlavor(args)) {
case Gnu:
if (isPETarget(args))
- return !mingw::link(args, canExitEarly(), llvm::outs(), llvm::errs());
-return !elf::link(args, canExitEarly(), llvm::outs(), llvm::errs());
+ return !mingw::link(args, exitEarly, stdoutOS, stderrOS);
+return !elf::link(args, exitEarly, stdoutOS, stderrOS);
case WinLink:
-return !coff::link(args, canExitEarly(), llvm::outs(), llvm::errs());
+return !coff::link(args, exitEarly, stdoutOS, stderrOS);
case Darwin:
-return !mach_o::link(args, canExitEarly(), llvm::outs(), llvm::errs());
+return !mach_o::link(args, exitEarly, stdoutOS, stderrOS);
case DarwinNew:
-return !macho::link(args, canExitEarly(), llvm::outs(), llvm::errs());
+return !macho::link(args, exitEarly, stdoutOS, stderrOS);
case Wasm:
-return !wasm::link(args, canExitEarly(), llvm::outs(), llvm::errs());
+return !lld::wasm::link(args, exitEarly, stdoutOS, stderrOS);
default:
die("lld is a generic driver.\n"
"Invoke ld.lld (Unix), ld64.lld (macOS), lld-link (Windows), wasm-ld"
" (WebAssembly) instead");
}
}
+
+// Similar to lldMain except that exceptions are caught.
+SafeReturn lld::safeLldMain(int argc, const char **argv,
+llvm::raw_ostream ,
+llvm::raw_ostream ) {
+ int r = 0;
+ {
+// The crash recovery is here only to be able to recover from arbitrary
+// control flow when fatal() is called
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
rnk accepted this revision. rnk added a comment. Looks good to me, I didn't review very in depth, but I see the test case that we need. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
amccarth accepted this revision. amccarth added a comment. LGTM. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
MaskRay added a comment. @rnk @amccarth Do you have more comments? ☺️ Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea added a comment. @MaskRay Thank you! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
MaskRay accepted this revision. MaskRay added a comment. Looks great! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea added a comment. Ping! @MaskRay any further comments? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea added inline comments. Comment at: clang/tools/driver/driver.cpp:537 + // When running in integrated-cc1 mode, the CrashRecoveryContext returns + // the same code as if the program crashed. On Unix, that is codes >128. + IsCrash |= CommandRes > 128; MaskRay wrote: > (From a non-native speaker) > > "that is codes >128" Should the singular form be used? Rephrased. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea updated this revision to Diff 287952.
aganea marked 2 inline comments as done.
aganea added a comment.
Address comments.
Added a CrashRecoveryTest.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
Files:
clang/tools/driver/driver.cpp
lld/COFF/Driver.cpp
lld/COFF/Writer.cpp
lld/COFF/Writer.h
lld/Common/ErrorHandler.cpp
lld/ELF/Driver.cpp
lld/MachO/Driver.cpp
lld/include/lld/Common/Driver.h
lld/include/lld/Common/ErrorHandler.h
lld/lib/Driver/DarwinLdDriver.cpp
lld/test/COFF/dll.test
lld/test/COFF/guardcf-lto.ll
lld/test/COFF/lit.local.cfg
lld/tools/lld/lld.cpp
lld/wasm/Driver.cpp
llvm/include/llvm/Support/CrashRecoveryContext.h
llvm/include/llvm/Support/Signals.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/Unix/Signals.inc
llvm/lib/Support/Windows/Signals.inc
llvm/unittests/Support/CrashRecoveryTest.cpp
Index: llvm/unittests/Support/CrashRecoveryTest.cpp
===
--- llvm/unittests/Support/CrashRecoveryTest.cpp
+++ llvm/unittests/Support/CrashRecoveryTest.cpp
@@ -109,4 +109,11 @@
EXPECT_TRUE(CrashRecoveryContext().RunSafely(outputString));
}
+TEST(CrashRecoveryTest, Abort) {
+ llvm::CrashRecoveryContext::Enable();
+ auto A = []() { abort(); };
+ EXPECT_FALSE(CrashRecoveryContext().RunSafely(A));
+ // Test a second time to ensure we reinstall the abort signal handler.
+ EXPECT_FALSE(CrashRecoveryContext().RunSafely(A));
+}
#endif
Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -868,3 +868,5 @@
#pragma GCC diagnostic warning "-Wformat"
#pragma GCC diagnostic warning "-Wformat-extra-args"
#endif
+
+void sys::unregisterHandlers() {}
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -331,7 +331,7 @@
registerHandler(S, SignalKind::IsInfo);
}
-static void UnregisterHandlers() {
+void sys::unregisterHandlers() {
// Restore all of the signal handlers to how they were before we showed up.
for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@
// crashes when we return and the signal reissues. This also ensures that if
// we crash in our signal handler that the program will terminate immediately
// instead of recursing in the signal handler.
- UnregisterHandlers();
+ sys::unregisterHandlers();
// Unmask all potentially blocked kill signals.
sigset_t SigMask;
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -97,6 +97,13 @@
CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
+CrashRecoveryContext::CrashRecoveryContext() {
+ // On Windows, if abort() was previously triggered (and caught by a previous
+ // CrashRecoveryContext) the Windows CRT removes our installed signal handler,
+ // so we need to install it again.
+ sys::DisableSystemDialogsOnCrash();
+}
+
CrashRecoveryContext::~CrashRecoveryContext() {
// Reclaim registered resources.
CrashRecoveryContextCleanup *i = head;
@@ -370,9 +377,10 @@
sigaddset(, Signal);
sigprocmask(SIG_UNBLOCK, , nullptr);
- // As per convention, -2 indicates a crash or timeout as opposed to failure to
- // execute (see llvm/include/llvm/Support/Program.h)
- int RetCode = -2;
+ // Return the same error code as if the program crashed, as mentioned in the
+ // section "Exit Status for Commands":
+ // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+ int RetCode = 128 + Signal;
// Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
if (Signal == SIGPIPE)
Index: llvm/include/llvm/Support/Signals.h
===
--- llvm/include/llvm/Support/Signals.h
+++ llvm/include/llvm/Support/Signals.h
@@ -115,6 +115,8 @@
/// Context is a system-specific failure context: it is the signal type on
/// Unix; the ExceptionContext on Windows.
void CleanupOnSignal(uintptr_t Context);
+
+ void unregisterHandlers();
} // End sys namespace
} // End llvm namespace
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -44,11 +44,11 @@
/// executed in any case, whether crash occurs or not. These actions may be used
/// to reclaim resources in the case of crash.
class CrashRecoveryContext {
-
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
MaskRay added inline comments. Comment at: clang/tools/driver/driver.cpp:537 + // When running in integrated-cc1 mode, the CrashRecoveryContext returns + // the same code as if the program crashed. On Unix, that is codes >128. + IsCrash |= CommandRes > 128; (From a non-native speaker) "that is codes >128" Should the singular form be used? Comment at: llvm/include/llvm/Support/Signals.h:119 + + void UnregisterHandlers(); } // End sys namespace `[readability-identifier-naming]` Many very old function use UpperCase, but newer functions should use lowerCase. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70378/new/ https://reviews.llvm.org/D70378 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D70378: [LLD][COFF] Cover usage of LLD as a library
aganea updated this revision to Diff 287747.
aganea marked an inline comment as done.
aganea added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
Rebase.
Address @MaskRay's suggestions.
Made `safeLldLink` a public API and made it possible to provide user-defined
stdout/stderr streams.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D70378/new/
https://reviews.llvm.org/D70378
Files:
clang/tools/driver/driver.cpp
lld/COFF/Driver.cpp
lld/COFF/Writer.cpp
lld/COFF/Writer.h
lld/Common/ErrorHandler.cpp
lld/ELF/Driver.cpp
lld/MachO/Driver.cpp
lld/include/lld/Common/Driver.h
lld/include/lld/Common/ErrorHandler.h
lld/lib/Driver/DarwinLdDriver.cpp
lld/test/COFF/dll.test
lld/test/COFF/guardcf-lto.ll
lld/test/COFF/lit.local.cfg
lld/tools/lld/lld.cpp
lld/wasm/Driver.cpp
llvm/include/llvm/Support/CrashRecoveryContext.h
llvm/include/llvm/Support/Signals.h
llvm/lib/Support/CrashRecoveryContext.cpp
llvm/lib/Support/Unix/Signals.inc
llvm/lib/Support/Windows/Signals.inc
Index: llvm/lib/Support/Windows/Signals.inc
===
--- llvm/lib/Support/Windows/Signals.inc
+++ llvm/lib/Support/Windows/Signals.inc
@@ -868,3 +868,5 @@
#pragma GCC diagnostic warning "-Wformat"
#pragma GCC diagnostic warning "-Wformat-extra-args"
#endif
+
+void sys::UnregisterHandlers() {}
Index: llvm/lib/Support/Unix/Signals.inc
===
--- llvm/lib/Support/Unix/Signals.inc
+++ llvm/lib/Support/Unix/Signals.inc
@@ -331,7 +331,7 @@
registerHandler(S, SignalKind::IsInfo);
}
-static void UnregisterHandlers() {
+void sys::UnregisterHandlers() {
// Restore all of the signal handlers to how they were before we showed up.
for (unsigned i = 0, e = NumRegisteredSignals.load(); i != e; ++i) {
sigaction(RegisteredSignalInfo[i].SigNo,
@@ -367,7 +367,7 @@
// crashes when we return and the signal reissues. This also ensures that if
// we crash in our signal handler that the program will terminate immediately
// instead of recursing in the signal handler.
- UnregisterHandlers();
+ sys::UnregisterHandlers();
// Unmask all potentially blocked kill signals.
sigset_t SigMask;
Index: llvm/lib/Support/CrashRecoveryContext.cpp
===
--- llvm/lib/Support/CrashRecoveryContext.cpp
+++ llvm/lib/Support/CrashRecoveryContext.cpp
@@ -97,6 +97,13 @@
CrashRecoveryContextCleanup::~CrashRecoveryContextCleanup() {}
+CrashRecoveryContext::CrashRecoveryContext() {
+ // On Windows, if abort() was previously triggered (and caught by a previous
+ // CrashRecoveryContext) the Windows CRT removes our installed signal handler,
+ // so we need to install it again.
+ sys::DisableSystemDialogsOnCrash();
+}
+
CrashRecoveryContext::~CrashRecoveryContext() {
// Reclaim registered resources.
CrashRecoveryContextCleanup *i = head;
@@ -370,9 +377,10 @@
sigaddset(, Signal);
sigprocmask(SIG_UNBLOCK, , nullptr);
- // As per convention, -2 indicates a crash or timeout as opposed to failure to
- // execute (see llvm/include/llvm/Support/Program.h)
- int RetCode = -2;
+ // Return the same error code as if the program crashed, as mentioned in the
+ // section "Exit Status for Commands":
+ // https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xcu_chap02.html
+ int RetCode = 128 + Signal;
// Don't consider a broken pipe as a crash (see clang/lib/Driver/Driver.cpp)
if (Signal == SIGPIPE)
Index: llvm/include/llvm/Support/Signals.h
===
--- llvm/include/llvm/Support/Signals.h
+++ llvm/include/llvm/Support/Signals.h
@@ -115,6 +115,8 @@
/// Context is a system-specific failure context: it is the signal type on
/// Unix; the ExceptionContext on Windows.
void CleanupOnSignal(uintptr_t Context);
+
+ void UnregisterHandlers();
} // End sys namespace
} // End llvm namespace
Index: llvm/include/llvm/Support/CrashRecoveryContext.h
===
--- llvm/include/llvm/Support/CrashRecoveryContext.h
+++ llvm/include/llvm/Support/CrashRecoveryContext.h
@@ -44,11 +44,11 @@
/// executed in any case, whether crash occurs or not. These actions may be used
/// to reclaim resources in the case of crash.
class CrashRecoveryContext {
- void *Impl;
- CrashRecoveryContextCleanup *head;
+ void *Impl = nullptr;
+ CrashRecoveryContextCleanup *head = nullptr;
public:
- CrashRecoveryContext() : Impl(nullptr), head(nullptr) {}
+ CrashRecoveryContext();
~CrashRecoveryContext();
/// Register cleanup handler, which is used when the recovery context is
Index: lld/wasm/Driver.cpp
===
--- lld/wasm/Driver.cpp
+++ lld/wasm/Driver.cpp
@@ -85,6 +85,8 @@
