Doom-Utils / deutex

WAD composer for Doom, Heretic, Hexen, and Strife
Other
61 stars 17 forks source link

--help causes DeuTex to segfault on x86 #57

Closed chungy closed 6 years ago

chungy commented 6 years ago

By bisecting, I've found that commit 85d821dd3c145be1a998ca2a704930caaad73030 introduced a segfault on x86 (possibly others) while attempting to fix a crash on sparc64.

MartinHowe426 commented 6 years ago

This also happens on Ubuntu 17.10.1 x64. Using gdb gives the point of failure as:

Program received signal SIGSEGV, Segmentation fault.
opt_widths () at deutex.c:1244
1244                    memcpy(current_section->com, &tmp, sizeof(tmp));
landfillbaby commented 6 years ago

also on aarch64. same point of failure. probably architecture independent

andwj commented 6 years ago

The following patch fixes the silly behavior of storing width values into char pointers, and fixes the segfault here.

diff --git a/src/deutex.c b/src/deutex.c
index c6d85a0f437b..5a356507213b 100644
--- a/src/deutex.c
+++ b/src/deutex.c
@@ -710,6 +710,8 @@ typedef struct {
     comfun_t exec;
     char *use;
     char *help;
+    uint16_t width1;
+    uint16_t width2;
 } comdef_t;

 /* FIXME should be at the top of the file but we need comdef_t */
@@ -1124,14 +1126,11 @@ void COMhelp(int argc, const char *argv[])
         /* Do a first pass on all the options for this section. Find out how
            wide the left and right columns need to be. */
         if (d->type == SEC) {
-            uint16_t tmp;
             if (section++)
                 putchar('\n');
             printf("%s:\n", d->help);
-            memcpy(&tmp, d->exec, sizeof(tmp));
-            width1 = tmp + OPTINDENT;
-            memcpy(&tmp, d->use, sizeof(tmp));
-            width2 = tmp;
+            width1 = d->width1 + OPTINDENT;
+            width2 = d->width2;
             if (width1 + 1 + width2 > TTYCOL)
                 width1 = TTYCOL - width2 - COLSPACING;
         }
@@ -1241,17 +1240,17 @@ static void opt_widths(void)
                 if (tmp != width2r)
                     /* Can't happen */
                     tmp = SHRT_MAX;
-                memcpy(current_section->com, &tmp, sizeof(tmp));
+                /* current_section->width2r = tmp; */

                 tmp = width1t;
                 if (tmp != width1t)
                     tmp = SHRT_MAX;
-                memcpy(current_section->exec, &tmp, sizeof(tmp));
+                current_section->width1 = tmp;

                 tmp = width2t;
                 if (tmp != width2t)
                     tmp = SHRT_MAX;
-                memcpy(current_section->use, &tmp, sizeof(tmp));
+                current_section->width2 = tmp;
             }
         }
chungy commented 6 years ago

Thanks. Always wanted to investigate but always put it off :)