Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bool LivenessAnalyzer::query(Location loc, Type type, const MachRegister& machReg, bool &live) not mapping valid register name on ppc64 #1165

Open
wcohen opened this issue Dec 3, 2021 · 4 comments
Labels
dataflowAPI This issue is directly related to dataflowAPI PowerPC user-reported This issue was reported by someone outside of the core Dyninst developers

Comments

@wcohen
Copy link
Contributor

wcohen commented Dec 3, 2021

I was trying to track down the cause for MachRegister ppc64::r3 not being properly indexed the following code in dataflowAPI/src/liveness.C:

bool LivenessAnalyzer::query(Location loc, Type type, const MachRegister& machReg, bool &live){
bitArray liveRegs;
if (query(loc, type, liveRegs)){
int index = getIndex(machReg);
assert(index >= 0);
live = liveRegs[index];
return true;
}
return false;

}

I would expect that the valid register name would have an entry in the bit vector, but it results in throwing the assert:

stap: /builddir/build/BUILD/dyninst-11.0.0/dyninst-11.0.0/dataflowAPI/src/liveness.C:418: bool LivenessAnalyzer::query(Dyninst::ParseAPI::Location, LivenessAnalyzer::Type, const Dyninst::MachRegister&, bool&): Assertion `index >= 0' failed.

Seeing this with the newly released systemtap-4.6 that is using dyninst liveness analysis to warn if a register modified in guru mode will have no effect. On a rhel8 system with new systemtap-4.6 one can reproduce with:

stap -gvp4 -e 'probe kernel.function("*do_fork").return {$return = -1}'

Pass 1: parsed user script and 509 library scripts using 143488virt/108224res/21568shr/84096data kb, in 760usr/20sys/776real ms.
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
can't find 0x28080310
stap: /builddir/build/BUILD/dyninst-11.0.0/dyninst-11.0.0/dataflowAPI/src/liveness.C:418: bool LivenessAnalyzer::query(Dyninst::ParseAPI::Location, LivenessAnalyzer::Type, const Dyninst::MachRegister&, bool&): Assertion `index >= 0' failed.
Aborted (core dumped)

I have been looking through the dyninst code to see if there might be some reason that the ppc64 valid register doesn't get mapped properly. I haven't found a specific cause, but I did run across the following in common/src/dyn_regs.C:267 that looks questionable as it mixes ppc64 and aarch64 stuff:

    case Arch_ppc64: {
                         if((reg & 0x00ff0000) == aarch64::FPR)
                             return 16;
                         return 8;
                     }
@wcohen
Copy link
Contributor Author

wcohen commented Dec 3, 2021

Doing some more digging it appears that dyninst is getting confused about whether the code is 32-bit or 64-bit. The liveness analysis is being done in 64-bit mode based on information obtained from the following line in the systemtap analyzer.

https://sourceware.org/git/?p=systemtap.git;a=blob;f=analysis.cxx;h=a7a579eb9d984fab4d29d837ba7ffa71c518813b;hb=HEAD#l230

However, when the register is being mapped to an index by https://github.com/dyninst/dyninst/blob/master/dataflowAPI/src/ABI.C#L58 it appears to be using 32-bit register names (note addr_width = 4 and the register names would be 0x240000):

(gdb) s
ABI::getIndex (this=0x1254debb0, machReg=...)
    at /usr/src/debug/dyninst-11.0.0-3.el8.ppc64le/dyninst-11.0.0/dataflowAPI/src/ABI.C:59
59		if (index->find(machReg) == index->end()){
(gdb) print this
$36 = (ABI * const) 0x1254debb0
(gdb) print *this
$37 = {static globalABI_ = 0x125b35800, static globalABI64_ = 0x1254debb0, 
  index = 0x128286cf0, addr_width = 4, static callRead_ = 0x12d3cfdb0, 
  static callRead64_ = 0x12d3d1f40, static callWritten_ = 0x12d3cfde0, 
  static callWritten64_ = 0x12d3d1f70, static returnRead_ = 0x12d3cfd80, 
  static returnRead64_ = 0x12d3d1f10, static returnRegs_ = 0x12d3cfd20, 
  static returnRegs64_ = 0x12d3d1eb0, static callParam_ = 0x12d3cfd50, 
  static callParam64_ = 0x12d3d1ee0, static syscallRead_ = 0x12d3cfe10, 
  static syscallRead64_ = 0x12d3d1fa0, static syscallWritten_ = 0x12d3cfe40, 
  static syscallWritten64_ = 0x12d3d1fd0, static allRegs_ = 0x12d3cfe70, 
  static allRegs64_ = 0x12d3d2000}
(gdb) print this->index
$38 = (std::map<Dyninst::MachRegister, int, std::less<Dyninst::MachRegister>, std::allocator<std::pair<Dyninst::MachRegister const, int> > > *) 0x128286cf0
(gdb) print *this->index
$39 = std::map with 171 elements = {[{reg = 604045312}] = 0, [{
    reg = 604045313}] = 1, [{reg = 604045314}] = 2, [{reg = 604045315}] = 3, [{
    reg = 604045316}] = 4, [{reg = 604045317}] = 5, [{reg = 604045318}] = 6, [{
    reg = 604045319}] = 7, [{reg = 604045320}] = 8, [{reg = 604045321}] = 9, [{
    reg = 604045322}] = 10, [{reg = 604045323}] = 11, [{^CPython Exception <class 'KeyboardInterrupt'> <class 'KeyboardInterrupt'>: 

    reg = 604045324}] = 12...}
(gdb) print/x 604045315
$40 = 0x24010003

@wcohen
Copy link
Contributor Author

wcohen commented Dec 3, 2021

The code in ABI.C doesn't honor whether it is 32-bit or 64-bit code being analyzed:

https://github.com/dyninst/dyninst/blob/master/dataflowAPI/src/ABI.C#L80


#if defined(arch_power)
	globalABI64_->addr_width = 4;
	globalABI_->index = &machRegIndex_ppc();
	globalABI64_->index = &machRegIndex_ppc();

#endif

@wcohen
Copy link
Contributor Author

wcohen commented Dec 3, 2021

For ppc use of dyninst are the 32-bit register names used for both 32- and 64-bit? It wasn't clear from the commit that created that code in ABI.c whether that is the case:

commit 0bc9696
Author: Xiaozhu Meng xmeng@coriander.cs.wisc.edu
Date: Tue Nov 15 10:50:06 2011 -0600

Prototype of DataflowAPI liveness analysis

@hainest
Copy link
Contributor

hainest commented Dec 3, 2021

@wcohen

The code in ABI.C doesn't honor whether it is 32-bit or 64-bit code being analyzed:

This is a known issue (see #1142) that I recently encountered, as well. The problem is that the ppc64le implementation assumes that it can use the ppc32le implementation without issue. For the mos part, this is okay, but there are corner cases where it doesn't work (as you have seen). Converting completely to ppc64le will take time since the two implementations are so woven together.

@hainest hainest added dataflowAPI This issue is directly related to dataflowAPI PowerPC user-reported This issue was reported by someone outside of the core Dyninst developers labels Feb 23, 2022
@hainest hainest added this to To Do in Migrate to 64-bit PowerPC via automation May 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dataflowAPI This issue is directly related to dataflowAPI PowerPC user-reported This issue was reported by someone outside of the core Dyninst developers
Development

No branches or pull requests

2 participants