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

flag_register_to_human is not implemented on RISC-V architecture #1076

Merged
merged 4 commits into from
Apr 8, 2024

Conversation

yuk1i
Copy link
Contributor

@yuk1i yuk1i commented Mar 15, 2024

flag_register_to_human is not implemented in all architectures, such as RISCV.

This issue will crash context command.

This issue is confirmed in gef dfd3868.

Note: The backend is an openocd instance and VisionFive2 board over Jlink debugger.
However, I am convinced this issue is unrelated to the specific backend configuration.

Log

Copyright (C) 2023 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word".
GEF for linux ready, type `gef' to start, `gef config' to configure
88 commands loaded and 5 functions added for GDB 13.2 in 0.00ms using Python engine 3.11
The target architecture is set to "riscv:rv64".
shutdown () at os/sbi.c:38
warning: Source file is more recent than executable.
38              while(1) asm volatile("nop" ::: "memory");
[ Legend: Modified register | Code | Heap | Stack | String ]
────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── registers ────
$zero: 0x0000000000000000 $ra  : 0x0000000080202678 $sp  : 0x000000008025bf10 $gp  : 0x00000000f76fadd0 $tp  : 0x0000000000000001 $t0  : 0x00000000f7fec8e0 $t1  : 0x000000000000002d $t2  : 0x0a00000000000000 $fp  : 0x000000008025bf20 
$s1  : 0x0000000080249000 $a0  : 0x0000000000000000 $a1  : 0x0000000000000000 $a2  : 0x0000000000000000 $a3  : 0x000000008025be42 $a4  : 0x0000000000000001 $a5  : 0x00000000802055f1 $a6  : 0x0000000000000001 $a7  : 0x0000000000000001 
$s2  : 0x00000000807f5000 $s3  : 0x0000000080249000 $s4  : 0x0000000080660000 $s5  : 0x0000000000001000 $s6  : 0x000000008024c000 $s7  : 0x0000000000001000 $s8  : 0x000000008024b216 $s9  : 0x00000000802490e5 $s10 : 0xffffffff7fdb8000 
$s11 : 0x00000000802050e0 $t3  : 0x0000000000000052 $t4  : 0x0000000000000000 $t5  : 0x0000000000000000 $t6  : 0x00000000f76fabf0 
[!] Command 'context' failed to execute properly, reason: 
Breakpoint 1 at 0x80200000: file os/entry.S, line 4.
(qemu) gef➤  gef config gef.debug True
(qemu) gef➤  context
[ Legend: Modified register | Code | Heap | Stack | String ]
───────────────────────────────────────────────────────────────────────────────────── registers ────
$zero: 0x0000000000000000 $ra  : 0x0000000080202678 $sp  : 0x000000008025bf10 
$gp  : 0x00000000f76fadd0 $tp  : 0x0000000000000001 $t0  : 0x00000000f7fec8e0 
$t1  : 0x000000000000002d $t2  : 0x0a00000000000000 $fp  : 0x000000008025bf20 
$s1  : 0x0000000080249000 $a0  : 0x0000000000000000 $a1  : 0x0000000000000000 
$a2  : 0x0000000000000000 $a3  : 0x000000008025be42 $a4  : 0x0000000000000001 
$a5  : 0x00000000802055f1 $a6  : 0x0000000000000001 $a7  : 0x0000000000000001 
$s2  : 0x00000000807f5000 $s3  : 0x0000000080249000 $s4  : 0x0000000080660000 
$s5  : 0x0000000000001000 $s6  : 0x000000008024c000 $s7  : 0x0000000000001000 
$s8  : 0x000000008024b216 $s9  : 0x00000000802490e5 $s10 : 0xffffffff7fdb8000 
$s11 : 0x00000000802050e0 $t3  : 0x0000000000000052 $t4  : 0x0000000000000000 
$t5  : 0x0000000000000000 $t6  : 0x00000000f76fabf0 

