[erlang-bugs] enif_make_badarg crashes is_binary

[Sverker Eriksson]

Mikael Pettersson wrote:
> Steve Vinoski writes:
>  > I was recently writing a nif and had one C function returning an
>  > ERL_NIF_TERM that was then being passed to a second C function. The
>  > second one checked the passed term using
>  > enif_inspect_iolist_as_binary(), and it crashed. I debugged it and
>  > it's apparently caused by the fact that the ERL_NIF_TERM being passed
>  > was returned from enif_make_badarg(), so it was a 0. It caused this
>  > error on my Macbook Pro:
>  > 
>  > Program received signal EXC_BAD_ACCESS, Could not access memory.
>  > Reason: KERN_INVALID_ADDRESS at address: 0xfffffffe
>  > 
>  > The reason for that invalid address is the is_binary(x) macro at the
>  > beginning of enif_inspect_iolist_as_binary(). It expands to
>  > 
>  >     if (((!(((((term)))) & (0x3 -0x2))) && (((*((Eterm*) (((((term)))
>  > - 0x2)))) & (0x3F -(0x3 << 2))) == (0x0|(0x8 << 2))))) {
>  > 
>  > The first test, which I believe comes from _unchecked_is_boxed(x),
>  > passes because it's testing !0 which of course is 1, then the second
>  > test which I believe is _unchecked_boxed_val(x) fails with the invalid
>  > address error because the code more or less dereferences a null
>  > pointer.
>  > 
>  > I think the fix is to make macros like is_binary(x) first check the
>  > term using is_value(x).
>
> No.  Non-values are emphatically NOT permitted inside Erlang process
> heaps, live BEAM VM registers, live terms, or as parameters to internal
> C functions (except for very few special cases).  The non-values are
> very private to the VM, and are (as I recall, it's been a while since
> I wrote that code), limited to (a) indicating exception returns from
> C BIFs (handled immediately by beam_emu), (b) temporary magic markers
> during GC, and (c) signalling error returns from some internal C
> functions.  In no case is a non-value allowed to leak into
> application-visible Erlang state.
>
> The fix, therefore, is to catch whatever produced that non-value
> (error from a C function?) and turn that into an error return or
> Erlang exception rather than trying to pass it on as a valid term.
>
> /Mikael
>
>   
Yes, Mikael is right.
The documentation could be more clear about the only valid use of 
enif_make_badarg(); return its value from the nif. You are not even 
allowed to change your mind, once enif_make_badarg() has been called, 
its return value *must* be returned from the nif.
enif_make_badarg() was a bit of a quick fix just to get the most common 
use case to work. In future we might want functions to throw customized 
exceptions. A small step in that direction could be an enif_is_value() 
(or maybe enif_is_exception) in order to  distinguish values from 
exceptions.


/Sverker, Erlang/OTP