[erlang-bugs] A bug in address patching in hipe_x86.c?

[Sverker Eriksson]

On 01/31/2014 01:58 PM, Yiannis Tsiouris wrote:
> Hey,
>
> While trying to locate a bug in our x86 backend that caused a segfault 
> at load-time, we came at something that we think is a bug in the final 
> address calculation. In functions hipe_patch_load_fe and 
> hipe_patch_insn in erts/emulator/hipe/hipe_x86.c file we think that 
> the final address should always be calculated as the sum of "address" 
> and "value". We base this on our observation that "address" seems to 
> be the offset and "value" seems to be the base address.
>
> The patch that works for us is attached.
>
> If any developer can confirm that this is a bug (and that the patch is 
> the correct way to fix it) I can submit it properly (if needed). :-)
>
> Thanks,
> Yiannis (as member of the ErLLVM team)
>

Thank for the patch, Yiannis

(comments below)

diff --git a/erts/emulator/hipe/hipe_x86.c b/erts/emulator/hipe/hipe_x86.c
index 327c74e..6c233cb 100644
--- a/erts/emulator/hipe/hipe_x86.c
+++ b/erts/emulator/hipe/hipe_x86.c
@@ -36,7 +36,7 @@
  void hipe_patch_load_fe(Uint32 *address, Uint32 value)
  {
      /* address points to a disp32 or imm32 operand */
-    *address = value;
+    *address += value;
  }

  int hipe_patch_insn(void *address, Uint32 value, Eterm type)
@@ -46,14 +46,12 @@ int hipe_patch_insn(void *address, Uint32 value, 
Eterm type)
        case am_constant:
        case am_atom:
        case am_c_const:
-    break;
        case am_x86_abs_pcrel:
-    value += (Uint)address;
      break;
        default:
      return -1;
      }
-    *(Uint32*)address = value;
+    *(Uint32*)address += value;
      return 0;
  }

If this has worked before (which it has) then the old value of *address 
must always have been zero.

But it looks like hipe_patch_insn() can be called in a module upgrade 
scenario (see hipe_bifs_redirect_referred_from_1 in hipe_bif0.c)
and then the old value of *address must (I guess?) refer to something in 
the old module instance.
Adding old and new values can not be right (whatever "value" may be).


Have you tried this patch with the existing x86 backend? And have you 
tried upgrading a module and verified that it is reachable after the 
upgrade?

/Sverker, Erlang/OTP