─────────────────────────────── Exception raised ───────────────────────────────
NotImplementedError: 
───────────────────────────── Detailed stacktrace ──────────────────────────────
↳ File "/data/os-riscv/gef/gef.py", line 2319, in flag_register_to_human()
    →     raise NotImplementedError
↳ File "/data/os-riscv/gef/gef.py", line 7589, in context_regs()
    →     gef_print(f"Flags: {gef.arch.flag_register_to_human()}")
↳ File "/data/os-riscv/gef/gef.py", line 7484, in do_invoke()
    →     display_pane_function()
↳ File "/data/os-riscv/gef/gef.py", line 368, in wrapper()
    →     return f(*args, **kwargs)
↳ File "/data/os-riscv/gef/gef.py", line 244, in wrapper()
    →     rv = f(*args, **kwargs)
↳ File "/data/os-riscv/gef/gef.py", line 4604, in invoke()
    →     bufferize(self.do_invoke)(argv)
─────────────────────────────────── Version ────────────────────────────────────
GEF: rev:dfd3868e353a00f4aa8215f2f00263ffb961d0db (Git - clean)
SHA256(/data/os-riscv/gef/gef.py): 1274ab88d5fa1850d5460a3963c67bfb4ef8376992d347ea73ba5d5be149ece2
GDB: 13.2
GDB-Python: 3.11
obsolete loaded_command_names
Loaded commands: $, aliases, aliases add, aliases ls, aliases rm, aslr, canary, checksec, context, dereference, edit-flags, elf-info, entry-break, format-string-helper, functions, gef-remote, got, heap, heap arenas, heap bins, heap bins fast, heap bins large, heap bins small, heap bins tcache, heap bins unsorted, heap chunk, heap chunks, heap set-arena, heap-analysis-helper, hexdump, hexdump byte, hexdump dword, hexdump qword, hexdump word, highlight, highlight add, highlight clear, highlight list, highlight remove, hijack-fd, memory, memory list, memory reset, memory unwatch, memory watch, name-break, nop, patch, patch byte, patch dword, patch qword, patch string, patch word, pattern, pattern create, pattern search, pcustom, pcustom edit, pcustom list, pcustom show, pie, pie attach, pie breakpoint, pie delete, pie info, pie remote, pie run, print-format, process-search, process-status, registers, reset-cache, scan, search-pattern, shellcode, shellcode get, shellcode search, skipi, stub, theme, trace-run, version, vmmap, xfiles, xinfo, xor-memory, xor-memory display, xor-memory patch
───────────────────────────── Last 10 GDB commands ─────────────────────────────
  248  i r sys
  249  q
  250  load
  251  c
  252  c
  253  q
  254  wq
  255  q
  256  gef config gef.debug True
  257  context
───────────────────────────── Runtime environment ──────────────────────────────
* GDB: 13.2
* Python: 3.11.8 - final
* OS: Linux - 6.7.7-amd64 (x86_64)
No LSB modules are available.
Distributor ID: Debian
Description:    Debian GNU/Linux trixie/sid
Release:        n/a
Codename:       trixie
────────────────────────────────────────────────────────────────────────────────

.gdbinit

set confirm off
source /data/os-riscv/gef/gef.py
set architecture riscv:rv64
file build/kernel
gef config context.show_registers_raw 1
#target remote 127.0.0.1:3333
gef-remote --qemu-user --qemu-binary build/kernel 127.0.0.1 3333
set riscv use-compressed-breakpoints yes
b *0x80200000

Checklist

  • My code follows the code style of this project.
  • My change includes a change to the documentation, if required.
  • If my change adds new code, adequate tests have been added.
  • I have read and agree to the CONTRIBUTING document.

Copy link

🤖 Coverage update for 5ac497d 🟢

Old New
Commit dfd3868 5ac497d
Score 71.5448% 71.5831% (0.0383)

@Grazfather
Copy link
Collaborator

Proper solution is to just implement that method for the arch. Would you be able to do that instead?

