[llvm] r320830 - [X86] Fix XSAVE64 and similar instructions to not be allowed by the assembler in 32-bit mode.

Evgenii Stepanov via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 15 13:02:13 PST 2017


Sounds right. Could you replace it with AND32ri8?

On Fri, Dec 15, 2017 at 12:54 PM, Teresa Johnson <tejohnson at google.com>
wrote:

> +Evgenii who added this
>
> On Fri, Dec 15, 2017 at 12:51 PM, Craig Topper <craig.topper at gmail.com>
> wrote:
>
>> Looks like Asan explicitly uses the wrong AND instruction here. I think
>> that should be AND32ri8.
>>
>>   void EmitCallAsanReport(unsigned AccessSize, bool IsWrite, MCContext
>> &Ctx,
>>                           MCStreamer &Out, const RegisterContext &RegCtx)
>> {
>>     EmitInstruction(Out, MCInstBuilder(X86::CLD));
>>     EmitInstruction(Out, MCInstBuilder(X86::MMX_EMMS));
>>
>>     EmitInstruction(Out, MCInstBuilder(X86::AND64ri8)
>>                              .addReg(X86::ESP)
>>                              .addReg(X86::ESP)
>>                              .addImm(-16));
>>     EmitInstruction(
>>         Out, MCInstBuilder(X86::PUSH32r).addReg(RegCtx.AddressReg(32)));
>>
>>     MCSymbol *FnSym = Ctx.getOrCreateSymbol(Twine("__asan_report_") +
>>                                             (IsWrite ? "store" : "load") +
>>                                             Twine(AccessSize));
>>     const MCSymbolRefExpr *FnExpr =
>>         MCSymbolRefExpr::create(FnSym, MCSymbolRefExpr::VK_PLT, Ctx);
>>     EmitInstruction(Out, MCInstBuilder(X86::CALLpcrel32
>> ).addExpr(FnExpr));
>>   }
>>
>> ~Craig
>>
>> On Fri, Dec 15, 2017 at 12:44 PM, Teresa Johnson <tejohnson at google.com>
>> wrote:
>>
>>> 1134     assert(!(TSFlags & X86II::REX_W) && "REX.W requires 64bit
>>> mode.");
>>> (gdb) p TSFlags
>>> $1 = 281320751164
>>> (gdb) p /x TSFlags
>>> $2 = 0x418006003c
>>>
>>> Teresa
>>>
>>> On Fri, Dec 15, 2017 at 12:04 PM, Teresa Johnson <tejohnson at google.com>
>>> wrote:
>>>
>>>> Will do - I am running regression tests with the assert removed right
>>>> now, but will put it back and get this info afterwards.
>>>>
>>>> Teresa
>>>>
>>>> On Fri, Dec 15, 2017 at 11:41 AM, Craig Topper <craig.topper at gmail.com>
>>>> wrote:
>>>>
>>>>> Any chance you can get the value of the TSFlags variable
>>>>> in X86MCCodeEmitter::emitOpcodePrefix at the time of the assertion
>>>>> failure. That should tell me which instruction is failing.
>>>>>
>>>>> ~Craig
>>>>>
>>>>> On Fri, Dec 15, 2017 at 11:40 AM, Craig Topper <craig.topper at gmail.com
>>>>> > wrote:
>>>>>
>>>>>> Well that's a little concerning. I've removed the assert in r320850.
>>>>>> I'll try to investigate why it was failing. Unless my logic is wrong, that
>>>>>> means we weren't encoding the instruction we think we are.
>>>>>>
>>>>>> ~Craig
>>>>>>
>>>>>> On Fri, Dec 15, 2017 at 11:33 AM, Teresa Johnson via llvm-commits <
>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>
>>>>>>> I am hitting this assert in projects/compiler-rt/lib/asan/tests/asan_asm_test.cc
>>>>>>> when regression testing on Linux x86.
>>>>>>> Teresa
>>>>>>>
>>>>>>> On Fri, Dec 15, 2017 at 9:22 AM, Craig Topper via llvm-commits <
>>>>>>> llvm-commits at lists.llvm.org> wrote:
>>>>>>>
>>>>>>>> Author: ctopper
>>>>>>>> Date: Fri Dec 15 09:22:58 2017
>>>>>>>> New Revision: 320830
>>>>>>>>
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=320830&view=rev
>>>>>>>> Log:
>>>>>>>> [X86] Fix XSAVE64 and similar instructions to not be allowed by the
>>>>>>>> assembler in 32-bit mode.
>>>>>>>>
>>>>>>>> There was a top level "let Predicates =" in the .td file that was
>>>>>>>> overriding the Requires on each instruction.
>>>>>>>>
>>>>>>>> I've added an assert to the code emitter to catch more cases like
>>>>>>>> this. I'm sure this isn't the only place where the right predicates aren't
>>>>>>>> being applied. This assert already found that we don't block btq/btsq/btrq
>>>>>>>> in 32-bit mode.
>>>>>>>>
>>>>>>>> Modified:
>>>>>>>>     llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
>>>>>>>>     llvm/trunk/lib/Target/X86/X86InstrFPStack.td
>>>>>>>>     llvm/trunk/lib/Target/X86/X86InstrSystem.td
>>>>>>>>     llvm/trunk/test/MC/X86/x86-32-coverage.s
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/lib/Target/X86/MCTa
>>>>>>>> rgetDesc/X86MCCodeEmitter.cpp
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>>>>>>>> 6/MCTargetDesc/X86MCCodeEmitter.cpp?rev=320830&r1=320829&r2=
>>>>>>>> 320830&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
>>>>>>>> (original)
>>>>>>>> +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp
>>>>>>>> Fri Dec 15 09:22:58 2017
>>>>>>>> @@ -1130,6 +1130,8 @@ bool X86MCCodeEmitter::emitOpcodePrefix(
>>>>>>>>        EmitByte(0x40 | REX, CurByte, OS);
>>>>>>>>        Ret = true;
>>>>>>>>      }
>>>>>>>> +  } else {
>>>>>>>> +    assert(!(TSFlags & X86II::REX_W) && "REX.W requires 64bit
>>>>>>>> mode.");
>>>>>>>>    }
>>>>>>>>
>>>>>>>>    // 0x0F escape code must be emitted just before the opcode.
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/lib/Target/X86/X86InstrFPStack.td
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>>>>>>>> 6/X86InstrFPStack.td?rev=320830&r1=320829&r2=320830&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- llvm/trunk/lib/Target/X86/X86InstrFPStack.td (original)
>>>>>>>> +++ llvm/trunk/lib/Target/X86/X86InstrFPStack.td Fri Dec 15
>>>>>>>> 09:22:58 2017
>>>>>>>> @@ -697,19 +697,18 @@ def FSCALE : I<0xD9, MRM_FD, (outs), (in
>>>>>>>>  def FCOMPP : I<0xDE, MRM_D9, (outs), (ins), "fcompp", [],
>>>>>>>> IIC_FCOMPP>;
>>>>>>>>  } // Defs = [FPSW]
>>>>>>>>
>>>>>>>> -let Predicates = [HasFXSR] in {
>>>>>>>> -  def FXSAVE : I<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -               "fxsave\t$dst", [(int_x86_fxsave addr:$dst)],
>>>>>>>> IIC_FXSAVE>, TB;
>>>>>>>> -  def FXSAVE64 : RI<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -                 "fxsave64\t$dst", [(int_x86_fxsave64 addr:$dst)],
>>>>>>>> -                 IIC_FXSAVE>, TB, Requires<[In64BitMode]>;
>>>>>>>> -  def FXRSTOR : I<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>>>>>>>> -                "fxrstor\t$src", [(int_x86_fxrstor addr:$src)],
>>>>>>>> IIC_FXRSTOR>,
>>>>>>>> -                TB;
>>>>>>>> -  def FXRSTOR64 : RI<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>>>>>>>> -                  "fxrstor64\t$src", [(int_x86_fxrstor64
>>>>>>>> addr:$src)],
>>>>>>>> -                  IIC_FXRSTOR>, TB, Requires<[In64BitMode]>;
>>>>>>>> -} // Predicates = [FeatureFXSR]
>>>>>>>> +def FXSAVE : I<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +             "fxsave\t$dst", [(int_x86_fxsave addr:$dst)],
>>>>>>>> IIC_FXSAVE>, TB,
>>>>>>>> +             Requires<[HasFXSR]>;
>>>>>>>> +def FXSAVE64 : RI<0xAE, MRM0m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +               "fxsave64\t$dst", [(int_x86_fxsave64 addr:$dst)],
>>>>>>>> +               IIC_FXSAVE>, TB, Requires<[HasFXSR, In64BitMode]>;
>>>>>>>> +def FXRSTOR : I<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>>>>>>>> +              "fxrstor\t$src", [(int_x86_fxrstor addr:$src)],
>>>>>>>> IIC_FXRSTOR>,
>>>>>>>> +              TB, Requires<[HasFXSR]>;
>>>>>>>> +def FXRSTOR64 : RI<0xAE, MRM1m, (outs), (ins opaque512mem:$src),
>>>>>>>> +                "fxrstor64\t$src", [(int_x86_fxrstor64 addr:$src)],
>>>>>>>> +                IIC_FXRSTOR>, TB, Requires<[HasFXSR, In64BitMode]>;
>>>>>>>>  } // SchedRW
>>>>>>>>
>>>>>>>>  //===------------------------------------------------------
>>>>>>>> ----------------===//
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/lib/Target/X86/X86InstrSystem.td
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>>>>>>>> 6/X86InstrSystem.td?rev=320830&r1=320829&r2=320830&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- llvm/trunk/lib/Target/X86/X86InstrSystem.td (original)
>>>>>>>> +++ llvm/trunk/lib/Target/X86/X86InstrSystem.td Fri Dec 15
>>>>>>>> 09:22:58 2017
>>>>>>>> @@ -555,49 +555,47 @@ let Uses = [EDX, EAX, ECX] in
>>>>>>>>  } // HasXSAVE
>>>>>>>>
>>>>>>>>  let Uses = [EDX, EAX] in {
>>>>>>>> -let Predicates = [HasXSAVE] in {
>>>>>>>> -  def XSAVE : I<0xAE, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -                "xsave\t$dst",
>>>>>>>> -                [(int_x86_xsave addr:$dst, EDX, EAX)]>, PS;
>>>>>>>> -  def XSAVE64 : RI<0xAE, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -                   "xsave64\t$dst",
>>>>>>>> -                   [(int_x86_xsave64 addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[In64BitMode]>;
>>>>>>>> -  def XRSTOR : I<0xAE, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -                 "xrstor\t$dst",
>>>>>>>> -                 [(int_x86_xrstor addr:$dst, EDX, EAX)]>, PS;
>>>>>>>> -  def XRSTOR64 : RI<0xAE, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>> -                    "xrstor64\t$dst",
>>>>>>>> -                    [(int_x86_xrstor64 addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[In64BitMode]>;
>>>>>>>> -}
>>>>>>>> +def XSAVE : I<0xAE, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +              "xsave\t$dst",
>>>>>>>> +              [(int_x86_xsave addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[HasXSAVE]>;
>>>>>>>> +def XSAVE64 : RI<0xAE, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +                 "xsave64\t$dst",
>>>>>>>> +                 [(int_x86_xsave64 addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[HasXSAVE, In64BitMode]>;
>>>>>>>> +def XRSTOR : I<0xAE, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +               "xrstor\t$dst",
>>>>>>>> +               [(int_x86_xrstor addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[HasXSAVE]>;
>>>>>>>> +def XRSTOR64 : RI<0xAE, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>> +                  "xrstor64\t$dst",
>>>>>>>> +                  [(int_x86_xrstor64 addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[HasXSAVE, In64BitMode]>;
>>>>>>>>  let Predicates = [HasXSAVEOPT] in {
>>>>>>>>    def XSAVEOPT : I<0xAE, MRM6m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                     "xsaveopt\t$dst",
>>>>>>>> -                   [(int_x86_xsaveopt addr:$dst, EDX, EAX)]>, PS;
>>>>>>>> +                   [(int_x86_xsaveopt addr:$dst, EDX, EAX)]>, PS,
>>>>>>>> Requires<[HasXSAVEOPT]>;
>>>>>>>>    def XSAVEOPT64 : RI<0xAE, MRM6m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                        "xsaveopt64\t$dst",
>>>>>>>> -                      [(int_x86_xsaveopt64 addr:$dst, EDX, EAX)]>,
>>>>>>>> PS, Requires<[In64BitMode]>;
>>>>>>>> +                      [(int_x86_xsaveopt64 addr:$dst, EDX, EAX)]>,
>>>>>>>> PS, Requires<[HasXSAVEOPT, In64BitMode]>;
>>>>>>>>  }
>>>>>>>>  let Predicates = [HasXSAVEC] in {
>>>>>>>>    def XSAVEC : I<0xC7, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                   "xsavec\t$dst",
>>>>>>>> -                 [(int_x86_xsavec addr:$dst, EDX, EAX)]>, TB;
>>>>>>>> +                 [(int_x86_xsavec addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[HasXSAVEC]>;
>>>>>>>>    def XSAVEC64 : RI<0xC7, MRM4m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                     "xsavec64\t$dst",
>>>>>>>> -                   [(int_x86_xsavec64 addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[In64BitMode]>;
>>>>>>>> +                   [(int_x86_xsavec64 addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[HasXSAVEC, In64BitMode]>;
>>>>>>>>  }
>>>>>>>>  let Predicates = [HasXSAVES] in {
>>>>>>>>    def XSAVES : I<0xC7, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                   "xsaves\t$dst",
>>>>>>>> -                 [(int_x86_xsaves addr:$dst, EDX, EAX)]>, TB;
>>>>>>>> +                 [(int_x86_xsaves addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[HasXSAVES]>;
>>>>>>>>    def XSAVES64 : RI<0xC7, MRM5m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                      "xsaves64\t$dst",
>>>>>>>> -                    [(int_x86_xsaves64 addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[In64BitMode]>;
>>>>>>>> +                    [(int_x86_xsaves64 addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[HasXSAVE, In64BitMode]>;
>>>>>>>>    def XRSTORS : I<0xC7, MRM3m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                    "xrstors\t$dst",
>>>>>>>> -                  [(int_x86_xrstors addr:$dst, EDX, EAX)]>, TB;
>>>>>>>> +                  [(int_x86_xrstors addr:$dst, EDX, EAX)]>, TB,
>>>>>>>> Requires<[HasXSAVES]>;
>>>>>>>>    def XRSTORS64 : RI<0xC7, MRM3m, (outs), (ins opaque512mem:$dst),
>>>>>>>>                       "xrstors64\t$dst",
>>>>>>>> -                     [(int_x86_xrstors64 addr:$dst, EDX, EAX)]>,
>>>>>>>> TB, Requires<[In64BitMode]>;
>>>>>>>> +                     [(int_x86_xrstors64 addr:$dst, EDX, EAX)]>,
>>>>>>>> TB, Requires<[HasXSAVES, In64BitMode]>;
>>>>>>>>  }
>>>>>>>>  } // Uses
>>>>>>>>  } // SchedRW
>>>>>>>>
>>>>>>>> Modified: llvm/trunk/test/MC/X86/x86-32-coverage.s
>>>>>>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x
>>>>>>>> 86-32-coverage.s?rev=320830&r1=320829&r2=320830&view=diff
>>>>>>>> ============================================================
>>>>>>>> ==================
>>>>>>>> --- llvm/trunk/test/MC/X86/x86-32-coverage.s (original)
>>>>>>>> +++ llvm/trunk/test/MC/X86/x86-32-coverage.s Fri Dec 15 09:22:58
>>>>>>>> 2017
>>>>>>>> @@ -10601,35 +10601,27 @@ blendvps %xmm0, (%eax), %xmm1
>>>>>>>>  // CHECK: btl $4, (%eax)
>>>>>>>>  // CHECK: btw $4, (%eax)
>>>>>>>>  // CHECK: btl $4, (%eax)
>>>>>>>> -// CHECK: btq $4, (%eax)
>>>>>>>>  // CHECK: btsl $4, (%eax)
>>>>>>>>  // CHECK: btsw $4, (%eax)
>>>>>>>>  // CHECK: btsl $4, (%eax)
>>>>>>>> -// CHECK: btsq $4, (%eax)
>>>>>>>>  // CHECK: btrl $4, (%eax)
>>>>>>>>  // CHECK: btrw $4, (%eax)
>>>>>>>>  // CHECK: btrl $4, (%eax)
>>>>>>>> -// CHECK: btrq $4, (%eax)
>>>>>>>>  // CHECK: btcl $4, (%eax)
>>>>>>>>  // CHECK: btcw $4, (%eax)
>>>>>>>>  // CHECK: btcl $4, (%eax)
>>>>>>>> -// CHECK: btcq $4, (%eax)
>>>>>>>>  bt $4, (%eax)
>>>>>>>>  btw $4, (%eax)
>>>>>>>>  btl $4, (%eax)
>>>>>>>> -btq $4, (%eax)
>>>>>>>>  bts $4, (%eax)
>>>>>>>>  btsw $4, (%eax)
>>>>>>>>  btsl $4, (%eax)
>>>>>>>> -btsq $4, (%eax)
>>>>>>>>  btr $4, (%eax)
>>>>>>>>  btrw $4, (%eax)
>>>>>>>>  btrl $4, (%eax)
>>>>>>>> -btrq $4, (%eax)
>>>>>>>>  btc $4, (%eax)
>>>>>>>>  btcw $4, (%eax)
>>>>>>>>  btcl $4, (%eax)
>>>>>>>> -btcq $4, (%eax)
>>>>>>>>
>>>>>>>>  // CHECK: clflushopt   3735928559(%ebx,%ecx,8)
>>>>>>>>  // CHECK:  encoding: [0x66,0x0f,0xae,0xbc,0xcb,0xef
>>>>>>>> ,0xbe,0xad,0xde]
>>>>>>>>
>>>>>>>>
>>>>>>>> _______________________________________________
>>>>>>>> llvm-commits mailing list
>>>>>>>> llvm-commits at lists.llvm.org
>>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> --
>>>>>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>>>>>> 408-460-2413 <(408)%20460-2413>
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> llvm-commits mailing list
>>>>>>> llvm-commits at lists.llvm.org
>>>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>>> 408-460-2413 <(408)%20460-2413>
>>>>
>>>
>>>
>>>
>>> --
>>> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
>>> 408-460-2413 <(408)%20460-2413>
>>>
>>
>>
>
>
> --
> Teresa Johnson |  Software Engineer |  tejohnson at google.com |
> 408-460-2413 <(408)%20460-2413>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171215/c89d528d/attachment.html>


More information about the llvm-commits mailing list