From b182aa2cfab793e566c92183c361e78f4d6f84cb Mon Sep 17 00:00:00 2001
From: Sverker Eriksson <sverker@erlang.org>
Date: Wed, 3 Sep 2014 17:13:22 +0200
Subject: erts: Fix bug with enif_make_copy reallocating writable binary

that could invalidate a pointer received from an earlier
call to enif_inspect_binary.

Solution: Emasculate writable binary at enif_inspect_binary. There are
room for optimizations here as we now do an unconditional emasculation
even though enif_make_copy is not called later in the NIF.
---
 erts/emulator/beam/erl_nif.c                   | 12 ++++++++++++
 erts/emulator/test/nif_SUITE.erl               | 17 +++++++++++++++++
 erts/emulator/test/nif_SUITE_data/nif_SUITE.c  | 21 +++++++++++++++++++++
 system/doc/efficiency_guide/binaryhandling.xml |  7 ++++---
 4 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/erts/emulator/beam/erl_nif.c b/erts/emulator/beam/erl_nif.c
index ff551ea3af..8c4bf73dbe 100644
--- a/erts/emulator/beam/erl_nif.c
+++ b/erts/emulator/beam/erl_nif.c
@@ -472,6 +472,18 @@ int enif_inspect_binary(ErlNifEnv* env, Eterm bin_term, ErlNifBinary* bin)
 	struct enif_tmp_obj_t* tmp;
 	byte* raw_ptr;
     }u;
+
+    if (is_boxed(bin_term) && *binary_val(bin_term) == HEADER_SUB_BIN) {
+	ErlSubBin* sb = (ErlSubBin*) binary_val(bin_term);
+	if (sb->is_writable) {
+	    ProcBin* pb = (ProcBin*) binary_val(sb->orig);
+	    ASSERT(pb->thing_word == HEADER_PROC_BIN);
+	    if (pb->flags) {
+		erts_emasculate_writable_binary(pb);
+		sb->is_writable = 0;
+	    }
+	}
+    }
     u.tmp = NULL;
     bin->data = erts_get_aligned_binary_bytes_extra(bin_term, &u.raw_ptr, allocator,
 						    sizeof(struct enif_tmp_obj_t));
diff --git a/erts/emulator/test/nif_SUITE.erl b/erts/emulator/test/nif_SUITE.erl
index b2da6f58af..2e9b4aeab1 100644
--- a/erts/emulator/test/nif_SUITE.erl
+++ b/erts/emulator/test/nif_SUITE.erl
@@ -37,6 +37,7 @@
 	 threading/1, send/1, send2/1, send3/1, send_threaded/1, neg/1, 
 	 is_checks/1,
 	 get_length/1, make_atom/1, make_string/1, reverse_list_test/1,
+	 otp_9828/1,
 	 otp_9668/1, consume_timeslice/1, dirty_nif/1, dirty_nif_send/1
 	]).
 
@@ -64,6 +65,7 @@ all() ->
      resource_takeover, threading, send, send2, send3,
      send_threaded, neg, is_checks, get_length, make_atom,
      make_string,reverse_list_test,
+     otp_9828,
      otp_9668, consume_timeslice, dirty_nif, dirty_nif_send
     ].
 
@@ -1440,6 +1442,20 @@ otp_9668(Config) ->
     ?line verify_tmpmem(TmpMem),
     ok.
 
+otp_9828(doc) -> ["Copy of writable binary"];
+otp_9828(Config) ->
+    ensure_lib_loaded(Config, 1),
+
+    otp_9828_loop(<<"I'm alive!">>, 1000).
+
+otp_9828_loop(Bin, 0) ->
+    ok;
+otp_9828_loop(Bin, Val) ->
+    WrtBin = <<Bin/binary, Val:32>>,
+    ok = otp_9828_nif(WrtBin),
+    otp_9828_loop(WrtBin, Val-1).
+
+
 consume_timeslice(Config) when is_list(Config) ->
     CONTEXT_REDS = 2000,
     Me = self(),
@@ -1684,6 +1700,7 @@ reverse_list(_) -> ?nif_stub.
 echo_int(_) -> ?nif_stub.
 type_sizes() -> ?nif_stub.
 otp_9668_nif(_) -> ?nif_stub.
+otp_9828_nif(_) -> ?nif_stub.
 consume_timeslice_nif(_,_) -> ?nif_stub.
 call_dirty_nif(_,_,_) -> ?nif_stub.
 send_from_dirty_nif(_) -> ?nif_stub.
diff --git a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
index 955dc64189..25e20f3e7d 100644
--- a/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
+++ b/erts/emulator/test/nif_SUITE_data/nif_SUITE.c
@@ -1473,6 +1473,26 @@ static ERL_NIF_TERM otp_9668_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM ar
     return atom_ok;
 }
 
+static ERL_NIF_TERM otp_9828_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
+{
+    /* copy a writable binary could reallocate it due to "emasculation"
+       and thereby render a previous inspection invalid.
+     */
+    ErlNifBinary bin1;
+    ErlNifEnv* myenv;
+
+    if (!enif_inspect_binary(env, argv[0], &bin1)) {
+	return enif_make_badarg(env);
+    }
+
+    myenv = enif_alloc_env();
+    enif_make_copy(myenv, argv[0]);
+    enif_free_env(myenv);
+
+    return memcmp(bin1.data, "I'm alive!", 10)==0 ? atom_ok : atom_false;
+}
+
+
 static ERL_NIF_TERM consume_timeslice_nif(ErlNifEnv* env, int argc, const ERL_NIF_TERM argv[])
 {
     int percent;
@@ -1741,6 +1761,7 @@ static ErlNifFunc nif_funcs[] =
     {"echo_int", 1, echo_int},
     {"type_sizes", 0, type_sizes},
     {"otp_9668_nif", 1, otp_9668_nif},
+    {"otp_9828_nif", 1, otp_9828_nif},
     {"consume_timeslice_nif", 2, consume_timeslice_nif},
 #ifdef ERL_NIF_DIRTY_SCHEDULER_SUPPORT
     {"call_dirty_nif", 3, call_dirty_nif},
diff --git a/system/doc/efficiency_guide/binaryhandling.xml b/system/doc/efficiency_guide/binaryhandling.xml
index 6b0df49011..4ba1378059 100644
--- a/system/doc/efficiency_guide/binaryhandling.xml
+++ b/system/doc/efficiency_guide/binaryhandling.xml
@@ -5,7 +5,7 @@
   <header>
     <copyright>
       <year>2007</year>
-      <year>2013</year>
+      <year>2014</year>
       <holder>Ericsson AB, All Rights Reserved</holder>
     </copyright>
     <legalnotice>
@@ -237,8 +237,9 @@ Bin = <<Bin1,...>>  %% Bin1 will be COPIED
       <p><c>Bin1</c> will be copied in the third line.</p>
 
       <p>The same thing happens if you insert a binary into an <em>ets</em>
-      table or send it to a port using <c>erlang:port_command/2</c>.</p>
-
+      table or send it to a port using <c>erlang:port_command/2</c> or pass it to
+      <seealso marker="erts:erl_nif#enif_inspect_binary">enif_inspect_binary</seealso>
+      in a NIF.</p>
       <p>Matching a binary will also cause it to shrink and the next append
       operation will copy the binary data:</p>
 
-- 
cgit v1.2.3