[Error] Add stack traces for llvm::Error invariant violations.

[lhames]

Summary

This patch adds stack traces for two llvm::Error invariant violations,
dropped errors and failure values passed to cantFail calls. In each
case the stack trace of the failure point will now be printed. E.g.

"""
Failure value returned from cantFail wrapped call
foo
cantFail called from:
0 error-backtrace-test 0x0000000100e220a3 llvm::sys::
PrintStackTrace(llvm::raw_ostream&) + 307
1 error-backtrace-test 0x0000000100d4c286 llvm::detail::
fatalCantFailUnhandledError(llvm::Error, char const*) + 614
2 error-backtrace-test 0x0000000100ea475a llvm::
cantFail(llvm::Error, char const*) + 458
3 error-backtrace-test 0x0000000100ea4517 foo() + 231
4 error-backtrace-test 0x0000000100ea49d1 main + 497
5 libdyld.dylib 0x00007fff5c321011 start + 1
"""

If the LLVM_ENABLE_ERROR_TRACING environment variable is set (and the Error
was a failure value) then a second trace will be printed before the first
showing where the error originated:

"""
Error thrown from:
0 error-backtrace-test 0x00000001016f20a3 llvm::sys::
PrintStackTrace(llvm::raw_ostream&) + 307
1 error-backtrace-test 0x000000010161b206 llvm::detail::
annotateErrorWithTrace(llvm::ErrorInfoBase&) + 278
2 error-backtrace-test 0x00000001017742fb llvm::Error
llvm::make_error<llvm::StringError, char const (&) [4], std::1::error_code>(
char const (&) [4], std::1::error_code&&) + 683
3 error-backtrace-test 0x0000000101773fcc bar() + 412
4 error-backtrace-test 0x0000000101774508 foo() + 216
5 error-backtrace-test 0x00000001017749d1 main + 497
6 libdyld.dylib 0x00007fff5c321011 start + 1
7 libdyld.dylib 0x0000000000000001 start + 18446603338969378801
"""

There are no performance implications for the first stack trace (to the failure
point) since printing only happens on invariant violation where we were about
to abort anyway.

The second scheme does have performance implications, however I believe the
trade-offs are reasonable:

• Success values are unaffected.
• When the flag is off the impact for each failure value is two null-checks: One at failure value construction time, and one at failure value destruction time.
• When the flag is on the impact for each failure value is one stack trace and two map operations: One to store the trace and a second to discard it.

This scheme is intended as a generalization of the proposals in
https://reviews.llvm.org/D70259 and https://reviews.llvm.org/D70263 . The aim
is to make debugging of Error handling failures easier, including when working
with remote JITs where logging may be substantially easier than attaching a
debugger.

Known outstanding issues:

• LLVM's stack traces don't include file and line info on Darwin yet. I am investigating that now.
• Test cases needed.

[lhames]

• Replace report_fatal_error declaration that was accidentally removed.

[dblaikie]

llvm/include/llvm/Support/Error.h

762–763

