[x86] Fix getAddressFromInstr

[mlemay-intel]

Summary

Fix getAddressFromInstr so that it copies displacements of global variable
references.

[mlemay-intel]

ping

[qcolombet]

Hi,

Do you have a test case to go along this?

Thanks,
-Quentin

lib/Target/X86/X86InstrBuilder.h

112–113

Could you describe what is missing?

[mlemay-intel]

I don't have a test case yet, since I have only encountered this issue while developing a pass that I have not yet open-sourced. However, I'll look for where else this routine is used and I'll see if I can generate a test based on that.

[mlemay-intel]

lib/Target/X86/X86InstrBuilder.h

112–113

I'm not familiar with all of the different types of memory operands, but I'm concerned that additional types may not be handled properly here given this issue with handling global address operands. Just to pick one more example, perhaps external symbol operands need special handling, but I'm not sure. Maybe I should revise my comment to "FIXME: Special handling may be required for additional types of operands below."

[mlemay-intel]

Added test.

lib/Target/X86/X86InstrBuilder.h

static inline X86AddressMode getAddressFromInstr(const MachineInstr *MI,
 
   const MachineOperand &Op1 = MI->getOperand(Operand + 1);
   AM.Scale = Op1.getImm();
 
   const MachineOperand &Op2 = MI->getOperand(Operand + 2);
   AM.IndexReg = Op2.getReg();
 
   const MachineOperand &Op3 = MI->getOperand(Operand + 3);
-  if (Op3.isGlobal())
+  // FIXME: Handle additional types of memory references below.
+  if (Op3.isGlobal()) {
     AM.GV = Op3.getGlobal();
-  else
+    AM.Disp = Op3.getOffset();
+  } else
     AM.Disp = Op3.getImm();
 
   return AM;
 }
 
 /// addDirectMem - This function is used to add a direct memory reference to the
 /// current instruction -- that is, a dereference of an address in a register,
 /// with no scale, index or displacement. An example is: DWORD PTR [EAX].

test/CodeGen/X86/safestack-bounds-checking-combiner.mir

entry:
     %6 = getelementptr i8, i8* %5, i32 4
     %7 = getelementptr i8, i8* %6, i64 1
     call void asm "bndcu $0, %bnd0", "*m"(i8* %7)
     store i8 0, i8* %6
     store i8* %unsafe_stack_ptr, i8** @__safestack_unsafe_stack_ptr
     ret void
   }
 
+  %struct.glob_s = type { i32, i32 }
+
+  @x = external local_unnamed_addr global [1 x %struct.glob_s], align 4
+
+  ; Function Attrs: nounwind safestack uwtable
+  define void @glob_access(i32 %i) local_unnamed_addr #0 {
+  entry:
+    %idxprom = sext i32 %i to i64
+    %y = getelementptr inbounds [1 x %struct.glob_s], [1 x %struct.glob_s]* @x, i64 0, i64 %idxprom, i32 0
+    %0 = bitcast i32* %y to i8*
+    %1 = getelementptr i8, i8* %0, i64 4
+    call void asm "bndcu $0, %bnd0", "*m"(i8* %1)
+    store i32 0, i32* %y, align 4
+    %z = getelementptr inbounds [1 x %struct.glob_s], [1 x %struct.glob_s]* @x, i64 0, i64 %idxprom, i32 1
+    %2 = bitcast i32* %z to i8*
+    %3 = getelementptr i8, i8* %2, i64 4
+    call void asm "bndcu $0, %bnd0", "*m"(i8* %3)
+    store i32 0, i32* %z, align 4
+    ret void
+  }
+
   attributes #0 = { nounwind safestack uwtable "target-features"="+mpx,+separate-stack-seg" }
 
 ...
 ---
 name:            overflow_gep_store
+# CHECK-LABEL: name:            overflow_gep_store
 alignment:       4
 exposesReturnsTwice: false
 legalized:       false
 regBankSelected: false
 selected:        false
 tracksRegLiveness: true
 frameInfo:
   isFrameAddressTaken: false
bb.0.entry:
     ; CHECK-NEXT: MOV8mi
     MOV8mi %rcx, 1, _, -4, _, 0 :: (store 1 into %ir.2)
     INLINEASM $"bndcu $0, %bnd0", 8, 196654, %rcx, 1, _, 1, _
     MOV8mi %rcx, 1, _, 0, _, 0 :: (store 1 into %ir.6)
     MOV64mr killed %rax, 1, _, 0, %fs, killed %rcx :: (store 8 into @__safestack_unsafe_stack_ptr)
     RETQ
 
 ...
+---
+name:            glob_access
+# CHECK-LABEL: glob_access
+alignment:       4
+exposesReturnsTwice: false
+legalized:       false
+regBankSelected: false
+selected:        false
+tracksRegLiveness: true
+liveins:
+  - { reg: '%edi' }
+frameInfo:
+  isFrameAddressTaken: false
+  isReturnAddressTaken: false
+  hasStackMap:     false
+  hasPatchPoint:   false
+  stackSize:       0
+  offsetAdjustment: 0
+  maxAlignment:    0
+  adjustsStack:    false
+  hasCalls:        false
+  maxCallFrameSize: 0
+  hasOpaqueSPAdjustment: false
+  hasVAStart:      false
+  hasMustTailInVarArgFunc: false
+body:             |
+  bb.0.entry:
+    liveins: %edi
+
+    %rax = MOVSX64rr32 killed %edi
+    INLINEASM $"bndcu $0, %bnd0", 8, 196654, _, 8, %rax, @x + 4, _
+    ; CHECK: INLINEASM $"bndcu $0, %bnd0", 8, 196654, _, 8, %rax, @x + 8, _
+    MOV32mi _, 8, %rax, @x, _, 0
+    INLINEASM $"bndcu $0, %bnd0", 8, 196654, _, 8, %rax, @x + 8, _
+    ; CHECK-NOT: INLINEASM $"bndcu $0, %bnd0", 8, 196654, _, 8, %rax, @x + 8, _
+    MOV32mi _, 8, killed %rax, @x + 4, _, 0
+    RETQ
+
+...