@hugsy
Copy link
Owner

hugsy commented Mar 16, 2024

Hi @yuk1i

flag_register_to_human is not implemented in all architectures, such as RISCV.

I've gone through the code of GEF, and RISC-V is the only architecture for which it's not implemented. All the others are:

  • arm / aarch64
  • x86 / x64
  • mips / mips64
  • powerpc / powerpc64
  • sparc / sparc64

As @Grazfather mentioned, your PR in this form is therefore unlikely to get merged. A more useful contribution would be to implement this function for RISC-V instead, since you seem to have easy access to a RISC-V environment to test it. This is pretty easy, and if so, feel free to ask for help on the Discord if needed.

If you don't wish to implement it yourself, then you can close this PR and open an issue instead (linking this PR).

Thanks

@hugsy hugsy changed the title NotImplementedError when calling gef.arch.flag_register_to_human() flag_register_to_human is not implemented on RISC-V architecture Mar 16, 2024
@yuk1i
Copy link
Contributor Author

yuk1i commented Mar 20, 2024

Hi @yuk1i

flag_register_to_human is not implemented in all architectures, such as RISCV.

I've gone through the code of GEF, and RISC-V is the only architecture for which it's not implemented. All the others are:

  • arm / aarch64
  • x86 / x64
  • mips / mips64
  • powerpc / powerpc64
  • sparc / sparc64

As @Grazfather mentioned, your PR in this form is therefore unlikely to get merged. A more useful contribution would be to implement this function for RISC-V instead, since you seem to have easy access to a RISC-V environment to test it. This is pretty easy, and if so, feel free to ask for help on the Discord if needed.

If you don't wish to implement it yourself, then you can close this PR and open an issue instead (linking this PR).

Thanks

If I remember correctly, RISC-V does not have the flags register like x86's EFLAGS and ARM's NZCV.

According to the existing code that uses this function's returned value in a print command, I think it's better to leave other archs' code as is, and simply short cut this function for risc-v.

@hugsy
Copy link
Owner

hugsy commented Mar 20, 2024

If I remember correctly, RISC-V does not have the flags register like x86's EFLAGS and ARM's NZCV.

Correct, we've already added a note in the code about that. This doesn't change the fact the function will be called regardless of the architecture.

So to re-iterate:

A more useful contribution would be to implement this function for RISC-V instead, since you seem to have easy access to a RISC-V environment to test it.

Here the contribution I mentioned would simply add a definition that function to the class RISCV to make it return an empty string. Since RISCV has no flag register, this is perfectly acceptable. On the other hand, your current code will prevent us from seeing any error happening in flag_register_to_human on any architecture (not just RISCV), which is not acceptable.

Copy link

🤖 Coverage update for adb8ac4 🟢

Old New
Commit dfd3868 adb8ac4
Score 71.5448% 71.5448% (0)

@yuk1i
Copy link
Contributor Author

yuk1i commented Mar 22, 2024

When I search the flag_register_to_human function in the code, I find a better way suppress the dummy "Flags: " string.

The "flags" command shows the flag register, and it checks arch's flag_register attribute, which is defined as None in RISCV.

So simply do the same thing in the "context" command.

        if not gef.arch.flag_register:
            warn(f"The architecture {gef.arch.arch}:{gef.arch.mode} doesn't have flag register.")
            return

Copy link

github-actions bot commented Apr 8, 2024

🤖 Coverage update for f7ff1a0 🟢

Old New
Commit dfd3868 f7ff1a0
Score 71.5448% 71.5448% (0)

Copy link

github-actions bot commented Apr 8, 2024

🤖 Coverage update for 06fdfe7 🟢

Old New
Commit dfd3868 06fdfe7
Score 71.5448% 71.5448% (0)

@hugsy hugsy merged commit 429a0e5 into hugsy:main Apr 8, 2024
6 checks passed
@hugsy hugsy added this to the 2024.05 milestone Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants