From 8d459b218d966bbf903e7f7e19a4e8e1b0174a7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=D9=85=D8=B5=D8=B7=D9=81=D9=8A=20=D9=85=D8=AD=D9=85=D9=88?= =?UTF-8?q?=D8=AF=20=D9=83=D9=85=D8=A7=D9=84=20=D8=A7=D9=84=D8=AF=D9=8A?= =?UTF-8?q?=D9=86?= <48567303+moste00@users.noreply.github.com> Date: Thu, 26 Feb 2026 06:17:03 +0200 Subject: [PATCH] librz/bin: fix patching relocations for RISC-V ELFs * fix unused return value in patch_relocs_riscv and unreachable code in extension parser --- librz/arch/p/arch_riscv_extension_parser.inc | 38 ++++--- librz/bin/format/elf/elf_relocs_patching.c | 105 ++++++++++++------- sys/riscv_extension_parser_generator.py | 45 +++++--- 3 files changed, 124 insertions(+), 64 deletions(-) diff --git a/librz/arch/p/arch_riscv_extension_parser.inc b/librz/arch/p/arch_riscv_extension_parser.inc index 867d408bef..16bb799e82 100644 --- a/librz/arch/p/arch_riscv_extension_parser.inc +++ b/librz/arch/p/arch_riscv_extension_parser.inc @@ -9,8 +9,8 @@ * RISC-V Extension Parser Using a Trie trick * Generated from arch/RISCV/RISCVGenSubtargetInfo.inc in Capstone sources * - * This function parses a single RISC-V extension name and ORs the corresponding - * feature flags with an accumulator 'mode'. + * This function parses a single RISC-V extension name and returns the corresponding + * feature flag. * * Trie Diagram: * ROOT @@ -556,13 +556,27 @@ typedef enum { } \ } while (0) +/* Macro: Conditionally invoke a macro if the current character is not on a given blacklist*/ +#define DO_IF_NOT_ANY_OF(blacklist, thing) \ + do { \ + bool __will_do__ = true; \ + char *__curr__ = blacklist; \ + while (*__curr__ != '\0') { \ + if (*p == *__curr__++) { \ + __will_do__ = false; \ + } \ + } \ + if (__will_do__) { \ + thing; \ + } \ + } while (0) + /** * Main parse routine: Try to consume a RISC-V extension name from the architecture string at a given position. * * @param arch_str: The full architecture string * @param idx: Pointer to current index (will be updated to show consumed length) - * @param mode: Pointer to the feature accumulator (will be ORed with the feature flag if extension found) - * @return: Parse result indicating why parsing stopped and whether a match was found + * @param mode: Pointer to the feature result * @return: Parse result indicating why parsing stopped and whether a match was found * * Stopping conditions: * - '\0' (end of string) @@ -1031,7 +1045,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { } } else if (*p == 'h') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZfh); + DO_IF_NOT_ANY_OF("m", STOP_WITH_MATCH(RISCV_FeatureStdExtZfh)); /* State: 'zfh' expecting [m] */ if (*p == 'm') { @@ -1087,7 +1101,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { /* State: 'zhin' expecting [x] */ if (*p == 'x') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZhinx); + DO_IF_NOT_ANY_OF("m", STOP_WITH_MATCH(RISCV_FeatureStdExtZhinx)); /* State: 'zhinx' expecting [m] */ if (*p == 'm') { @@ -1473,12 +1487,12 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { } } else if (*p == 'k') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZk); + DO_IF_NOT_ANY_OF("nrst", STOP_WITH_MATCH(RISCV_FeatureStdExtZk)); /* State: 'zk' expecting [nrst] */ if (*p == 'n') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZkn); + DO_IF_NOT_ANY_OF("deh", STOP_WITH_MATCH(RISCV_FeatureStdExtZkn)); /* State: 'zkn' expecting [deh] */ if (*p == 'd') { @@ -1499,7 +1513,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { STOP_WITH_MATCH(RISCV_FeatureStdExtZkr); } else if (*p == 's') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZks); + DO_IF_NOT_ANY_OF("eh", STOP_WITH_MATCH(RISCV_FeatureStdExtZks)); /* State: 'zks' expecting [eh] */ if (*p == 'e') { @@ -1686,7 +1700,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { } } else if (*p == 'h') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZvfh); + DO_IF_NOT_ANY_OF("m", STOP_WITH_MATCH(RISCV_FeatureStdExtZvfh)); /* State: 'zvfh' expecting [m] */ if (*p == 'm') { @@ -1725,7 +1739,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { STOP_WITH_MATCH(RISCV_FeatureStdExtZvkg); } else if (*p == 'n') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZvkn); + DO_IF_NOT_ANY_OF("cegh", STOP_WITH_MATCH(RISCV_FeatureStdExtZvkn)); /* State: 'zvkn' expecting [cegh] */ if (*p == 'c') { @@ -1763,7 +1777,7 @@ try_consume_riscv_ext_from(const char *ext_name, size_t *idx, ut64 *mode) { } } else if (*p == 's') { p++; - STOP_WITH_MATCH(RISCV_FeatureStdExtZvks); + DO_IF_NOT_ANY_OF("cegh", STOP_WITH_MATCH(RISCV_FeatureStdExtZvks)); /* State: 'zvks' expecting [cegh] */ if (*p == 'c') { diff --git a/librz/bin/format/elf/elf_relocs_patching.c b/librz/bin/format/elf/elf_relocs_patching.c index d2d2dea088..8a29cd26e5 100644 --- a/librz/bin/format/elf/elf_relocs_patching.c +++ b/librz/bin/format/elf/elf_relocs_patching.c @@ -7,6 +7,7 @@ #include "elf/glibc_elf.h" #include "rz_types.h" #include "rz_types_base.h" +#include "rz_util/rz_assert.h" #include "rz_util/rz_buf.h" #include "rz_util/rz_log.h" @@ -2342,25 +2343,31 @@ static void patch_reloc_riscv(RZ_INOUT RzBuffer *buf_patched, const ut64 patch_a case R_RISCV_NONE: return; - case R_RISCV_32: + case R_RISCV_32: { val = S + A; - rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + bool success = rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + rz_return_if_fail(success); break; - - case R_RISCV_64: + } + case R_RISCV_64: { val = S + A; - rz_buf_write_ble64_at(buf_patched, patch_addr, val, big_endian); + bool success = rz_buf_write_ble64_at(buf_patched, patch_addr, val, big_endian); + rz_return_if_fail(success); break; - + } case R_RISCV_RELATIVE: val = A + B; switch (bits) { - case 32: - rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + case 32: { + bool success = rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + rz_return_if_fail(success); break; - case 64: - rz_buf_write_ble64_at(buf_patched, patch_addr, val, big_endian); + } + case 64: { + bool success = rz_buf_write_ble64_at(buf_patched, patch_addr, val, big_endian); + rz_return_if_fail(success); break; + } default: RZ_LOG_WARN("Unsupported number of bits for R_RISCV_RELATIVE: %d, only 32 bits and 64 bits are supported", bits); return; @@ -2412,22 +2419,28 @@ static void patch_reloc_riscv(RZ_INOUT RzBuffer *buf_patched, const ut64 patch_a case R_RISCV_JUMP_SLOT: switch (bits) { - case 32: - rz_buf_write_ble32_at(buf_patched, patch_addr, S, big_endian); + case 32: { + bool success = rz_buf_write_ble32_at(buf_patched, patch_addr, S, big_endian); + rz_return_if_fail(success); break; - case 64: - rz_buf_write_ble64_at(buf_patched, patch_addr, S, big_endian); + } + case 64: { + bool success = rz_buf_write_ble64_at(buf_patched, patch_addr, S, big_endian); + rz_return_if_fail(success); break; + } default: RZ_LOG_WARN("Unsupported number of bits for R_RISCV_JUMP_SLOT: %d, only 32 bits and 64 bits are supported", bits); break; } break; - case R_RISCV_32_PCREL: + case R_RISCV_32_PCREL: { val = S + A - P; - rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + bool success = rz_buf_write_ble32_at(buf_patched, patch_addr, val, big_endian); + rz_return_if_fail(success); break; + } case R_RISCV_GOT_HI20: { val = fs->G + fs->GOT + A - P; @@ -2506,91 +2519,111 @@ static void patch_reloc_riscv(RZ_INOUT RzBuffer *buf_patched, const ut64 patch_a rz_buf_read_ble8_at(buf_patched, patch_addr, &old_val, big_endian); ut64 result = ((ut64)old_val) + S + A; unsigned long long addr = patch_addr; - rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)result, big_endian); + bool success = rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_ADD16: { ut16 old_val = 0; - rz_buf_read_ble16_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble16_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) + S + A; - rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)result, big_endian); + success = rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_ADD32: { ut32 old_val = 0; - rz_buf_read_ble32_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble32_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) + S + A; - rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)result, big_endian); + success = rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_ADD64: { ut64 old_val = 0; - rz_buf_read_ble64_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble64_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = old_val + S + A; - rz_buf_write_ble64_at(buf_patched, patch_addr, result, big_endian); + success = rz_buf_write_ble64_at(buf_patched, patch_addr, result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SUB8: { ut8 old_val = 0; - rz_buf_read_ble8_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble8_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) - S - A; unsigned long long addr = patch_addr; - rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)result, big_endian); + success = rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SUB16: { ut16 old_val = 0; - rz_buf_read_ble16_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble16_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) - S - A; - rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)result, big_endian); + success = rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SUB32: { ut32 old_val = 0; - rz_buf_read_ble32_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble32_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) - S - A; - rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)result, big_endian); + success = rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SUB64: { ut64 old_val = 0; - rz_buf_read_ble64_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble64_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); ut64 result = ((ut64)old_val) - S - A; - rz_buf_write_ble64_at(buf_patched, patch_addr, result, big_endian); + success = rz_buf_write_ble64_at(buf_patched, patch_addr, result, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SET8: { val = S + A; unsigned long long addr = patch_addr; - rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)val, big_endian); + bool success = rz_buf_write_ble8_offset(buf_patched, &addr, (ut8)val, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SET16: { val = S + A; - rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)val, big_endian); + bool success = rz_buf_write_ble16_at(buf_patched, patch_addr, (ut16)val, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SET32: { val = S + A; - rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)val, big_endian); + bool success = rz_buf_write_ble32_at(buf_patched, patch_addr, (ut32)val, big_endian); + rz_return_if_fail(success); break; } case R_RISCV_SET6: case R_RISCV_SUB6: { ut8 old_val = 0; - rz_buf_read_ble8_at(buf_patched, patch_addr, &old_val, big_endian); + bool success = rz_buf_read_ble8_at(buf_patched, patch_addr, &old_val, big_endian); + rz_return_if_fail(success); val = S + A; ut8 result = (rel_type == R_RISCV_SET6) ? val : ((old_val & 0x3F) - val); - rz_buf_write_ble8_at(buf_patched, patch_addr, (old_val & 0xC0) | (result & 0x3F), big_endian); + success = rz_buf_write_ble8_at(buf_patched, patch_addr, (old_val & 0xC0) | (result & 0x3F), big_endian); + rz_return_if_fail(success); break; } diff --git a/sys/riscv_extension_parser_generator.py b/sys/riscv_extension_parser_generator.py index 97154b5464..3bcada92c4 100644 --- a/sys/riscv_extension_parser_generator.py +++ b/sys/riscv_extension_parser_generator.py @@ -5,12 +5,11 @@ """ RISC-V Extension Trie-Based Parser Generator -Given a list of RISCV_FeatureStdExt* enums, generates C code that: -1. Parses extension names using a trie data structure -2. ORs the corresponding enum value with a running accumulator +Given a list of RISCV_FeatureStdExt* enums (by passing arch/RISCV/RISCVGenSubtargetInfo.inc), it generates C code that: +1. Parses extension names using a trie data structure (to efficiently exploit common prefixes) +2. Increments the current index given to the parsing routine and returns an enum representing the extension parsed 3. Includes an ASCII art tree visualization of the trie -4. Annotates branches with parse state -5. Uses macros to reduce boilerplate +4. Annotates parsing control flow branches with parse state """ import sys @@ -130,15 +129,17 @@ def generate_c_code(trie: Trie, extensions: List[Tuple[str, str]]) -> str: return lines # Non-leaf node - check if this is also a valid end point - if node.enum_name: - lines.append(f"{indent}STOP_WITH_MATCH({node.enum_name});") - lines.append("") - - # Process children - add state comment only before branching sorted_children = sorted(node.children.items()) expected_chars = [c for c, _ in sorted_children] expected_str = "".join(expected_chars) + if node.enum_name: + lines.append( + f'{indent}DO_IF_NOT_ANY_OF("{expected_str}", STOP_WITH_MATCH({node.enum_name}));' + ) + lines.append("") + + # Process children - add state comment only before branching lines.append(f"{indent}/* State: '{path}' expecting [{expected_str}] */") for i, (char, child) in enumerate(sorted_children): @@ -184,8 +185,8 @@ def generate_c_code(trie: Trie, extensions: List[Tuple[str, str]]) -> str: " * Generated from arch/RISCV/RISCVGenSubtargetInfo.inc in Capstone sources \n" ) full_code += " * \n" - full_code += " * This function parses a single RISC-V extension name and ORs the corresponding\n" - full_code += " * feature flags with an accumulator 'mode'.\n" + full_code += " * This function parses a single RISC-V extension name and returns the corresponding\n" + full_code += " * feature flag.\n" full_code += " * \n" full_code += " * Trie Diagram:\n" full_code += tree_comment + "\n" @@ -271,6 +272,21 @@ def generate_c_code(trie: Trie, extensions: List[Tuple[str, str]]) -> str: full_code += " } \\\n" full_code += " } while (0)\n\n" + full_code += "/* Macro: Conditionally invoke a macro if the current character is not on a given blacklist*/\n" + full_code += "#define DO_IF_NOT_ANY_OF(blacklist, thing) \\\n" + full_code += " do { \\\n" + full_code += " bool __will_do__ = true; \\\n" + full_code += " char *__curr__ = blacklist; \\\n" + full_code += " while (*__curr__ != '\\0') { \\\n" + full_code += " if (*p == *__curr__++) { \\\n" + full_code += " __will_do__ = false; \\\n" + full_code += " } \\\n" + full_code += " } \\\n" + full_code += " if (__will_do__) { \\\n" + full_code += " thing; \\\n" + full_code += " } \\\n" + full_code += " } while (0)\n\n" + full_code += "/**\n" full_code += ( " * Main parse routine: Try to consume a RISC-V extension name " @@ -282,10 +298,7 @@ def generate_c_code(trie: Trie, extensions: List[Tuple[str, str]]) -> str: " * @param idx: Pointer to current index " "(will be updated to show consumed length)\n" ) - full_code += ( - " * @param mode: Pointer to the feature accumulator " - "(will be ORed with the feature flag if extension found)\n" - ) + full_code += " * @param mode: Pointer to the feature result" full_code += " * @return: Parse result indicating why parsing stopped and whether a match was found\n" full_code += " * \n" full_code += " * Stopping conditions:\n"