[llvm-commits] [llvm] r81422 - in /llvm/trunk/lib/Target/ARM: ARMCodeEmitter.cpp ARMJITInfo.cpp

[Evan Cheng]

Ah actually, it looks like we are not distinguishing data indirect symbol from function stub (which is bad).

In emitFunctionStub(), I see:
      if (!LazyPtr) {
        // In PIC mode, the function stub is loading a lazy-ptr.                                                                                                                                  
        LazyPtr= (intptr_t)emitGlobalValueIndirectSym((GlobalValue*)F, Fn, JCE);
        DEBUG(if (F)
                errs() << "JIT: Indirect symbol emitted at [" << LazyPtr
                       << "] for GV '" << F->getName() << "'\n";
              else
                errs() << "JIT: Stub emitted at [" << LazyPtr
                       << "] for external function at '" << Fn << "'\n");
      }

The function stub does need to be marked executable.

Evan

On Nov 23, 2009, at 6:04 PM, Evan Cheng wrote:

> It doesn't. Looks like a copy+paste bug.
> 
> Evan
> 
> On Nov 23, 2009, at 4:53 PM, Jeffrey Yasskin wrote:
> 
>> Hi Evan. Why does ARM need to mark the indirect symbol executable?
>> It's loading from that address not executing it.
>> 
>> Is there a test that the change to ARMJITInfo.cpp fixes?
>> 
>> On Wed, Sep 9, 2009 at 5:23 PM, Evan Cheng <evan.cheng at apple.com> wrote:
>>> Author: evancheng
>>> Date: Wed Sep  9 20:23:53 2009
>>> New Revision: 81422
>>> 
>>> URL: http://llvm.org/viewvc/llvm-project?rev=81422&view=rev
>>> Log:
>>> Proper support of non-lazy indirect symbols.
>>> 
>>> Modified:
>>>   llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
>>>   llvm/trunk/lib/Target/ARM/ARMJITInfo.cpp
>>> 
>>> Modified: llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp?rev=81422&r1=81421&r2=81422&view=diff
>>> 
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp (original)
>>> +++ llvm/trunk/lib/Target/ARM/ARMCodeEmitter.cpp Wed Sep  9 20:23:53 2009
>>> @@ -60,6 +60,7 @@
>>>    ARMJITInfo                *JTI;
>>>    const ARMInstrInfo        *II;
>>>    const TargetData          *TD;
>>> +    const ARMSubtarget        *Subtarget;
>>>    TargetMachine             &TM;
>>>    CodeEmitter               &MCE;
>>>    const std::vector<MachineConstantPoolEntry> *MCPEs;
>>> @@ -163,7 +164,7 @@
>>>    /// Routines that handle operands which add machine relocations which are
>>>    /// fixed up by the relocation stage.
>>>    void emitGlobalAddress(GlobalValue *GV, unsigned Reloc,
>>> -                           bool NeedStub, intptr_t ACPV = 0);
>>> +                           bool NeedStub,  bool Indirect, intptr_t ACPV = 0);
>>>    void emitExternalSymbolAddress(const char *ES, unsigned Reloc);
>>>    void emitConstPoolAddress(unsigned CPI, unsigned Reloc);
>>>    void emitJumpTableAddress(unsigned JTIndex, unsigned Reloc);
>>> @@ -195,9 +196,10 @@
>>>  assert((MF.getTarget().getRelocationModel() != Reloc::Default ||
>>>          MF.getTarget().getRelocationModel() != Reloc::Static) &&
>>>         "JIT relocation model must be set to static or default!");
>>> +  JTI = ((ARMTargetMachine&)MF.getTarget()).getJITInfo();
>>>  II = ((ARMTargetMachine&)MF.getTarget()).getInstrInfo();
>>>  TD = ((ARMTargetMachine&)MF.getTarget()).getTargetData();
>>> -  JTI = ((ARMTargetMachine&)MF.getTarget()).getJITInfo();
>>> +  Subtarget = &TM.getSubtarget<ARMSubtarget>();
>>>  MCPEs = &MF.getConstantPool()->getConstants();
>>>  MJTEs = &MF.getJumpTableInfo()->getJumpTables();
>>>  IsPIC = TM.getRelocationModel() == Reloc::PIC_;
>>> @@ -244,7 +246,7 @@
>>>  else if (MO.isImm())
>>>    return static_cast<unsigned>(MO.getImm());
>>>  else if (MO.isGlobal())
>>> -    emitGlobalAddress(MO.getGlobal(), ARM::reloc_arm_branch, true);
>>> +    emitGlobalAddress(MO.getGlobal(), ARM::reloc_arm_branch, true, false);
>>>  else if (MO.isSymbol())
>>>    emitExternalSymbolAddress(MO.getSymbolName(), ARM::reloc_arm_branch);
>>>  else if (MO.isCPI()) {
>>> @@ -270,9 +272,14 @@
>>> ///
>>> template<class CodeEmitter>
>>> void Emitter<CodeEmitter>::emitGlobalAddress(GlobalValue *GV, unsigned Reloc,
>>> -                                             bool NeedStub, intptr_t ACPV) {
>>> -  MCE.addRelocation(MachineRelocation::getGV(MCE.getCurrentPCOffset(), Reloc,
>>> -                                             GV, ACPV, NeedStub));
>>> +                                             bool NeedStub, bool Indirect,
>>> +                                             intptr_t ACPV) {
>>> +  MachineRelocation MR = Indirect
>>> +    ? MachineRelocation::getIndirectSymbol(MCE.getCurrentPCOffset(), Reloc,
>>> +                                           GV, ACPV, NeedStub)
>>> +    : MachineRelocation::getGV(MCE.getCurrentPCOffset(), Reloc,
>>> +                               GV, ACPV, NeedStub);
>>> +  MCE.addRelocation(MR);
>>> }
>>> 
>>> /// emitExternalSymbolAddress - Arrange for the address of an external symbol to
>>> @@ -417,8 +424,11 @@
>>> 
>>>    GlobalValue *GV = ACPV->getGV();
>>>    if (GV) {
>>> +      Reloc::Model RelocM = TM.getRelocationModel();
>>>      emitGlobalAddress(GV, ARM::reloc_arm_machine_cp_entry,
>>> -                        isa<Function>(GV), (intptr_t)ACPV);
>>> +                        isa<Function>(GV),
>>> +                        Subtarget->GVIsIndirectSymbol(GV, RelocM),
>>> +                        (intptr_t)ACPV);
>>>     } else  {
>>>      emitExternalSymbolAddress(ACPV->getSymbol(), ARM::reloc_arm_absolute);
>>>    }
>>> @@ -437,7 +447,7 @@
>>>      });
>>> 
>>>    if (GlobalValue *GV = dyn_cast<GlobalValue>(CV)) {
>>> -      emitGlobalAddress(GV, ARM::reloc_arm_absolute, isa<Function>(GV));
>>> +      emitGlobalAddress(GV, ARM::reloc_arm_absolute, isa<Function>(GV), false);
>>>      emitWordLE(0);
>>>    } else if (const ConstantInt *CI = dyn_cast<ConstantInt>(CV)) {
>>>      uint32_t Val = *(uint32_t*)CI->getValue().getRawData();
>>> 
>>> Modified: llvm/trunk/lib/Target/ARM/ARMJITInfo.cpp
>>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/ARM/ARMJITInfo.cpp?rev=81422&r1=81421&r2=81422&view=diff
>>> 
>>> ==============================================================================
>>> --- llvm/trunk/lib/Target/ARM/ARMJITInfo.cpp (original)
>>> +++ llvm/trunk/lib/Target/ARM/ARMJITInfo.cpp Wed Sep  9 20:23:53 2009
>>> @@ -145,6 +145,9 @@
>>>    llvm_unreachable("ERROR: Unable to mark indirect symbol writable");
>>>  }
>>>  JCE.emitWordLE((intptr_t)Ptr);
>>> +  if (!sys::Memory::setRangeExecutable((void*)Addr, 4)) {
>>> +    llvm_unreachable("ERROR: Unable to mark indirect symbol executable");
>>> +  }
>>>  void *PtrAddr = JCE.finishGVStub(GV);
>>>  addIndirectSymAddr(Ptr, (intptr_t)PtrAddr);
>>>  return PtrAddr;
>>> 
>>> 
>>> _______________________________________________
>>> llvm-commits mailing list
>>> llvm-commits at cs.uiuc.edu
>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>>> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits