lifting-bits / remill

Library for lifting machine code to LLVM bitcode
Apache License 2.0
1.27k stars 145 forks source link

PowerPC Support #645

Closed tetsuo-cpp closed 1 year ago

tetsuo-cpp commented 1 year ago

@Ninja3047 Let me know if 648a466 fixes the UB in the shift instructions. I wasn't sure what instruction you tested.

Ninja3047 commented 1 year ago

Found an issue that i fixed here https://github.com/lifting-bits/remill/pull/644/commits/4da65bb89ac4cf50011dc2c3430d3a6b8c471577 although it looks like it doesn't handle the case where the shift amount is negative

Also fixed another issue with INT2FLOAT, it's supposed to output the appropriate sized float based on the output size https://github.com/lifting-bits/remill/pull/644/commits/5424abf19ca302a419ef8c20a21921a9dc501533

Ninja3047 commented 1 year ago

Unique ptrs in the entry block appears to work though 🎉

tetsuo-cpp commented 1 year ago

Found an issue that i fixed here 4da65bb

Oops thanks for the fix.

although it looks like it doesn't handle the case where the shift amount is negative

Have you found negative shift amounts being lifted? The PCode reference indicates that it should be unsigned.

Ninja3047 commented 1 year ago

Haven't seen any examples of this occurring but i was looking at how they implement the semantic behavior in Ghidra itself and they seem to also check if it's negative. I don't think it matters either way since we probably won't run into it in practice. https://github.com/NationalSecurityAgency/ghidra/blob/master/Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/pcode/opbehavior/OpBehaviorIntLeft.java#L33

tetsuo-cpp commented 1 year ago

@2over12 I'm passing the VLE value into Sleigh with 639bd99. This should be cleaner than what I was originally doing.

tetsuo-cpp commented 1 year ago

@2over12 I'm thinking that we should rename the target branch to patchable-ir-main, similar to what we're doing with Anvill.

tetsuo-cpp commented 1 year ago

@Ninja3047 What happened with e4a4b74? Can we no longer support LLVM 14?

Edit: Ok, I saw the Slack conversation. I'll remove the ifdefs around LLVM_VERSION since we don't need those anymore.

tetsuo-cpp commented 1 year ago

I believe I've addressed everything so far. Any comment that I wasn't sure about or requires a second opinion has been left unresolved. You might have to click the "load more" above to see the comments since there's a lot of activity on this PR.