Sverker Eriksson writes:
> 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.
No. Both procedures implement plain assignment semantics. The old
value of *address is not used. I believe the old values always will be
zero, but that's only because we have to pad the instruction stream
with zeros to make room for the to-be-patched immediates.
/Mikael