[PATCH] D70157: Align branches within 32-Byte boundary

[Kan Shengchen via Phabricator via cfe-commits]

skan added a comment.

In D70157#1771771 <https://reviews.llvm.org/D70157#1771771>, @reames wrote:

> We uncovered another functional issue with this patch, or at least, the interaction of this patch and other parts of LLVM.  In our support for STATEPOINT, PATCHPOINT, and STACKMAP we use N-byte nop sequences for regions of code which might be patched out.  It's important that these regions are exactly N bytes as concurrent patching which doesn't replace an integral number of instructions is ill-defined on X86-64.  This patch causes the N-byte nop sequence to sometimes become (N+M) bytes which breaks the patching.  I believe that the XRAY support may have a similar issue.
>
> More generally, I'm worried about the legality of arbitrarily prefixing instructions from unknown sources.  In the particular example we saw, we had something along the following:
>
> .Ltmp0:
>
>   	.p2align	3, 0x90
>   	(16 byte nop sequence)
>
> .Ltmp3:
>
>   	jmp *%rax
>
>
> In addition to the patching legality issue above, padding the nop sequence does something else interesting in this example.  It changes the alignment of Ltmp3.  Before, Ltmp3 was always 8 byte aligned, after prefixes are added, it's not.  It's not clear to me exactly what the required semantics here are, but we at least had been assuming the alignment of Ltmp3 was guaranteed in this case.  (That's actually how we found the patching issue.)


I could not reproduce the phenomenon that N-byte nop becomes (N+M) bytes with your example. So according to my understanding, I slightly modified your case. (If my understand is wrong, I hope you can point it out :-). )

      .text
      nop
  .Ltmp0:
      .p2align 3, 0x90
      .rept 16
      nop
      .endr
  .Ltmp3:
      movl  %eax, -4(%rsp)
      .rept 2
      nop
      .endr
      jmp .Ltmp0

The instruction `jmp .Ltmp0` starts at byte 0x1e and ends at byte 0x20. If we align the jump with preifx, two prefixes will be added to the `.rept2 16 nop .endr`. After prefixes are added, the 16-byte nop becomes 18-byte nop, then the label '.Ltmp3' is not 8-byte aligned any more.

I doubt whether the assumption that '.Ltmp3' is 8-byte aligned is right, since the alignment is not explicitly required.  For example, I think we can not assuming the instruction `jmp .Ltmp0` always starts at byte 0x1e. And in fact, as long as the binary generated by the compiler does not have a one-to-one correspondence with the assembly code, at least one implict assumption of an instruction will be broken.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D70157/new/

https://reviews.llvm.org/D70157