diff mbox series

[v2,16/20] disas/nanomips: Replace exception handling

Message ID 20220905095522.66941-17-milica.lazarevic@syrmia.com (mailing list archive)
State New, archived
Headers show
Series Convert nanoMIPS disassembler from C++ to C | expand

Commit Message

Milica Lazarevic Sept. 5, 2022, 9:55 a.m. UTC 
Since there's no support for exception handling in C, the try-catch
blocks have been deleted, and throw clauses are replaced. When a runtime
error happens, we're printing out the error message. Disassembling of
the current instruction interrupts. This behavior is achieved by adding
sigsetjmp() to discard further disassembling after the error message
prints and by adding the siglongjmp() function to imitate throwing an
error. The goal was to maintain the same output as it was.

Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
---
 disas/nanomips.cpp | 126 ++++++++++++++++++++++-----------------------
 1 file changed, 61 insertions(+), 65 deletions(-)

Comments

Richard Henderson Sept. 5, 2022, 11:50 a.m. UTC | #1 
On 9/5/22 10:55, Milica Lazarevic wrote:
> Since there's no support for exception handling in C, the try-catch
> blocks have been deleted, and throw clauses are replaced. When a runtime
> error happens, we're printing out the error message. Disassembling of
> the current instruction interrupts. This behavior is achieved by adding
> sigsetjmp() to discard further disassembling after the error message
> prints and by adding the siglongjmp() function to imitate throwing an
> error. The goal was to maintain the same output as it was.
> 
> Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
> ---
>   disas/nanomips.cpp | 126 ++++++++++++++++++++++-----------------------
>   1 file changed, 61 insertions(+), 65 deletions(-)
> 
> diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
> index 5142f307fc..a8295ebfa8 100644
> --- a/disas/nanomips.cpp
> +++ b/disas/nanomips.cpp
> @@ -31,7 +31,6 @@
>   #include "disas/dis-asm.h"
>   
>   #include <string.h>
> -#include <stdexcept>
>   #include <stdio.h>
>   #include <stdarg.h>
>   
> @@ -134,10 +133,12 @@ static uint64 renumber_registers(uint64 index, uint64 *register_list,
>           return register_list[index];
>       }
>   
> -    throw std::runtime_error(img_format(
> -                   "Invalid register mapping index %" PRIu64
> -                   ", size of list = %zu",
> -                   index, register_list_size));
> +    g_autofree char *err = img_format(
> +                      "Invalid register mapping index %" PRIu64
> +                      ", size of list = %zu",
> +                      index, register_list_size);
> +    info->fprintf_func(info->stream, "%s", err);

There's no point passing the output of sprintf into fprintf like this.
Just fprintf in the first place.


> +                        disassembly_function dis_fn = table[i].disassembly;
> +                        if (dis_fn == 0) {
> +                            strcpy(dis,
> +                            "disassembler failure - bad table entry");
> +                            return -6;
> +                        }
> +                        type = table[i].type;
> +                        g_autofree char *dis_str = dis_fn(op_code, info);
> +                        strcpy(dis, dis_str);
> +                        return table[i].instructions_size;
> +                    } else {
> +                        strcpy(dis, "reserved instruction");
> +                        return -2;

Ought these errors use the same error path?


r~
Milica Lazarevic Sept. 7, 2022, 5:09 p.m. UTC | #2 
On 9/5/22 10:55, Milica Lazarevic wrote:
> Since there's no support for exception handling in C, the try-catch
> blocks have been deleted, and throw clauses are replaced. When a runtime
> error happens, we're printing out the error message. Disassembling of
> the current instruction interrupts. This behavior is achieved by adding
> sigsetjmp() to discard further disassembling after the error message
> prints and by adding the siglongjmp() function to imitate throwing an
> error. The goal was to maintain the same output as it was.
>
> Signed-off-by: Milica Lazarevic <milica.lazarevic@syrmia.com>
> ---
>   disas/nanomips.cpp | 126 ++++++++++++++++++++++-----------------------
>   1 file changed, 61 insertions(+), 65 deletions(-)
>
> diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
> index 5142f307fc..a8295ebfa8 100644
> --- a/disas/nanomips.cpp
> +++ b/disas/nanomips.cpp
> @@ -31,7 +31,6 @@

> +                        disassembly_function dis_fn = table[i].disassembly;
> +                        if (dis_fn == 0) {
> +                            strcpy(dis,
> +                            "disassembler failure - bad table entry");
> +                            return -6;
> +                        }
> +                        type = table[i].type;
> +                        g_autofree char *dis_str = dis_fn(op_code, info);
> +                        strcpy(dis, dis_str);
> +                        return table[i].instructions_size;
> +                    } else {
> +                        strcpy(dis, "reserved instruction");
> +                        return -2;

Ought these errors use the same error path?


r~

I'm not sure if I understood the question correctly. The difference between these errors and the errors covered by the patch is that, in the case of the latter, the disassembling of one part of the instruction has already happened. For example, if we have an irregular GPR register index, the output of the assembler will look something like this:
...
0x80200622:  86ad 9018      Invalid GPR register index 33
...
On the other hand, if we have some error like "disassembler failure - bad table entry" - that string would be the only output regarding the instruction we're trying to disassemble. The disassembling process, in this case, hasn't begun, so there's no need for an interruption like in the previous example.

But yes, in both cases after printing the error message, the disassembling of the particular instruction is not continuing.
diff mbox series

Patch

diff --git a/disas/nanomips.cpp b/disas/nanomips.cpp
index 5142f307fc..a8295ebfa8 100644
--- a/disas/nanomips.cpp
+++ b/disas/nanomips.cpp
@@ -31,7 +31,6 @@ 
 #include "disas/dis-asm.h"
 
 #include <string.h>
-#include <stdexcept>
 #include <stdio.h>
 #include <stdarg.h>
 
@@ -134,10 +133,12 @@  static uint64 renumber_registers(uint64 index, uint64 *register_list,
         return register_list[index];
     }
 
-    throw std::runtime_error(img_format(
-                   "Invalid register mapping index %" PRIu64
-                   ", size of list = %zu",
-                   index, register_list_size));
+    g_autofree char *err = img_format(
+                      "Invalid register mapping index %" PRIu64
+                      ", size of list = %zu",
+                      index, register_list_size);
+    info->fprintf_func(info->stream, "%s", err);
+    siglongjmp(info->buf, 1);
 }
 
 
@@ -514,8 +515,10 @@  static const char *GPR(uint64 reg, struct Dis_info *info)
         return gpr_reg[reg];
     }
 
-    throw std::runtime_error(img_format("Invalid GPR register index %" PRIu64,
-                                         reg));
+    g_autofree char *err = img_format("Invalid GPR register index %" PRIu64,
+                                 reg);
+    info->fprintf_func(info->stream, "%s", err);
+    siglongjmp(info->buf, 1);
 }
 
 
@@ -553,8 +556,10 @@  static const char *FPR(uint64 reg, struct Dis_info *info)
         return fpr_reg[reg];
     }
 
-    throw std::runtime_error(img_format("Invalid FPR register index %" PRIu64,
-                                         reg));
+    g_autofree const char *err = img_format(
+        "Invalid FPR register index %" PRIu64, reg);
+    info->fprintf_func(info->stream, "%s", err);
+    siglongjmp(info->buf, 1);
 }
 
 
@@ -568,8 +573,10 @@  static const char *AC(uint64 reg, struct Dis_info *info)
         return ac_reg[reg];
     }
 
-    throw std::runtime_error(img_format("Invalid AC register index %" PRIu64,
-                                         reg));
+    const char *err = img_format("Invalid AC register index %" PRIu64,
+                                 reg);
+    info->fprintf_func(info->stream, "%s", err);
+    siglongjmp(info->buf, 1);
 }
 
 
@@ -631,65 +638,48 @@  static int Disassemble(const uint16 *data, char *dis,
                        TABLE_ENTRY_TYPE & type, const Pool *table,
                        int table_size, struct Dis_info *info)
 {
-    try
-    {
-        for (int i = 0; i < table_size; i++) {
-            uint64 op_code = extract_op_code_value(data,
-                                 table[i].instructions_size);
-            if ((op_code & table[i].mask) == table[i].value) {
-                /* possible match */
-                conditional_function cond = table[i].condition;
-                if ((cond == NULL) || cond(op_code)) {
-                    try
-                    {
-                        if (table[i].type == pool) {
-                            return Disassemble(data, dis, type,
-                                               table[i].next_table,
-                                               table[i].next_table_size, info);
-                        } else if ((table[i].type == instruction) ||
-                                   (table[i].type == call_instruction) ||
-                                   (table[i].type == branch_instruction) ||
-                                   (table[i].type == return_instruction)) {
-                            if ((table[i].attributes != 0) &&
-                                (ALL_ATTRIBUTES & table[i].attributes) == 0) {
-                                /*
-                                 * failed due to instruction having
-                                 * an ASE attribute and the requested version
-                                 * not having that attribute
-                                 */
-                                strcpy(dis, "ASE attribute mismatch");
-                                return -5;
-                            }
-                            disassembly_function dis_fn = table[i].disassembly;
-                            if (dis_fn == 0) {
-                                strcpy(dis,
-                                "disassembler failure - bad table entry");
-                                return -6;
-                            }
-                            type = table[i].type;
-                            g_autofree char *dis_str = dis_fn(op_code, info);
-                            strcpy(dis, dis_str);
-                            return table[i].instructions_size;
-                        } else {
-                            strcpy(dis, "reserved instruction");
-                            return -2;
+    for (int i = 0; i < table_size; i++) {
+        uint64 op_code = extract_op_code_value(data,
+                             table[i].instructions_size);
+        if ((op_code & table[i].mask) == table[i].value) {
+            /* possible match */
+            conditional_function cond = table[i].condition;
+            if ((cond == NULL) || cond(op_code)) {
+                if (table[i].type == pool) {
+                    return Disassemble(data, dis, type,
+                                       table[i].next_table,
+                                       table[i].next_table_size, info);
+                } else if ((table[i].type == instruction) ||
+                           (table[i].type == call_instruction) ||
+                           (table[i].type == branch_instruction) ||
+                           (table[i].type == return_instruction)) {
+                        if ((table[i].attributes != 0) &&
+                            (ALL_ATTRIBUTES & table[i].attributes) == 0) {
+                            /*
+                             * failed due to instruction having
+                             * an ASE attribute and the requested version
+                             * not having that attribute
+                             */
+                            strcpy(dis, "ASE attribute mismatch");
+                            return -5;
                         }
-                    }
-                    catch (std::runtime_error & e)
-                    {
-                        strcpy(dis, e.what());
-                        return -3;          /* runtime error */
+                        disassembly_function dis_fn = table[i].disassembly;
+                        if (dis_fn == 0) {
+                            strcpy(dis,
+                            "disassembler failure - bad table entry");
+                            return -6;
+                        }
+                        type = table[i].type;
+                        g_autofree char *dis_str = dis_fn(op_code, info);
+                        strcpy(dis, dis_str);
+                        return table[i].instructions_size;
+                    } else {
+                        strcpy(dis, "reserved instruction");
+                        return -2;
                     }
                 }
             }
         }
-    }
-    catch (std::exception & e)
-    {
-        strcpy(dis, e.what());
-        return -4;          /* runtime error */
-    }
-
     strcpy(dis, "failed to disassemble");
     return -1;      /* failed to disassemble        */
 }
@@ -22333,6 +22323,12 @@  int print_insn_nanomips(bfd_vma memaddr, struct disassemble_info *info)
         (*info->fprintf_func)(info->stream, "     ");
     }
 
+    /* Handle runtime errors. */
+    if (sigsetjmp(disassm_info.buf, 0) != 0) {
+        info->insn_type = dis_noninsn;
+        return insn3 ? 6 : insn2 ? 4 : 2;
+    }
+
     int length = nanomips_dis(buf, &disassm_info, insn1, insn2, insn3);
 
     /* FIXME: Should probably use a hash table on the major opcode here.  */