arm/emit.mlp - duplicate label in PIC mode

[progman1]

in v4.14/asmcomp/arm/emit.mlp # 330, namely the emit-load-symbol-addr function,
the generated asm sequence for the PIC case includes a label definition:

    `	ldr	{emit_reg tmp}, {emit_label lbl_got}\n`;
    `	ldr	{emit_reg dst}, {emit_label lbl_sym}\n`;
    `{emit_label lbl_pic}:	add	{emit_reg tmp}, pc, {emit_reg tmp}\n`;
    `	ldr	{emit_reg dst}, [{emit_reg tmp}, {emit_reg dst}] @ {emit_symbol s}\n`;

but on emission of the literal produced by

    let lbl_pic = new_label() in
    let lbl_got = gotrel_literal env lbl_pic in

the same label is defined again in emit-literals:

    List.iter
      (fun { lbl_got; lbl_pic } ->
        `{emit_label lbl_pic}:	.word	_GLOBAL_OFFSET_TABLE_-({emit_label lbl_got}+{emit_int offset})\n`)
      env.gotrel_literals;

since this is in a constant island within the .text section the two definitions should necessarily clash.
I cannot say a bug results from this as I'm still figuring out how to construct some ML code to generate the asm.

but it does look wrong.
and indeed I am having a hard time understanding how the generated code sequence is intended to work.
and maybe they are related! so any guidance appreciated!

more generally would someone comment on the following?:
emit-load-symbol-addr, by dealing with non-relative symbol addresses, is a means to avoid the issue of far pointers - out-of-range of limited ARM instructions - that would likely otherwise occur when linking compilation units.

thanks.

[progman1]

a bug it seems:
prior to 4.13 there was

let gotrel_literal l =
  let lbl = new_label() in
  size_literals := !size_literals + 1;
  gotrel_literals := (l, lbl) :: !gotrel_literals;
  lbl

which then became:

let gotrel_literal lbl_pic =
  let lbl_got = new_label() in
  size_literals := !size_literals + 1;
  gotrel_literals := (lbl_got, lbl_pic) :: !gotrel_literals;
  lbl_got

a swap in the saved pair order without adjustment in emit-literals
but it's probably the latter that needs fixing.

edit: indeed the usage of pic/got in emit-literals should be swapped

    List.iter
      (fun { lbl_got; lbl_pic } ->
        `{emit_label lbl_got}:	.word	_GLOBAL_OFFSET_TABLE_-({emit_label lbl_pic}+{emit_int offset})\n`)
      env.gotrel_literals;

it worked before 4.13 because the pair was the wrong way round!

[xavierleroy]

Well spotted! Looks like the bug was introduced by #8936. @gretay-js : could you please confirm and perhaps submit a fix?

[gretay-js]

Yes, the bug is swapping the order of the two labels in emit_literals, and the fix is exactly what @progman1 suggested, thank you! The CI seems unhappy with the fix for unrelated reasons, and I don't have access to an arm to test.