Update RISCV-64 sleigh files to support vector, bit manipulation, and crypto extensions - #5778
Update RISCV-64 sleigh files to support vector, bit manipulation, and crypto extensions#5778thixotropist wants to merge 9 commits into
Conversation
|
To fix build errors: |
|
I expect to fill in some gaps in this PR shortly. Scalar crypto extensions were skipped even though vector crypto extensions were added. openssl can use RISCV scalar crypto AES extension instructions but not (yet?) the vector crypto extensions. I also hope to add minimalist pcode semantics to allow decompilation of the simplest GCC-14 RISCV builtin intrinsic vector function examples - as used in Ghidra developers will have some serious design questions to thrash out when GCC-14 autovectorization support lands some time next year. |
|
|
||
| # Thead semi's extensions currently recognized by binutils objdump | ||
| # and documented in https://github.com/T-head-Semi/thead-extension-spec/releases/download/2.0.0/xthead-2022-09-05-2.0.0.pdf | ||
| @include "riscv.xthead.sinc" |
There was a problem hiding this comment.
This should be guarded by ifdef, with the define in a new xhead slaspec file
There was a problem hiding this comment.
That's reasonable. Does the new xthead slaspec file get named in riscv.ldefs so the user can invoke it, or do you suggest we generalize riscv.opinion to look for the Tag_RISCV_arch ELF attribute, recognize the current composition of extensions, and set finer-grain inclusion tags?
Tag_RISCV_arch: "rv64i2p1_m2p0_a2p1_f2p2_d2p2_v1p0_zicsr2p0_zifencei2p0_zmmul1p0_zve32f1p0_zve32x1p0_zve64d1p0_zve64f1p0_zve64x1p0_zvl128b1p0_zvl32b1p0_zvl64b1p0_xtheadfmemidx1p0"
Apparently binutils 2.41 concatenates all of the march extensions passed to gcc, so the tail end of this attribute reads something like:
"This binary requires an ISA supporting:"
x- a vendor specific Instruction Set Architecture module not currently part of a proposed standard profilethead- the lower case vendor name publishing the extension setfmemidx- the extension set module name1p0- the version of this extension set, likely1.0
Composing standard and vendor extension profiles raises lots of Ghidra import design questions. Apparently questions the binutils team has already addressed - do their answers work for Ghidra?
Update: These design decisions may lead to significant refactoring or code bloat.
- Does every known RISCV-based CPU get its own slaspec file and 5 MB sla file in the Ghidra distribution?
- even this doesn't always work with chips containing heterogeneous cores
- Should Ghidra scan user-specific directories for additional compiled sla and slaspec files for any RISCV CPU extension combinations individuals find useful?
- Are Ghidra build-time decisions generating slaspec files moved to run-time actions generating temporary sla files after parsing the import's ELF attributes?
- Are a new set of runtime ifdef statements allowed in build-time sla files to enable specific extensions at run time?
Personal opinion:
- ratified extensions with non-conflicting opcode codepoints should continue to be included in the baseline 32 bit and 64 bit slaspec and sla files
- Ghidra does not want to be responsible for recognition of proprietary extensions, but these will surely exist. Searching user directories for slaspec and sla 'plugins' should be enabled just as java and python user directories are enabled.
- The baseline ELF importer may be extended to expose ELF attributes on imports, such as the
Tag_RISCV_archfile attribute containing aggregate extensions used in each compilation unit.
|
THead extensions are now collected into separate slaspec files, which are now referenced in
|
|
@thixotropist I stubbed out some risc-v packed simd instructions and implemented some thead instructions here to fix some issues had with my work, feel free to cherry-pick the commits if you want. |
|
@madushan1000: Those look good - I'll be happy to cherrypick them into the branch. Have you any suggestions for RISCV integration tests to add to https://github.com/thixotropist/ghidra_import_tests? It's currently very weak in 32 bit and microcontroller exemplars, as I've been leaning towards linux-capable 64 bit examples. |
|
This sdk I'm working with has a bunch of rv32 examples, https://github.com/bouffalolab/bouffalo_sdk/tree/master/examples. |
| :vsetvli rd, rs1, "e32,mf2,tu,ma" is op2030=0x097 & op3131=0x0 & rs1 & op1214=0x7 & rd & op0006=0x57 {rd=vsetvli_e32mf2tuma(rs1);} | ||
| :vsetvli rd, rs1, "e64,m1,tu,ma" is op2030=0x098 & op3131=0x0 & rs1 & op1214=0x7 & rd & op0006=0x57 {rd=vsetvli_e64m1tuma(rs1);} | ||
| :vsetvli rd, rs1, "e32,mf8,tu,ma" is op2030=0x09d & op3131=0x0 & rs1 & op1214=0x7 & rd & op0006=0x57 {rd=vsetvli_e64mf8tuma(rs1);} | ||
| :vsetvli rd, rs1, "e32,mf8,tu,ma" is op2030=0x09d & op3131=0x0 & rs1 & op1214=0x7 & rd & op0006=0x57 {rd=vsetvli_e32mf8tuma(rs1);} |
There was a problem hiding this comment.
missing "define pcodeop vsetvli_e32mf8tuma" leads to compile errors:
> Task :RISCV:sleighCompile
Compiling ./data/languages/riscv.ilp32d.slaspec:
riscv.rvv.sinc:3397: unknown macro, userop, or specific symbol 'vsetvli_e32mf8tuma' in macro, user operation, or subpiece application
There was a problem hiding this comment.
Thanks! This should now be fixed with ffb5411.
Any comments on other vset* patterns that might be found in common exemplars would be appreciated. I'm grinding through autovectorized vector instruction sequences generated from https://github.com/ggerganov/llama.cpp currently, and will likely try and make the sinc pcodeop names align a bit better with the gcc riscv intrinsics defined in https://github.com/riscv-non-isa/rvv-intrinsic-doc.
to fix |
|
@jobermayr: Thanks for the fix. I'm pushing your patch |
| fassignS(frd, tmp); | ||
| } | ||
|
|
||
| :fcvt.w.h rdW,frs1S,FRM is frs1S & FRM & rdW & op0001=0x3 & op0204=0x4 & op0506=0x2 & funct7=0x62 & op2024=0x0 |
There was a problem hiding this comment.
There was a problem hiding this comment.
Does it make sense to not using rdW Here as well?
There was a problem hiding this comment.
Thanks - I'll take a look at this tomorrow. I want to look at how qemu handles similar riscv instructions.
Have you any thoughts on how the semantics of vector floating point conversion instructions should be handled? Element size is passed as often-changed SEL context variables, not in the instructions themselves.
Qemu source code spends about 24K lines of C to describe RISCV instruction semantics. I'm not sure how much sleigh pcode that implies, or how to finish what we're starting with emulation.
Update: I'll try getting rid of rdW in riscv.zfh.sinc, following your example. Your changes in PR 6492 fix some errors in analyzing my whisper.cpp exemplar. These updates will likely make for a saner decompiler window but badly confuse anyone trying to use the emulator until someone creates unsigned and fp16 trunc pcode ops and basic sleigh support.
|
I just tried latest version of thixotropist:isa_ext on BL808 BootROM, which is E907 (rv32 thead), and so far, everything looks okay. I still need to play with it more, but so far it was enough for everything I needed. Thanks everyone for this effort, I hope it will be possible to merge it at some point. |
|
This is a very large PR that has had some very active periods of development. I've been waiting to make sure that it was complete and stable enough before reviewing it. Looking through it now though, it looks like it should be ready for a review. |
|
Thanks for the comments - I'd like to see this merged too. The PR may still be in triage because it implicitly makes a lot of design decisions regarding pcode op typing and emulation, as well as ISA extension handling. The developers may need more discussion - public and internal - before they are willing to go down that path. The current state of the PR is stable. There are some newer RISCV ISA extensions for fractional floating point ops and saturating math - I don't plan on adding these to the existing PR, so it can be reviewed as is. As a discussion example, what does the Ghidra community want to see in the decompiler window when working with functions like: // compiled with RISCV march=rv64gcv, -O3, and -ffast-math
void test_1_ref(unsigned long long *in, unsigned long long *out, unsigned int size)
{
int i;
int upper_index = size - 1;
for (i=0; i < size; i++) {
out[i] = in[upper_index - i];
}
}SIMD or vector extensions can turn simple loops over structures into something not so simple. Type inference, compilable C extraction, and emulation in general are all abandoned with the design approach used in this PR. |
|
Is there anything we can do to help @GhidorahRex with this review? For instance
|
|
|
|
@jobermayr is correct. I've added guards as suggested, with additional guards sensitive to quad floating point support. I'll commit these changes shortly. In general, I've used single-precision registers for half-precision FP ops and double-precision registers for quad-precision FP ops as interim semantics. This raises more general questions:
|
|
Would the community ( @mumbel @thixotropist ) have any objection to removing the context register from attachment to the MISA register? In practice, the context register can be used for configuration of a processor, but tying it to a real register with a restricted size and set bitfields won't allow you to add other types of context that might be necessary for disassembly or other purposes. The normal reason to put these processor capability setting bits into the context would be if they were going to be used to configure the processor and restrict disassembly to only the instructions available on the particular processor variant. It looks like maybe this was a considered implementation direction at some point. The current implementation splits the various extensions into .sinc files and uses includes to configure the processor to restrict disassembly. |
|
Removing the link between context and the MISA register makes sense to me. That opens up the big question of what we think we should do with RISC-V context registers and SLEIGH context handling in general.
Here's a use-case to consider: RISC-V kernels or kernel modules include code to decompress or decrypt I/O file system reads and writes. That code can run on processors supporting a broad mix of ISA extensions for bit manipulation, decompression, and decryption. Not every hardware thread in the kernel will necessarily implement the same instructions. The kernel handles that with self-modifying code - how would we want Ghidra's decompiler to show that code? |
|
In general unless there are incompatible collision with instructions or architecture we tend to keep it all in the base. You never know if you are going to get a binary that supports multiple processors and only uses/installs those routines when the processor includes the extension. |
|
Is the general consensus on these changes that they are ready to merge in the baseline? |
|
Current master changed a lot of code. Please proof and rebase. |
|
@thixotropist Yep, the new squashed branch is what I was looking for. |
|
Excellent! When you are doing your code review, please look for ways to improve the vector instruction semantics sections. A lot of vector instructions can execute as read-modify-write ops on the output register or have hidden input dependencies on a vector mask register. That can distort Ghidra's analysis of Varnode dependencies and dead code. I'm slowly collecting examples of where and when that might be an issue. |
|
mask register issues are prevalent throughout SIMD/vector sets. In the AVX512 I tried to mitigate it as best I could through pcode. But with the RISCV and others it's more challenging because the bits don't correspond one-to-one with the bits in the vector registers - they correspond to elements, which aren't always the same size. |
|
Agreed. The only approach I've found is to make sure vector configuration status registers - which set and reset element size and vector register groups frequently - get propagated in MULTIEQUAL ops into the blocks they affect, then rely on a processor-specific decompiler plugin to follow the shifts in element size and vector register multipliers and fixup the pcode dynamically. That's doable in a research proof-of-concept sense, but is it maintainable or consistent with future Ghidra architectural improvements? I've no clue. |
|
@GhidorahRex - Have you an update on your review of this PR? I'd like to add some minor changes to the handling of configuration status register semantics. @emteere may have something useful to say here too, as he made the most recent changes to RISC-V CSR SLEIGH code. The proposed addition is to capture the semantics of certain control and status registers as user defined PCodeOps rather than as general register load and stores. Ghidra's decompiler treats CSR registers as regular global registers, trying much too hard to analyze dependencies by tracking possible locations at which those CSRs may change. For CSR's like Update: It may not be possible to update SLEIGH to convert just a few CSR operations from register ops to user pcode ops - it may be necessary to convert all RISC-V CSR loads and stores or none of them. That's not a minor change, so I don't expect to materially change this PR anytime soon. |
|
Thanks for continuing to support this PR @thixotropist! I recently looked at firmware for a RISCV processor, which used a few instructions of the bit manipulation extension. I noticed your PR implements pretty much all of these by introducing custom pcode ops, instead of implementing them using existing pcode ops. This is a bit unfortunate, since it hinders the decompiler and emulation. I can understand the desire to use custom pcode ops for the bit reverse and trailing-zero-count instructions (see #9078 though - those might become builtin pcode ops), and several other more complex instructions, but I think it'd be a great improvement to this PR if the number of custom pcode ops was kept as low as possible. Was there a specific reason you left these as custom pcode ops? It shouldn't be too much work to rewrite these in terms of existing pcode, but if you want, I can give it a shot. |
|
Great feedback - thanks! The bit manipulation instructions were implemented as custom pcode ops mostly as placeholders, ready to be replaced by anything better. The simpler ones could definitely be implemented as existing pcode. One path forward might be:
In a lot of vector and crypto cases the heavy use of custom pcode ops has another purpose - turning context-sensitive instructions into tokens that a custom decompiler Rule can then match against. It's not too hard to match for vectorized versions of Can you add an issue to ghidra_decompiler_plugins suggesting which bit manipulation instructions you are interested in? Include at least one example of an instruction that you think can't be easily represented in existing pcode ops, but which might be transformed into something clearer in a custom Rule. |
|
Your suggested path forward sounds like a pretty good long-term plan, but it might be a bit too much overhead for "use the builtin
While going through the opcodes, I found that So I think only 6 custom pcode ops are really needed:
That said, having a thorough test bench for decompiler development is definitely very useful, and I think such a test bench would make decompiler development a lot better (and could maybe help streamline the process of improving the decompiler output, though it seems the main bottleneck is getting decompiler PRs merged). If you want, I can also copy/paste (part of) this into an issue at your decompiler plugins repo, but I'm not sure duplicating this information is very helpful. Honestly, of these 6 custom pcode ops, it's mostly the The only unimplemented bit manipulation instructions that my (admittedly tiny) firmware contained were |
|
Let's see if the developers have any suggestions here. You make some great points regarding the RISC-V bit manipulation extension instructions that map cleanly into existing pcode ops. This PR implicitly assumes that binaries using those easy-to-model instructions will also use hard-to-model extensions, and that any SLEIGH-based emulator will choke immediately on the first such instruction it sees. That likely diminishes the long term value of spending effort on SLEIGH-based semantics. An Action that pipes functions into a QEMU emulation workspace might be easier to manage long term. |
I don't think that assumption holds up, especially since the vast majority of the bit manipulation instructions can be cleanly modeled using existing pcode ops, and the ones that cannot seem to be limited in use-cases. Perhaps they would be part of optimised implementations of some cryptographic operations (as hinted at by the reference I linked before), but I think the vast majority of code doesn't need the
I'm not quite sure what you mean by this. This sounds like it will require quite a lot of effort to put this system into place, much more than replacing the custom pcode ops in this PR that can be easily modeled using existing pcode. |
|
Good questions! I ran a quick-and-dirty survey using one of the DPDK apps. One of the default DPDK RISC-V build configurations is with This compiler generated about 960 This same binary had about 23000 You may get a different result with different binaries, especially with the current RISC-V cores. In the longer term, I suspect we need to solve the vector bit manipulation challenge first then treat scalar bit manipulation within that framework. Edit: corrected GCC compiler version from 16 to 15.2 |
|
Yeah, the decompiler is currently not well-equipped to handle vectorised instructions. I agree that those instructions are impossible to model nicely because of this. However, addressing this will take quite a bit of time, and getting the resulting PR merged will probably take even longer. Given how much work that will be, it's probably relatively simple to then fit previously made definitions in that framework (if they need adjusting in the first place). In the mean time, I think it's good to provide definitions for bit manipulation instructions. Especially for (scalar) As such, I think it makes sense for this PR to include definitions for the scalar bit operations, using as few custom pcode ops as possible. And then a different PR should more address the more general issue of the decompiler not handling vectorised instructions very well. Regarding your tests, did you count how many |
|
I like where you are going with these ideas, and will run the survey you requested. Adding support for
|
|
The bit manipulation instruction survey @LukeSerne requested can be found here in an initial form. |
|
Ah, you posted an update while I was typing my answer 😆
Hmm, not sure how that happened... To be clear, I'm not affiliated with the Ghidra team and (unfortunately) don't have any influence on getting this merged. I'll see if I can convince Github that I'm not a reviewer for this PR.
Not sure, I guess you could do what you did for the other tests you did, and just post the instruction counts in a comment on this PR. Not sure if much more data is needed. My questions were meant to get a rough understanding, not necessarily a scientifically rigorous study. EDIT: An issue on your repo seems fine as well 👍
That sounds good. By keeping that out of this PR, hopefully the Ghidra team will consider it in a completed state, and decide to merge it sooner.
I don't have a test bench of programs that I test with. Feel free to choose a location that you find useful. Perhaps a separate github repo could be used.
Interesting results! It seems like the instructions I expected to not be used much indeed did not appear much. The only instructions that do not easily map to existing pcode operations that appeared relatively often are the bit rotation instructions, |
Roughly two months have passed without any developer feedback 😞. @thixotropist, would you be open to replacing some of the custom pcode ops in this PR with existing pcode ops? |
|
@LukeSerne - I'd be happy to make those replacements. How would you go about testing the replacement semantics? Would the 32 bit and 64 bit semantics likely differ? Do any of those semantics depend on any Control and Status Register variables? @GhidorahRex mentioned in Issue #9198 that he wants to spend some time soon reviewing this PR, so any updates from us should make the PR easier to review. |
I'd just base the implementation on the manual (for example https://docs.riscv.org/reference/isa/unpriv/b-st-ext.html). If the manual turns out not to be accurate, that can always be fixed later. Perhaps by comparing the output of the pcode emulator and qemu or some real hardware that implements these extensions.
I think replacing some of the custom pcode ops with existing pcode ops shouldn't increase the complexity of this PR by too much. I think it will even improve this PR because the decompiler output will hopefully be better with most of the custom pcode ops removed. |
Vector ternary instructions need to mark destination registers as input registers too.
Add several RISCV Instruction Set extensions to Ghidra, following discussion #5744. This pull request tracks the tip of the binutils testsuite for vector, bitmap, and crypto instructions. You can verify the content by importing sample binaries from https://github.com/thixotropist/ghidra_import_tests. Import the RISCV-64 gas test suite, assemble to binary, then iterate on the Ghidra sleigh files until Ghidra and objdump give essentially the same disassembled output.
The sleigh files do not yet include pcode semantics. Recent updates to GCC-14 and libssl using RISCV vector and crypto extensions may give us sample binaries to work with, to see what pcode semantics actually add value with complex instructions like these.