fix missing C interface + fix bug (causes uncaught exceptions) in OCaml interface

Duncan Sands duncan.sands at gmail.com
Mon Jun 3 00:24:54 PDT 2013


Hi David,

On 01/06/13 08:45, David Monniaux wrote:
> llvm-c/Core.h has a family of LLVMIsAXXX functions, which were not
> updated when ConstantDataSequential and subclasses were added. Thus,
> there are no functions to check for membership in
> ConstantDataSequential, ConstantDataArray and ConstantDataVector.
>
> The OCaml interface consequently does not know about these in
> ValueKind.t, and thus classify_value fails (with an exception, thus
> generally crashing the client program) when asked to classify any value
> from a subclass of ConstantDataSequential.
>
> Solution: add the missing functions and cases in classify_value.
>
...
> diff -r -u llvm-3.2.src/bindings/ocaml/llvm/llvm_ocaml.c
> llvm-3.2-fixed_bindings/bindings/ocaml/llvm/llvm_ocaml.c
> --- llvm-3.2.src/bindings/ocaml/llvm/llvm_ocaml.c    2012-09-02
> 16:42:56.000000000 +0200
> +++ llvm-3.2-fixed_bindings/bindings/ocaml/llvm/llvm_ocaml.c
> 2013-05-31 18:20:11.091963286 +0200
> @@ -25,6 +25,8 @@
>   #include <stdlib.h>
>   #include <string.h>
>
> +/* DEBUG */
> +#include <stdio.h>

this #include shouldn't be there.

>
>   /* Can't use the recommended caml_named_value mechanism for backwards
>      compatibility reasons. This is largely equivalent. */
> @@ -416,6 +418,8 @@
>     BlockAddress,
>     ConstantAggregateZero,
>     ConstantArray,
> +  ConstantDataArray,
> +  ConstantDataVector,
>     ConstantExpr,
>     ConstantFP,
>     ConstantInt,
> @@ -441,6 +445,8 @@
>       DEFINE_CASE(Val, BlockAddress);
>       DEFINE_CASE(Val, ConstantAggregateZero);
>       DEFINE_CASE(Val, ConstantArray);
> +    DEFINE_CASE(Val, ConstantDataArray);
> +    DEFINE_CASE(Val, ConstantDataVector);
>       DEFINE_CASE(Val, ConstantExpr);
>       DEFINE_CASE(Val, ConstantFP);
>       DEFINE_CASE(Val, ConstantInt);
> diff -r -u llvm-3.2.src/include/llvm-c/Core.h
> llvm-3.2-fixed_bindings/include/llvm-c/Core.h
> --- llvm-3.2.src/include/llvm-c/Core.h    2012-10-15 22:35:56.000000000
> +0200
> +++ llvm-3.2-fixed_bindings/include/llvm-c/Core.h    2013-05-31
> 18:11:24.744595688 +0200
> @@ -1010,6 +1010,9 @@
>         macro(BlockAddress)                   \
>         macro(ConstantAggregateZero)          \
>         macro(ConstantArray)                  \
> +      macro(ConstantDataSequential)         \

Is it normal that ConstantDataSequential doesn't occur anywhere else in
your patch?

> +        macro(ConstantDataArray)            \
> +        macro(ConstantDataVector)           \

Indentation here seems odd, maybe using tabs?

>         macro(ConstantExpr)                   \
>         macro(ConstantFP)                     \
>         macro(ConstantInt)                    \
>

Otherwise it looks OK to me.

Ciao, Duncan.



More information about the llvm-commits mailing list