[foreign-jextract] [Rev 02] RFR: Improve jextract error reporting

Jorn Vernee jvernee at openjdk.java.net
Thu Mar 26 10:23:18 UTC 2020


> Hi,
> 
> I spent the last couple of days tracking down a crash I was seeing sometimes when running jextract. In the process I
> ended up doing several improvements to jextract's error checking and reporting, that I think are useful to keep in the
> long run.  The changes include:
> - Using `clang_parseTranslationUnit2` to parse, since that returns an error code, instead of a null translation unit.
>   This also means we can still process diagnostic for the returned translation unit (now returned in a buffer).
> - Checking the return codes for `clang_parseTranslationUnit2`, `clang_saveTranslationUnit`, and
>   `clang_reparseTranslationUnit` (using a proper enum) and throwing an exception in case they are erroneous.
> - Catching BadMacroException instead of Throwable in the macro parser. We only care about the former, and catching
>   Throwable could lead to too many exceptions being eaten. Technically, if there is an unknown exception, we could just
>   skip the macro, but this isn't always the right strategy. For example, when re-parsing fails the used translation unit
>   becomes invalid, so we really need to do something else than just skipping the macro in that case (to be addressed in
>   another patch https://bugs.openjdk.java.net/browse/JDK-8240614). Imo it's better to just crash than to skip when we
>   encounter an unknwon exception, so that we're more likely to find issues. Enabling libclang crash recovery by default.
>   By default the crash recovery was being disabled. This leads to the process exiting with OS exception code. If we
>   enable this however, it will instead try to return an error code from the libclang call, which we can then check for
>   and handle the failure however we want.
> 
> Thanks,
> Jorn

Jorn Vernee has updated the pull request incrementally with one additional commit since the last revision:

  Change crash recovery debug message to print enabled/disabled

-------------

Changes:
  - all: https://git.openjdk.java.net/panama-foreign/pull/40/files
  - new: https://git.openjdk.java.net/panama-foreign/pull/40/files/79bacd6c..1cb6a510

Webrevs:
 - full: https://webrevs.openjdk.java.net/panama-foreign/40/webrev.02
 - incr: https://webrevs.openjdk.java.net/panama-foreign/40/webrev.01-02

  Stats: 2 lines in 1 file changed: 0 ins; 0 del; 2 mod
  Patch: https://git.openjdk.java.net/panama-foreign/pull/40.diff
  Fetch: git fetch https://git.openjdk.java.net/panama-foreign pull/40/head:pull/40

PR: https://git.openjdk.java.net/panama-foreign/pull/40


More information about the panama-dev mailing list