LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 22368 - LLVMParseBitcode and LLVMParseBitcodeInContext exit on error
Summary: LLVMParseBitcode and LLVMParseBitcodeInContext exit on error
Status: RESOLVED FIXED
Alias: None
Product: new-bugs
Classification: Unclassified
Component: new bugs (show other bugs)
Version: unspecified
Hardware: PC All
: P normal
Assignee: Unassigned LLVM Bugs
URL:
Keywords:
Depends on:
Blocks: 22374
  Show dependency tree
 
Reported: 2015-01-28 05:23 PST by Antoine Pitrou
Modified: 2015-02-03 11:27 PST (History)
4 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Antoine Pitrou 2015-01-28 05:23:10 PST
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.
Comment 1 Richard Smith 2015-01-28 13:55:50 PST
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.
Comment 2 Hans Wennborg 2015-01-28 18:57:58 PST
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?
Comment 3 Hans Wennborg 2015-01-29 21:26:03 PST
(In reply to comment #0)
> 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.
Comment 4 Antoine Pitrou 2015-01-30 11:20:22 PST
> 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 :-)
Comment 5 Hans Wennborg 2015-01-30 11:38:16 PST
(In reply to comment #4)
> > 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.
Comment 6 Rafael Ávila de Espíndola 2015-02-02 19:55:04 PST
Fixed with r227903 and r227934.
Comment 7 Hans Wennborg 2015-02-03 11:27:40 PST
Thanks!

Merged to 3.6 in r227984 and r227985.