LLVMParseBitcode and LLVMParseBitcodeInContext exit on error

[pitrou]

Bugzilla Link	22368
Resolution	FIXED
Resolved on	Feb 03, 2015 11:27
Version	unspecified
OS	All
Blocks	#22748
CC	@zmodem,@zygoloid

Extended Description

LLVMParseBitcode() and LLVMParseBitcodeInContext() are specified to return true on error, false if ok; they also put the error message in the given pointer-to-string.

However, with 3.6rc1, those functions now exit(1) on error. This is because of LLVMContext::diagnose(). From C code there is therefore no obvious way to get back an error properly.

[zygoloid]

This is not entirely true; they exit on some kinds of errors and fail gracefully on others. Perhaps LLVMParseBitcodeInContext should be passing a diagnostic handler into parseBitcodeFile.

[zmodem]

The doxygen for LLVMContext::diagnose() is confusing:

"This function returns, in particular in the case of error reporting (DI.Severity == DS_Error), so the caller should leave the compilation process in a self-consistent state, even though the generated code need not be correct."

But it clearly doesn't return for DS_Error; it calls exit(1) :-/

It would be easy to pass in a DiagnosticHandler that just ignores all diagnostics. Or would we want to put them in OutMessage?

[zmodem]

LLVMParseBitcode() and LLVMParseBitcodeInContext() are specified to return
true on error, false if ok; they also put the error message in the given
pointer-to-string.

However, with 3.6rc1, those functions now exit(1) on error.
This is because
of LLVMContext::diagnose().

Did this not happen in older releases also? It looks like the exit(1) in LLVMContext::diagnose() has been there for a while.

From C code there is therefore no obvious way to
get back an error properly.

One way to work around this issue would be to set a custom diagnostic handler with LLVMContextSetDiagnosticHandler.

I started with a patch for LLVMParseBitcode that passes in a diagnostichandler which ignores everything. Then I started worrying that there could be many other functions that have the same problem.

Maybe a better solution would be to set an ignore-all diagnostichandler when creating a Context from the C API. That way the user can still set a handler if she wishes, but otherwise the API works as expected: no output or exits, just returning error codes.

In fact, given that LLVM is supposed to be usable as a library, having a default diagnostichandler that prints to stderr and can exit seems a little odd.

[pitrou]

Did this not happen in older releases also? It looks like the exit(1) in
LLVMContext::diagnose() has been there for a while.

I doubt this, because the llvmlite unit tests worked fine with 3.5, and they started exiting with 3.6rc1.

Maybe a better solution would be to set an ignore-all diagnostichandler when
creating a Context from the C API.

Is the error still propagated to the function return code, then?

For the purposes of llvmlite, we'd still like to get the error string for better exception messages on the Python side. Of course we could write our own diagnostic handler for that... it's just a bit more work :-)

[zmodem]

Did this not happen in older releases also? It looks like the exit(1) in
LLVMContext::diagnose() has been there for a while.

I doubt this, because the llvmlite unit tests worked fine with 3.5, and they
started exiting with 3.6rc1.

Aha, it seems we didn't use the DiagnosticHandler for bitcode parsing before http://llvm.org/viewvc/llvm-project?rev=225562&view=rev

Rafael, can you take a look? The default DiagnosticHandler exit(1)'s on error, which I don't think was the intention.

I'd like this to be fixed for 3.6.

[llvmbot]

Fixed with r227903 and r227934.

[zmodem]

Thanks!

Merged to 3.6 in r227984 and r227985.

[zmodem]

mentioned in issue #22748