Drop the else here & similar places ( https://llvm.org/docs/CodingStandards.html#don-t-use-else-after-a-return )

llvm/lib/Support/Error.cpp

73–74

Two map lookups - better to use find & check for end to do only one map lookup.

Actually DenseMap's 'lookup' might suffice.

86–105

I was going to ask - why all this rather than just asserting and letting the existing crash reporting stuff work. But I guess that crash reporting stuff only works for clang but not other LLVM tools? Perhaps it could be made to work? (though perhaps that's too much work - guess it involves reserving memory for use safely during crashes, etc))

Hmm, I guess it's not entirely about the clang crash reporter - a simple assert failure in llvm-dwarfdump does at least print the stack trace:

llvm-dwarfdump-tot: /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:632: int main(int, char **): Assertion `false' failed.
Stack dump:
0.      Program arguments: llvm-dwarfdump-tot -debug-info -v loc.o 
 #0 0x000000000145b977 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:548:11
 #1 0x000000000145bb19 PrintStackTraceSignalHandler(void*) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:609: 1

So, yeah - why not just an assert? (or wasn't that what the failure mode was before this change)

Oh, I see it was unreachable and still has an unreachable - that might make this error message a bit redundant - printing the stack trace & then unreachable, which will print the stack trace again?

(the ErrorTraces part is value-add/new functionality here, for sure & assert/unreachable wouldn't ever provide that functionality of course)

175

This abort'll also print the stack trace - so seems that the "PrintStackTrace(dbgs())" would be redundant with this as well?

llvm/lib/Support/InitLLVM.cpp

32–34

If the code for this is always going to be present - what's the benefit from using a side table rather than making Error itself a little bigger (a pointer) & skip the side table/global state/global ctors/dtors? It's not like storing lots of Errors is a supported scenario, so I doubt an extra pointer in an Error would significantly increase memory usage at any point?

[lhames]

llvm/include/llvm/Support/Error.h

762–763

Yep.

llvm/lib/Support/Error.cpp

73–74

Yep.

86–105

I was going to ask - why all this rather than just asserting and letting the existing crash reporting stuff work.

The benefit to printing ourselves is that we can disambiguate the stack traces ("this is for the error, this is for where you dropped it") and ensure (by calling PrintStackTrace manually) that these two things are rendered side-by-side. I have not satisfied myself that assertions will always generate a similar trace (and not be intercepted by some crash logging tool) yet, but if that's the case then we can just do that.

175

Yep -- error message polish is to-do here. We do want to avoid redundant traces.

llvm/lib/Support/InitLLVM.cpp

32–34

It's sort of much-of-a-muchness. We do support multiple in-flight errors, so I like the idea of not increasing the size of ErrorInfo for something that won't be used 99.9% of the time.

[dblaikie]

llvm/lib/Support/Error.cpp

86–105

Yeah, the current error message behavior I'm getting is a bit more than desirable, I imagine (the "discarded from" and "?

Program aborted due to an unhandled Error. Error message:
"version 6 of .debug_addr section at offset 0x0 is not supported"
Error thrown from:
#0 0x00000000014751a7 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Sign
als.inc:548:11
#1 0x00000000013dd931 llvm::detail::annotateErrorWithTrace(llvm::ErrorInfoBase&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Suppo
rt/Error.cpp:64:3
#2 0x0000000000d098d5 llvm::Error llvm::make_error<llvm::StringError, std::cxx11::basic_string<char, std::char_traits<char>, std::allocat
or<char> >&, std::error_code&>(std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >&, std::error_code&) /usr/loca
l/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:356:16
#3 0x0000000000d2de7b llvm::Error llvm::createStringError<unsigned short, unsigned long>(std::error_code, char const*, unsigned short const
&, unsigned long const&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:1211:1
#4 0x0000000000d2d231 llvm::DWARFDebugAddrTable::extract(llvm::DWARFDataExtractor, unsigned long*, unsigned short, unsigned char, std::func
tion<void (llvm::Error)>) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFDebugAddr.cpp:90:5
#5 0x0000000000cf470e dumpAddrSection(llvm::raw_ostream&, llvm::DWARFDataExtractor&, llvm::DIDumpOptions, unsigned short, unsigned char) /u
sr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:250:21
#6 0x0000000000cf1a9c llvm::DWARFContext::dump(llvm::raw_ostream&, llvm::DIDumpOptions, std::array<llvm::Optional<unsigned long>, 28ul>) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:537:18
#7 0x0000000000cbe626 dumpObjectFile(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:447:3
#8 0x0000000000cd83ed std::_Function_handler<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&), bool (*)(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::_M_invoke(std::_Any_data const&, llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine&&, llvm::raw_ostream&) /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:282:2
#9 0x0000000000cc9ade std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::operator()(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) const /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:687:7
#10 0x0000000000cbe90d handleBuffer(llvm::StringRef, llvm::MemoryBufferRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:494:14
#11 0x0000000000cbe131 handleFile(llvm::StringRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:528:3
#12 0x0000000000cbda32 main /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:643:7
#13 0x00007f187e56552b libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2352b)
#14 0x0000000000caf02a _start (/usr/local/google/home/blaikie/dev/llvm/build/default/bin/llvm-dwarfdump+0xcaf02a)
Error discarded from:
#0 0x00000000014751a7 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:548:11
#1 0x00000000013de260 llvm::Error::fatalUncheckedError() const /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Error.cpp:172:3
#2 0x0000000000cb517a llvm::Error::assertIsChecked() /usr/local/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:286:3
#3 0x0000000000cb3f3c llvm::Error::~Error() /usr/local/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:245:5
#4 0x0000000000cf47e2 dumpAddrSection(llvm::raw_ostream&, llvm::DWARFDataExtractor&, llvm::DIDumpOptions, unsigned short, unsigned char) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:250:15
#5 0x0000000000cf1a9c llvm::DWARFContext::dump(llvm::raw_ostream&, llvm::DIDumpOptions, std::array<llvm::Optional<unsigned long>, 28ul>) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:537:18
#6 0x0000000000cbe626 dumpObjectFile(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:447:3
#7 0x0000000000cd83ed std::_Function_handler<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&), bool (*)(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::_M_invoke(std::_Any_data const&, llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine&&, llvm::raw_ostream&) /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:282:2
#8 0x0000000000cc9ade std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::operator()(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) const /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:687:7
#9 0x0000000000cbe90d handleBuffer(llvm::StringRef, llvm::MemoryBufferRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:494:14
#10 0x0000000000cbe131 handleFile(llvm::StringRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:528:3
#11 0x0000000000cbda32 main /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:643:7
#12 0x00007f187e56552b libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2352b)
#13 0x0000000000caf02a _start (/usr/local/google/home/blaikie/dev/llvm/build/default/bin/llvm-dwarfdump+0xcaf02a)
Stack dump:
0. Program arguments: /usr/local/google/home/blaikie/dev/llvm/build/default/bin/llvm-dwarfdump -debug-addr x.o
#0 0x00000000014751a7 llvm::sys::PrintStackTrace(llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:548:11
#1 0x0000000001475349 PrintStackTraceSignalHandler(void*) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:609:1
#2 0x0000000001473c2b llvm::sys::RunSignalHandlers() /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Signals.cpp:67:5
#3 0x0000000001475ac5 SignalHandler(int) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/Support/Unix/Signals.inc:390:1
#4 0x00007f187f8553a0 restore_rt (/lib/x86_64-linux-gnu/libpthread.so.0+0x123a0)
#5 0x00007f187e578cfb raise (/lib/x86_64-linux-gnu/libc.so.6+0x36cfb)
#6 0x00007f187e5638ad abort (/lib/x86_64-linux-gnu/libc.so.6+0x218ad)
#7 0x00000000013de27e (/usr/local/google/home/blaikie/dev/llvm/build/default/bin/llvm-dwarfdump+0x13de27e)
#8 0x0000000000cb517a llvm::Error::assertIsChecked() /usr/local/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:286:3
#9 0x0000000000cb3f3c llvm::Error::~Error() /usr/local/google/home/blaikie/dev/llvm/src/llvm/include/llvm/Support/Error.h:245:5
#10 0x0000000000cf47e2 dumpAddrSection(llvm::raw_ostream&, llvm::DWARFDataExtractor&, llvm::DIDumpOptions, unsigned short, unsigned char) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:250:15
#11 0x0000000000cf1a9c llvm::DWARFContext::dump(llvm::raw_ostream&, llvm::DIDumpOptions, std::array<llvm::Optional<unsigned long>, 28ul>) /usr/local/google/home/blaikie/dev/llvm/src/llvm/lib/DebugInfo/DWARF/DWARFContext.cpp:537:18
#12 0x0000000000cbe626 dumpObjectFile(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:447:3
#13 0x0000000000cd83ed std::_Function_handler<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&), bool (*)(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::_M_invoke(std::_Any_data const&, llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine&&, llvm::raw_ostream&) /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:282:2
#14 0x0000000000cc9ade std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>::operator()(llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&) const /usr/local/google/home/blaikie/install/bin/../lib/gcc/x86_64-pc-linux-gnu/8.1.0/../../../../include/c++/8.1.0/bits/std_function.h:687:7
#15 0x0000000000cbe90d handleBuffer(llvm::StringRef, llvm::MemoryBufferRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:494:14
#16 0x0000000000cbe131 handleFile(llvm::StringRef, std::function<bool (llvm::object::ObjectFile&, llvm::DWARFContext&, llvm::Twine, llvm::raw_ostream&)>, llvm::raw_ostream&) /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:528:3
#17 0x0000000000cbda32 main /usr/local/google/home/blaikie/dev/llvm/src/llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:643:7
#18 0x00007f187e56552b libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x2352b)
#19 0x0000000000caf02a _start (/usr/local/google/home/blaikie/dev/llvm/build/default/bin/llvm-dwarfdump+0xcaf02a)
Aborted

(wonder if there's anything we can do about making these things more readable in general - line wrapping, colours, etc? even just some blank lines/bigger/bolder headers at the start of each stack trace might be helpful - but that sort of thing probably wouldn't be in this patch, of course)

If you're set on not producing a separate/duplicate stack trace from the one general stack dump one, fair enough/guess I can't talk you out of it & follow up patches could help make it more legible, for sure. (trimming the top few stack frames to get back up into user code/through the llvm::Error implementation details (something the generic code can't reasonably do), maybe pruning the common stack frames between the thrown/dropped stack? etc)

llvm/lib/Support/InitLLVM.cpp

32–34

I'd probably lean towards adding the extra pointer until there's a real use case for Error that has scaling problems because of the size rather than adding the side table now. Global variables/global ctors/dtors are something I think we're generally trying to avoid in LLVM (so users of LLVM don't have to pay for things they don't need - in the form of global ctors that hurt those users startup time) (though the cl::opt is the biggest perpetrator & hasn't been addressed yet).

Not super relevant, but FWIW, assigning to and reading from a unique_ptr isn't multithread safe - so you will have to make sure you call enableErrorTracing before any threads start that might create errors.

[lhames]

llvm/lib/Support/Error.cpp

86–105

Yeah, the current error message behavior I'm getting is a bit more than desirable...

I think stack traces are often long, and since this involves two it's going to be twice as long. I wonder if adding more verbiage at the top might actually help in this case, especially for people who will encounter this infrequently:

"Program crashed due to an unhandled error. The following two stack traces describe where the unhandled error was generated, and where it was discarded while in the unchecked state:"

I love the idea of adding line wrapping/colors to make this more readable, but I think that should just be applied to stack traces in general. I don't think it's specific to this patch.

Trimming the top few frames sounds *excellent*. I was thinking of proposing that myself. What about a "DropNFrames[=0]" argument to sys::PrintStackTrace for people who know their call to PrintStackTrace will sit some number of frames inside some uninteresting error handling machinery?

I love the idea of pruning common frames, but I can't figure out how to render it in a way that won't end up taking more cognitive effort to parse than just presenting two traces. Very happy to take suggestions on it, but I think it's something we could address in a follow-up.

llvm/lib/Support/InitLLVM.cpp

32–34

I'd be happy to switch it to a ManagedStatic (which I believe doesn't have static ctors/dtors) or a custom global with manual management and a zero-init, but I do want to keep it as a side-table.

Thanks for pointing out the unique_ptr thread safety issue. That's another reason to move away from unique_ptr here.

[sammccall]

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

[lhames]

In D70600#1801726, @sammccall wrote:

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way. I like the idea that the side table can be entirely compiled out in release mode (if we decide to go that way) without changing the shape of any error types or leaving any dead fields.

[lhames]

Oh, I remember part of my motivation for designing it this way: While failure values are expected to be expensive, we don't want to make them unnecessarily expensive, which is why the error-geneation-point trace is under a flag: You don't want to stack-trace every error that is ever generated. However, making this conditional means we need to check a global flag to decide whether we *should* do the stack trace. Once you're doing that, you may as well make your "flag" a pointer to the side table.

[dblaikie]

In D70600#1802235, @lhames wrote:

Oh, I remember part of my motivation for designing it this way: While failure values are expected to be expensive, we don't want to make them unnecessarily expensive, which is why the error-geneation-point trace is under a flag: You don't want to stack-trace every error that is ever generated. However, making this conditional means we need to check a global flag to decide whether we *should* do the stack trace. Once you're doing that, you may as well make your "flag" a pointer to the side table.

Oh, totally all for the conditional stack trace generation, and we've got lots of flags (cl::opts, env variable tests, some straight up global variables (the latter being fairly rare, but I think there's some precedent for them in LLVM?)) - but I think there's significantly (that's certainly a value judgment - impossible to quantify) complexity in having a mutable side table for those stack traces - so, for me at least, one doesn't follow from the other necessarily.

[sammccall]

In D70600#1802227, @lhames wrote:

In D70600#1801726, @sammccall wrote:

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way.

I don't follow this - are you saying if heap allocation is OK, locking a mutex must be too, as it's cheaper?
Locking/unlocking a shared global mutex can certainly be more expensive than malloc if there's a lot of contention. malloc implementations typically go to some lengths to avoid a global lock for this reason.

Example workload: clangd typically runs ~30 threads for background indexing, each thread frequently calls functions returning Expected<T> and recovers from errors (Error is documented as being for recoverable errors). It'd be a big surprise if this sort of use of Error led to contention/serialization on these worker threads.

[dblaikie]

In D70600#1802516, @sammccall wrote:

In D70600#1802227, @lhames wrote:

In D70600#1801726, @sammccall wrote:

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way.

I don't follow this - are you saying if heap allocation is OK, locking a mutex must be too, as it's cheaper?
Locking/unlocking a shared global mutex can certainly be more expensive than malloc if there's a lot of contention. malloc implementations typically go to some lengths to avoid a global lock for this reason.

Example workload: clangd typically runs ~30 threads for background indexing, each thread frequently calls functions returning Expected<T> and recovers from errors (Error is documented as being for recoverable errors). It'd be a big surprise if this sort of use of Error led to contention/serialization on these worker threads.

Note this feature's only intended to be opt-in for debugging purposes, so for production use cases it shouldn't be a huge deal (though could be cheaper/free if it was a build-time setting rather than something you could opt into/out of at runtime - perhaps that's the point you're making? Though we do check various globals (cl::opts, etc) in a fair number of codepaths, I think, such that adding one doesn't seem likely to be super problematic?). (as much as I agree with you that I'd rather not have a side table)

[hintonda]

In D70600#1803789, @dblaikie wrote:

In D70600#1802516, @sammccall wrote:

In D70600#1802227, @lhames wrote:

In D70600#1801726, @sammccall wrote:

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way.

I don't follow this - are you saying if heap allocation is OK, locking a mutex must be too, as it's cheaper?
Locking/unlocking a shared global mutex can certainly be more expensive than malloc if there's a lot of contention. malloc implementations typically go to some lengths to avoid a global lock for this reason.

Example workload: clangd typically runs ~30 threads for background indexing, each thread frequently calls functions returning Expected<T> and recovers from errors (Error is documented as being for recoverable errors). It'd be a big surprise if this sort of use of Error led to contention/serialization on these worker threads.

Note this feature's only intended to be opt-in for debugging purposes, so for production use cases it shouldn't be a huge deal (though could be cheaper/free if it was a build-time setting rather than something you could opt into/out of at runtime - perhaps that's the point you're making? Though we do check various globals (cl::opts, etc) in a fair number of codepaths, I think, such that adding one doesn't seem likely to be super problematic?). (as much as I agree with you that I'd rather not have a side table)

Part of the reason for this patch was a need to provide file/line number for errors that lead to aborts via llvm::cantFail. This can be very important when debugging failing bots for which you have not access and are unable to reproduce. Since this solution is too expensive to turn on all the time, it's doubtful it'll be enabled on any bots, which makes it useless for my primary use case.

That doesn't mean it wouldn't be useful in cases where you can reproduce locally, but that wasn't my initial motivation. I'm also wary of the side-table.

[dblaikie]

In D70600#1803899, @hintonda wrote:

In D70600#1803789, @dblaikie wrote:

In D70600#1802516, @sammccall wrote:

In D70600#1802227, @lhames wrote:

In D70600#1801726, @sammccall wrote:

FWIW working on a codebase that heavily uses both Error and threads, the side-table seems fairly a bit terrifying. Even if the thread safety issues are fixed, do we really want to pay for mutex lock/unlock to create errors, to save a pointer?

thread_local might be an option, though as I understand llvm/Support may have to be portable to environments where that doesn't work well.

An atomic pointer would work too. This cost is only paid on failure values and these are already heap allocated, so I'm I wouldn't expect the overhead to be prohibitive either way.

I don't follow this - are you saying if heap allocation is OK, locking a mutex must be too, as it's cheaper?
Locking/unlocking a shared global mutex can certainly be more expensive than malloc if there's a lot of contention. malloc implementations typically go to some lengths to avoid a global lock for this reason.

Example workload: clangd typically runs ~30 threads for background indexing, each thread frequently calls functions returning Expected<T> and recovers from errors (Error is documented as being for recoverable errors). It'd be a big surprise if this sort of use of Error led to contention/serialization on these worker threads.

Note this feature's only intended to be opt-in for debugging purposes, so for production use cases it shouldn't be a huge deal (though could be cheaper/free if it was a build-time setting rather than something you could opt into/out of at runtime - perhaps that's the point you're making? Though we do check various globals (cl::opts, etc) in a fair number of codepaths, I think, such that adding one doesn't seem likely to be super problematic?). (as much as I agree with you that I'd rather not have a side table)

Part of the reason for this patch was a need to provide file/line number for errors that lead to aborts via llvm::cantFail. This can be very important when debugging failing bots for which you have not access and are unable to reproduce. Since this solution is too expensive to turn on all the time, it's doubtful it'll be enabled on any bots, which makes it useless for my primary use case.

That doesn't mean it wouldn't be useful in cases where you can reproduce locally, but that wasn't my initial motivation. I'm also wary of the side-table.

Sorry - I meant opt-in for production use cases. I think it'd make a lot of sense to enable it for buildbots (same way that we enable assertions on buildbots - but you'd never want assertions enabled in your clang production build, it'd be much too slow). But I'd like to see some functionality to specifically disable all crash symbolication for crash-expected test cases (GUnit EXPECT_DEATH) - those crashes aren't reported anyway (gunit swallows the output of the crashing program).

llvm/include/llvm/Support/Error.h

 //===- llvm/Support/Error.h - Recoverable error handling --------*- C++ -*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
 // This file defines an API used to report recoverable errors.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_SUPPORT_ERROR_H
 #define LLVM_SUPPORT_ERROR_H
 
 #include "llvm-c/Error.h"
+#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Config/abi-breaking.h"
 #include "llvm/Support/AlignOf.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/Format.h"
+#include "llvm/Support/Signals.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cassert>
 #include <cstdint>
 #include <cstdlib>
 #include <functional>
 #include <memory>
 #include <new>
 #include <string>
 #include <system_error>
 #include <type_traits>
 #include <utility>
 #include <vector>
 
 namespace llvm {
 
+class ErrorInfoBase;
+
+namespace detail {
+using ErrorTraceMap = DenseMap<ErrorInfoBase *, std::string>;
+extern std::unique_ptr<ErrorTraceMap> ErrorTraces;
+
+void clearErrorTrace(ErrorInfoBase &);
+
+void annotateErrorWithTrace(ErrorInfoBase &EI);
+
+} // namespace detail
+
 class ErrorSuccess;
 
 /// Base class for error info classes. Do not extend this directly: Extend
 /// the ErrorInfo template subclass instead.
 class ErrorInfoBase {
 public:
-  virtual ~ErrorInfoBase() = default;
+  virtual ~ErrorInfoBase() {
+    if (detail::ErrorTraces)
+      detail::clearErrorTrace(*this);
+  }
 
   /// Print an error message to an output stream.
   virtual void log(raw_ostream &OS) const = 0;
 
   /// Return the error message as a string.
   virtual std::string message() const {
     std::string Msg;
     raw_string_ostream OS(Msg);

 
 /// Subclass of Error for the sole purpose of identifying the success path in
 /// the type system. This allows to catch invalid conversion to Expected<T> at
 /// compile time.
 class ErrorSuccess final : public Error {};
 
 inline ErrorSuccess Error::success() { return ErrorSuccess(); }
 
+/// Enable error tracing for debugging.
+/// This function should be called during program initialization.
+void enableErrorTracing();
+
 /// Make a Error instance representing failure using the given error info
 /// type.
 template <typename ErrT, typename... ArgTs> Error make_error(ArgTs &&... Args) {
-  return Error(std::make_unique<ErrT>(std::forward<ArgTs>(Args)...));
+
+  auto EI = std::make_unique<ErrT>(std::forward<ArgTs>(Args)...);
+  if (detail::ErrorTraces)
+    detail::annotateErrorWithTrace(*EI);
+  return Error(std::move(EI));
 }
 
 /// Base class for user error types. Users should declare their error types
 /// like:
 ///
 /// class MyError : public ErrorInfo<MyError> {
 ///   ....
 /// };
void fatalUncheckedExpected() const {
     dbgs() << "Expected<T> must be checked before access or destruction.\n";
     if (HasError) {
       dbgs() << "Unchecked Expected<T> contained error:\n";
       (*getErrorStorage())->log(dbgs());
     } else
       dbgs() << "Expected<T> value was in success state. (Note: Expected<T> "
                 "values in success mode must still be checked prior to being "
                 "destroyed).\n";
+    dbgs() << "Expected value discarded from:\n";
+    sys::PrintStackTrace(dbgs());
     abort();
   }
 #endif
 
   void assertIsChecked() {
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
     if (LLVM_UNLIKELY(Unchecked))
       fatalUncheckedExpected();

 #endif
 };
 
 /// Report a serious error, calling any installed error handler. See
 /// ErrorHandling.h.
 LLVM_ATTRIBUTE_NORETURN void report_fatal_error(Error Err,
                                                 bool gen_crash_diag = true);
 
+namespace detail {
+
+/// Utility function for cantFail.
+LLVM_ATTRIBUTE_NORETURN
+LLVM_ATTRIBUTE_NOINLINE
+void fatalCantFailUnhandledError(Error, const char *);
+
+} // end namespace detail
+
 /// Report a fatal error if Err is a failure value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
 /// is known that the Error will always be a success value. E.g.
 ///
 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns
 ///   // Error::success().
 ///   Error foo(bool DoFallibleOperation);
 ///
 ///   cantFail(foo(false));
 ///   @endcode
 inline void cantFail(Error Err, const char *Msg = nullptr) {
-  if (Err) {
-    if (!Msg)
-      Msg = "Failure value returned from cantFail wrapped call";
-#ifndef NDEBUG
-    std::string Str;
-    raw_string_ostream OS(Str);
-    OS << Msg << "\n" << Err;
-    Msg = OS.str().c_str();
-#endif
-    llvm_unreachable(Msg);
-  }
+  if (LLVM_UNLIKELY(Err))
+    detail::fatalCantFailUnhandledError(std::move(Err), Msg);
 }
 
 /// Report a fatal error if ValOrErr is a failure value, otherwise unwraps and
 /// returns the contained value.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
 /// is known that the Error will always be a success value. E.g.
 ///
 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns an int.
 ///   Expected<int> foo(bool DoFallibleOperation);
 ///
 ///   int X = cantFail(foo(false));
 ///   @endcode
 template <typename T>
 T cantFail(Expected<T> ValOrErr, const char *Msg = nullptr) {
-  if (ValOrErr)
+
+  if (LLVM_LIKELY(ValOrErr))
     return std::move(*ValOrErr);
-  else {
-    if (!Msg)
-      Msg = "Failure value returned from cantFail wrapped call";
-#ifndef NDEBUG
-    std::string Str;
-    raw_string_ostream OS(Str);
-    auto E = ValOrErr.takeError();
-    OS << Msg << "\n" << E;
-    Msg = OS.str().c_str();
-#endif
-    llvm_unreachable(Msg);
-  }
+  else
+    detail::fatalCantFailUnhandledError(ValOrErr.takeError(), Msg);
 }
 
 /// Report a fatal error if ValOrErr is a failure value, otherwise unwraps and
 /// returns the contained reference.
 ///
 /// This function can be used to wrap calls to fallible functions ONLY when it
 /// is known that the Error will always be a success value. E.g.
 ///
 ///   @code{.cpp}
 ///   // foo only attempts the fallible operation if DoFallibleOperation is
 ///   // true. If DoFallibleOperation is false then foo always returns a Bar&.
 ///   Expected<Bar&> foo(bool DoFallibleOperation);
 ///
 ///   Bar &X = cantFail(foo(false));
 ///   @endcode
 template <typename T>
 T& cantFail(Expected<T&> ValOrErr, const char *Msg = nullptr) {
-  if (ValOrErr)
+  if (LLVM_LIKELY(ValOrErr))
     return *ValOrErr;
-  else {
-    if (!Msg)
-      Msg = "Failure value returned from cantFail wrapped call";
-#ifndef NDEBUG
-    std::string Str;
-    raw_string_ostream OS(Str);
-    auto E = ValOrErr.takeError();
-    OS << Msg << "\n" << E;
-    Msg = OS.str().c_str();
-#endif
-    llvm_unreachable(Msg);
-  }
+  else
+    detail::fatalCantFailUnhandledError(ValOrErr.takeError(), Msg);
 }
 
 /// Helper for testing applicability of, and applying, handlers for
 /// ErrorInfo types.
 template <typename HandlerT>
 class ErrorHandlerTraits
     : public ErrorHandlerTraits<decltype(
           &std::remove_reference<HandlerT>::type::operator())> {};
if (Err) {
       exit(ExitCode);
     }
   }
 
   std::string Banner;
   std::function<int(const Error &)> GetExitCode;
 };
 
+/// Enable LLVM error tracing.
+void enableErrorTracing();
+
 /// Conversion from Error to LLVMErrorRef for C error bindings.
 inline LLVMErrorRef wrap(Error Err) {
   return reinterpret_cast<LLVMErrorRef>(Err.takePayload().release());
 }
 
 /// Conversion from LLVMErrorRef to Error for C error bindings.
 inline Error unwrap(LLVMErrorRef ErrRef) {
   return Error(std::unique_ptr<ErrorInfoBase>(
       reinterpret_cast<ErrorInfoBase *>(ErrRef)));
 }
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_ERROR_H

llvm/include/llvm/Support/InitLLVM.h

 //  2. Set up the global new-handler which is called when a memory allocation
 //     attempt fails.
 //
 //  3. If running on Windows, obtain command line arguments using a
 //     multibyte character-aware API and convert arguments into UTF-8
 //     encoding, so that you can assume that command line arguments are
 //     always encoded in UTF-8 on any platform.
 //
+//  4. Checks the LLVM_ENABLE_ERROR_TRACING environment variable and enables
+//     backtraces for llvm::Error asserts.
+//
 // InitLLVM calls llvm_shutdown() on destruction, which cleans up
 // ManagedStatic objects.
 namespace llvm {
 class InitLLVM {
 public:
   InitLLVM(int &Argc, const char **&Argv,
            bool InstallPipeSignalExitHandler = true);
   InitLLVM(int &Argc, char **&Argv, bool InstallPipeSignalExitHandler = true)

llvm/lib/Object/Object.cpp

 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm-c/Object.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Object/ObjectFile.h"
 #include "llvm/Object/MachOUniversal.h"
+#include "llvm/Support/Error.h"
 
 using namespace llvm;
 using namespace object;
 
 inline OwningBinary<ObjectFile> *unwrap(LLVMObjectFileRef OF) {
   return reinterpret_cast<OwningBinary<ObjectFile> *>(OF);
 }
 

llvm/lib/Object/ObjectFile.cpp

 #include "llvm/ADT/StringRef.h"
 #include "llvm/BinaryFormat/Magic.h"
 #include "llvm/Object/Binary.h"
 #include "llvm/Object/COFF.h"
 #include "llvm/Object/Error.h"
 #include "llvm/Object/MachO.h"
 #include "llvm/Object/Wasm.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ErrorOr.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/raw_ostream.h"
 #include <algorithm>
 #include <cstdint>
 #include <memory>
 #include <system_error>

llvm/lib/Support/Error.cpp

 //===----- lib/Support/Error.cpp - Error and associated utilities ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Support/Error.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h"
+
+#include <mutex>
 #include <system_error>
 
 using namespace llvm;
 
 namespace {
 
   enum class ErrorErrorCode : int {
     MultipleErrors = 1,
namespace {
   };
 
 }
 
 static ManagedStatic<ErrorErrorCategory> ErrorErrorCat;
 
 namespace llvm {
 
+namespace detail {
+std::mutex ErrorTraceMutex;
+std::unique_ptr<ErrorTraceMap> ErrorTraces;
+
+void annotateErrorWithTrace(ErrorInfoBase &EIB) {
+  std::string Trace;
+  {
+    raw_string_ostream TraceStream(Trace);
+    sys::PrintStackTrace(TraceStream);
+  }
+  std::lock_guard<std::mutex> Lock(ErrorTraceMutex);
+  (*ErrorTraces)[&EIB] = std::move(Trace);
+}
+
+void printErrorTrace(raw_ostream &OS, ErrorInfoBase &EI) {
+  assert(ErrorTraces &&
+         "printErrorTrace should not be called unless tracing is enabled");
+  std::lock_guard<std::mutex> Lock(ErrorTraceMutex);
+  if (ErrorTraces->count(&EI))
+    OS << (*ErrorTraces)[&EI];
+  else
+    OS << "No trace recorded for ErrorInfo " << &EI << "\n";
+}
+
+void clearErrorTrace(ErrorInfoBase &EI) {
+  assert(ErrorTraces &&
+         "printErrorTrace should not be called unless tracing is enabled");
+  std::lock_guard<std::mutex> Lock(ErrorTraceMutex);
+  ErrorTraces->erase(&EI);
+}
+
+void fatalCantFailUnhandledError(Error Err, const char *Msg) {
+  if (!Msg)
+    Msg = "Failure value returned from cantFail wrapped call";
+  std::string Str;
+  {
+    raw_string_ostream OS(Str);
+    OS << Msg << "\n" << Err << "\n";
+    if (ErrorTraces) {
+      OS << "Error thrown from:\n";
+      handleAllErrors(std::move(Err),
+                      [&](ErrorInfoBase &EIB) { printErrorTrace(OS, EIB); });
+    }
+    OS << "cantFail called from:\n";
+    sys::PrintStackTrace(OS);
+    Msg = OS.str().c_str();
+  }
+  llvm_unreachable(Msg);
+}
+
+} // namespace detail
+
 void ErrorInfoBase::anchor() {}
 char ErrorInfoBase::ID = 0;
 char ErrorList::ID = 0;
 void ECError::anchor() {}
 char ECError::ID = 0;
 char StringError::ID = 0;
 char FileError::ID = 0;
 
+void enableErrorTracing() {
+  detail::ErrorTraces = std::make_unique<detail::ErrorTraceMap>();
+}
+
 void logAllUnhandledErrors(Error E, raw_ostream &OS, Twine ErrorBanner) {
   if (!E)
     return;
   OS << ErrorBanner;
   handleAllErrors(std::move(E), [&](const ErrorInfoBase &EI) {
     EI.log(OS);
     OS << "\n";
   });
 }
 
-
 std::error_code ErrorList::convertToErrorCode() const {
   return std::error_code(static_cast<int>(ErrorErrorCode::MultipleErrors),
                          *ErrorErrorCat);
 }
 
 std::error_code inconvertibleErrorCode() {
   return std::error_code(static_cast<int>(ErrorErrorCode::InconvertibleError),
                          *ErrorErrorCat);
std::error_code errorToErrorCode(Error Err) {
   });
   if (EC == inconvertibleErrorCode())
     report_fatal_error(EC.message());
   return EC;
 }
 
 #if LLVM_ENABLE_ABI_BREAKING_CHECKS
 void Error::fatalUncheckedError() const {
-  dbgs() << "Program aborted due to an unhandled Error:\n";
+  dbgs() << "Program aborted due to an unhandled Error. Error message:\n\"";
   if (getPtr()) {
     getPtr()->log(dbgs());
-    dbgs() << "\n";
-  }else
+    dbgs() << "\"\n";
+    if (detail::ErrorTraces) {
+      dbgs() << "Error thrown from:\n";
+      detail::printErrorTrace(dbgs(), *getPtr());
+    }
+    dbgs() << "Error discarded from:\n";
+    sys::PrintStackTrace(dbgs());
+  } else
     dbgs() << "Error value was Success. (Note: Success values must still be "
               "checked prior to being destroyed).\n";
   abort();
 }
 #endif
 
 StringError::StringError(std::error_code EC, const Twine &S)
     : Msg(S.str()), EC(EC) {}
 
 StringError::StringError(const Twine &S, std::error_code EC)
     : Msg(S.str()), EC(EC), PrintMsgOnly(true) {}

llvm/lib/Support/InitLLVM.cpp

 InitLLVM::InitLLVM(int &Argc, const char **&Argv,
                    bool InstallPipeSignalExitHandler)
     : StackPrinter(Argc, Argv) {
   if (InstallPipeSignalExitHandler)
     sys::SetOneShotPipeSignalFunction(sys::DefaultOneShotPipeSignalHandler);
   sys::PrintStackTraceOnErrorSignal(Argv[0]);
   install_out_of_memory_new_handler();
 
+  if (getenv("LLVM_ENABLE_ERROR_TRACING"))
+    enableErrorTracing();
+
 #ifdef _WIN32
   // We use UTF-8 as the internal character encoding. On Windows,
   // arguments passed to main() may not be encoded in UTF-8. In order
   // to reliably detect encoding of command line arguments, we use an
   // Windows API to obtain arguments, convert them to UTF-8, and then
   // write them back to the Argv vector.
   //
   // There's probably other way to do the same thing (e.g. using