Skip to content

Provide initial definition for __remill_sync_hyper_call#611

Merged
pgoodman merged 29 commits intomasterfrom
alex/x86-sync-hyper-call
Aug 22, 2022
Merged

Provide initial definition for __remill_sync_hyper_call#611
pgoodman merged 29 commits intomasterfrom
alex/x86-sync-hyper-call

Conversation

@tetsuo-cpp
Copy link
Contributor

@tetsuo-cpp tetsuo-cpp commented Jul 28, 2022

This PR is providing an initial implementation of __remill_sync_hyper_call. I'm aiming to handle what I can and, for anything that isn't feasible, calling into a more specific intrinsic like __remill_x86_some_intrinsic_func.

This is to reduce the number of state escapes that happen when lifting challenges.

@tetsuo-cpp tetsuo-cpp requested a review from pgoodman July 28, 2022 06:01
@tetsuo-cpp tetsuo-cpp marked this pull request as draft July 28, 2022 06:01
@tetsuo-cpp
Copy link
Contributor Author

tetsuo-cpp commented Jul 28, 2022

This is incomplete, I just wanted to ask some questions and check that this approach is looking ok.

The example that we looked at that had a cpuid instruction is able to remove references to State with this change though! 👍

asm volatile("wbinvd" :);
break;

// TODO(alex): There doesn't seem to be a way to figure out what what value
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pgoodman Do you know the reason behind this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I'd have to look at the arch docs to see how this instruction works.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one doesn't map to an instruction. It just gets as part of other instructions that eventually write to a segment register. What I don't understand is that the State structure has these segment registers.

Here's an instruction where it uses the hyper call:
https://github.com/lifting-bits/remill/blob/master/lib/Arch/X86/Semantics/POP.cpp#L163-L171

And here are the segment registers:
https://github.com/lifting-bits/remill/blob/master/include/remill/Arch/X86/Runtime/State.h#L463-L476

There are also some other registers called es_base, ss_base, etc. These ones actually get written to in some of the semantic functions:
https://github.com/lifting-bits/remill/blob/master/include/remill/Arch/X86/Runtime/State.h#L672-L685
https://github.com/lifting-bits/remill/blob/master/lib/Arch/X86/Semantics/CALL_RET.cpp#L26

I'm not sure what the distinction between the base and non-base variants is because the segment registers are also meant to hold the "base" address of the segment.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my memory, base address of the segment is "hidden." For practical purposes, we need to know it, though, as that hidden base is used in calculations what involve implicit or explicit segment register use.

WritePtr<addr_t>(next_sp _IF_32BIT(REG_SS_BASE))

That WritePtr is sort of a fancy cast, and conditional use of ss_base is saying "maybe displace by ss_base, i.e. next_sp + ss_base, so it's actually read in this instance.

I think I'd need to see the instructions that you're working with to put the picture together and contextualize this a bit more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd need to see the instructions that you're working with to put the picture together and contextualize this a bit more.

I'm not seeing these lifted in an example, I'm just going through each hyper call type and trying to implement it.

asm volatile("wbinvd" :);
break;

// TODO(alex): There doesn't seem to be a way to figure out what what value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I'd have to look at the arch docs to see how this instruction works.

@tetsuo-cpp tetsuo-cpp requested a review from pgoodman August 5, 2022 15:09
@tetsuo-cpp
Copy link
Contributor Author

I'm trying to figure out what this is doing: https://github.com/lifting-bits/remill/blob/master/cmake/BCCompiler.cmake#L168

It looks like I'll have to add something to here to compile the runtimes for the architecture they're for. At the moment, the inline assembly for non-x86 architectures doesn't work for this reason.

@pgoodman
Copy link
Contributor

pgoodman commented Aug 8, 2022

