Skip to content

Commit

Permalink
Fix MachRegister bool checks (#1613)
Browse files Browse the repository at this point in the history
The logic between the getRegisterX and isRegisterX members diverged over time. This implements the isRegisterX in terms of the getRegisterX while preserving extra checks where necessary.

* Write 'isPC' in terms of 'getPC'

* Add missing architectures in getFramePointer

* Don't assert in getFramePointer

* Write isFramePointer in terms of getFramePointer

This also adds correct detection of frame pointers on PPC.

* Add missing arch in getStackPointer

* Don't assert in getStackPointer

* Reorder checks in getStackPointer

For consistency

* Write isStackPointer in terms of getStackPointer

This also now includes StackTop.

* isFramePointer ws

* isPC ws

* isStackPointer typo

* Don't assert in getSyscallNumberReg

* Add missing arch in getSyscallNumberReg

* Write isSyscallNumberReg in terms of getSyscallNumberReg

The original implementation in 7b8d777 from 2013 used o{r,e}ax for
x86, but was changed to use {r,e}ax by 23a5a76 in 2015. Neither
the SystemV ABI nor Intel Dev Guide refer to o*ax, so I think this
check is now correct.

* Don't assert in getSyscallReturnValueReg

* Add missing arches in getSyscallReturnValueReg

* Write isSyscallReturnValueReg in terms of getSyscallReturnValueReg

These two had become completely unsynchronized. There is a reg for
aarch64 and both PPC registers were wrong in the bool check.

* Reorder checks in getZeroFlag

For consistency

* Don't assert in getZeroFlag

* Add missing arch in getZeroFlag

* Write isZeroFlag in terms of getZeroFlag
  • Loading branch information
hainest committed Oct 31, 2023
1 parent 362d5f8 commit 209121a
Showing 1 changed file with 32 additions and 30 deletions.
62 changes: 32 additions & 30 deletions common/src/registers/MachRegister.C
Expand Up @@ -340,12 +340,13 @@ namespace Dyninst {
case Arch_ppc32: return ppc32::r1;
case Arch_ppc64: return ppc64::r1;
case Arch_aarch64: return aarch64::x29; // aarch64: frame pointer is X29 by convention
case Arch_aarch32:
case Arch_cuda:
case Arch_intelGen9:
case Arch_amdgpu_gfx908:
case Arch_amdgpu_gfx90a:
case Arch_amdgpu_gfx940:
case Arch_none: return InvalidReg;

default: assert(0); return InvalidReg;
}
return InvalidReg;
}
Expand All @@ -358,12 +359,12 @@ namespace Dyninst {
case Arch_ppc64: return ppc64::r1;
case Arch_aarch64: return aarch64::sp; // aarch64: stack pointer is an independent register
case Arch_aarch32:
case Arch_cuda: assert(0); break;
case Arch_none:
case Arch_cuda:
case Arch_intelGen9:
case Arch_amdgpu_gfx908:
case Arch_amdgpu_gfx90a:
case Arch_amdgpu_gfx940: return InvalidReg;
default: assert(0); return InvalidReg;
case Arch_amdgpu_gfx940:
case Arch_none: return InvalidReg;
}
return InvalidReg;
}
Expand All @@ -377,11 +378,11 @@ namespace Dyninst {
case Arch_aarch64: return aarch64::x8;
case Arch_aarch32:
case Arch_cuda:
case Arch_intelGen9:
case Arch_amdgpu_gfx908:
case Arch_amdgpu_gfx90a:
case Arch_amdgpu_gfx940: assert(0); break;
case Arch_amdgpu_gfx940:
case Arch_none: return InvalidReg;
default: assert(0); return InvalidReg;
}
return InvalidReg;
}
Expand All @@ -406,8 +407,13 @@ namespace Dyninst {
case Arch_ppc32: return ppc32::r3;
case Arch_ppc64: return ppc64::r3;
case Arch_aarch64: return aarch64::x0; // returned value is save in x0
case Arch_aarch32:
case Arch_cuda:
case Arch_intelGen9:
case Arch_amdgpu_gfx908:
case Arch_amdgpu_gfx90a:
case Arch_amdgpu_gfx940:
case Arch_none: return InvalidReg;
default: assert(0); return InvalidReg;
}
return InvalidReg;
}
Expand Down Expand Up @@ -436,47 +442,44 @@ namespace Dyninst {
switch(arch) {
case Arch_x86: return x86::zf;
case Arch_x86_64: return x86_64::zf;
case Arch_aarch64: return aarch64::z;
case Arch_aarch32: assert(!"Not implemented"); break;
case Arch_ppc32: return ppc32::cr0e;
case Arch_ppc64: return ppc64::cr0e;
case Arch_aarch64: return aarch64::z;
case Arch_aarch32:
case Arch_cuda:
case Arch_intelGen9:
case Arch_amdgpu_gfx908:
case Arch_amdgpu_gfx90a:
case Arch_amdgpu_gfx940: assert(0); break;
case Arch_amdgpu_gfx940:
case Arch_none: return InvalidReg;
default: return InvalidReg;
}

return InvalidReg;
}

bool MachRegister::isPC() const {
return (*this == x86_64::rip || *this == x86::eip || *this == ppc32::pc || *this == ppc64::pc ||
*this == aarch64::pc || *this == amdgpu_gfx908::pc_all ||
*this == amdgpu_gfx90a::pc_all || *this == amdgpu_gfx940::pc_all);
if(*this == InvalidReg) return false;
return *this == getPC(getArchitecture());
}

bool MachRegister::isFramePointer() const {
return (*this == x86_64::rbp || *this == x86::ebp || *this == FrameBase ||
*this == aarch64::x29);
if(*this == InvalidReg) return false;
return *this == FrameBase || *this == getFramePointer(getArchitecture());
}

bool MachRegister::isStackPointer() const {
return (*this == x86_64::rsp || *this == x86::esp || *this == ppc32::r1 || *this == ppc64::r1 ||
*this == aarch64::sp);
if(*this == InvalidReg) return false;
return *this == StackTop || *this == getStackPointer(getArchitecture());
}

bool MachRegister::isSyscallNumberReg() const {
return (*this == x86_64::orax || *this == x86::oeax || *this == ppc32::r1 ||
*this == ppc64::r1 || *this == aarch64::x8);
if(*this == InvalidReg) return false;
return *this == getSyscallNumberReg(getArchitecture());
}

bool MachRegister::isSyscallReturnValueReg() const {
if(getArchitecture() == Arch_aarch64)
assert(0);
return (*this == x86_64::rax || *this == x86::eax || *this == ppc32::r1 || *this == ppc64::r1 ||
*this == aarch64::x0);
if(*this == InvalidReg) return false;
return *this == getSyscallReturnValueReg(getArchitecture());
}

bool MachRegister::isFlag() const {
Expand Down Expand Up @@ -506,10 +509,8 @@ namespace Dyninst {
}

bool MachRegister::isZeroFlag() const {
if(*this == InvalidReg) return false;
switch(getArchitecture()) {
case Arch_x86: return *this == x86::zf;
case Arch_x86_64: return *this == x86_64::zf;
case Arch_aarch64: return *this == aarch64::z;
case Arch_ppc32:
case Arch_ppc64: {
// For power, we have a different register representation.
Expand All @@ -519,7 +520,8 @@ namespace Dyninst {
return (baseID <= 731 && baseID >= 700 && baseID % 4 == 2) ||
(baseID <= 628 && baseID >= 621);
}
default: assert(!"Not implemented!");
default:
return *this == getZeroFlag(getArchitecture());
}
return false;
}
Expand Down

0 comments on commit 209121a

Please sign in to comment.