@tetsuo-cpp that looks like a tricky issue to solve, especially with Apple Clang. That used to be a viable target. Over time, I've tried to add a few things to Runtime/, e.g. Float.h, that would try to remove or factor out dependence on system libraries, due to issues in trying to do cross-compilation. I think I took it all a bit further even in my unfinished msp430 branch in this repo. At the end of the day, you might have to fix the target triple (what that string is) to something linux based. I think we have methods with hard-coded platform-specific triples in remill, e.g. Arch::DefaultTriple or something like that. You could try one of those strings here, and then evaluate what breaks. This might be better suited toward a different issue / PR, though.

@tetsuo-cpp tetsuo-cpp force-pushed the alex/x86-sync-hyper-call branch from d06e9a1 to fcd24b2 Compare August 10, 2022 02:30
@tetsuo-cpp tetsuo-cpp changed the title Provide definition for __remill_sync_hyper_call Provide initial definition for __remill_sync_hyper_call Aug 12, 2022
@tetsuo-cpp
Copy link
Contributor Author

tetsuo-cpp commented Aug 12, 2022

Ok, my approach has changed significantly since last time. In order to implement some of these hyper calls (cpuid for instance), the only realistic option is to invoke it via inline assembly and then get Clang to give us bitcode for it.

In order to do this, I now have to cross-compile this part of the runtime. Because I'm cross-compiling, I now don't have access to standard library headers which is why there's a lot of code under lib/Arch/Runtime/ to fill in these gaps. Most of this code is a tweaked version of @pgoodman's code in his https://github.com/lifting-bits/remill/tree/msp430 branch. I specifically have chosen to cross-compile just the hyper calls and not the entire runtime to reduce the amount of functionality we have to recreate. For example, the runtimes make use of various STL containers like std::bitset and stuff from <algorithm>. Our build is currently already invoking llvm-link to stitch together each of the bitcode files to make a final runtime bitcode file.

If we agree, I'd prefer to not be exhaustive with the hyper calls right now. If there are more that I could realistically handle, I'd like to follow up with extra PRs for those, since this PR already contains a lot of machinery to even get us to the starting blocks.

@tetsuo-cpp tetsuo-cpp requested a review from pgoodman August 12, 2022 16:21
@tetsuo-cpp tetsuo-cpp marked this pull request as ready for review August 12, 2022 16:21
@tetsuo-cpp
Copy link
Contributor Author

Seems like I'll need to bring Limits.h across too judging from that CI failure. Not sure how it's building fine on my machine. Will look into it more tomorrow.

But the general approach will stay the same.

@tetsuo-cpp
Copy link
Contributor Author

tetsuo-cpp commented Aug 15, 2022

build_linux is failing in master due to #616. I think this is good to go.

@tetsuo-cpp
Copy link
Contributor Author

Something is wrong with the llvm-link step. The __remill_sync_hyper_call implementation isn't getting picked up for some reason. I'm in the middle of debugging this.

@tetsuo-cpp tetsuo-cpp force-pushed the alex/x86-sync-hyper-call branch from 460171f to 35a9723 Compare August 19, 2022 10:22

#endif

default: break;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, I can't abort here anymore since I don't have access to the standard library.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

__builtin_unreachable();?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tetsuo-cpp tetsuo-cpp requested a review from pgoodman August 19, 2022 11:56
@pgoodman
Copy link
Contributor

I agree about moving some stuff to a future PR. I think that you've rightly found that the pre-existing hypercall mechanism isn't flexible enough, and that where added flexibility is needed, new hyper call intrinsics should be introduced. You've done this, e.g. with setting segment registers, but then they are being called inside __remill_sync_hyper_call, and a future PR should instead just call those new intrinsics in the semantics rather than indirecting through __remill_sync_hyper_call. Realistically, maybe all of the enums should go, and then we'd just have a bunch of different intrinsic calls. Again, separate PR for that.

@pgoodman pgoodman merged commit fe6d427 into master Aug 22, 2022
@pgoodman pgoodman deleted the alex/x86-sync-hyper-call branch August 22, 2022 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants