From 9d30a7cdaa4599377fd88968a390d9a448a3bfa8 Mon Sep 17 00:00:00 2001
From: Jonathan Wilkes <jon.w.wilkes@gmail.com>
Date: Wed, 23 Aug 2017 22:17:07 -0400
Subject: [PATCH] get rid of undefined behavior in union member mismatch for $@

The code for $@ depends on reading a_w.w_symbol values, but the other
A_DOLLAR branch (i.e., for $1 - $n) depend on reading a_w.w_index.

So for the conditional at the top of binbuf_eval, this means that any
normal A_DOLLAR that has its a_w.w_index set is actually trying to read
a_w.w_symbol. On a 64-bit arch with certain compilers, this will result
in a read of uninitalized data in the padding of the part of w_symbol
that exceeds the sizeof(int).

The workaround here is to define a sentinel value DOLLARALL to a negative
int unlikely to ever be used in real A_DOLLAR values. We then use DOLLARALL
to mean the "@" in "$@" so we can create a special branch for it in the
parser.

One consequence-- we probably need to add a special case error when
this value is out of range. So rather than confusing the user with the
mysterious negative value of DOLLARALL we print "$@" for them. (Currently,
the only place I can see this happening is when "$@" is used as the
target for a Pd message.)
---
 pd/src/m_atom.c   |  4 +++-
 pd/src/m_binbuf.c | 13 ++++++++-----
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/pd/src/m_atom.c b/pd/src/m_atom.c
index b38fa8d22..3c4f5be2e 100644
--- a/pd/src/m_atom.c
+++ b/pd/src/m_atom.c
@@ -6,6 +6,8 @@
 #include <stdio.h>
 #include <string.h>
 
+#define DOLLARALL -0x7fffffff /* defined in m_binbuf.c, too. Consider merging */
+
     /* convenience routines for checking and getting values of
         atoms.  There's no "pointer" version since there's nothing
         safe to return if there's an error. */
@@ -129,7 +131,7 @@ void atom_string(t_atom *a, char *buf, unsigned int bufsize)
     }
         break;
     case A_DOLLAR:
-        if(a->a_w.w_symbol == gensym("@"))
+        if(a->a_w.w_index == DOLLARALL)
         {
             /* JMZ: $@ expansion */
             sprintf(buf, "$@");
diff --git a/pd/src/m_binbuf.c b/pd/src/m_binbuf.c
index 9aa47d806..1f815aa6a 100644
--- a/pd/src/m_binbuf.c
+++ b/pd/src/m_binbuf.c
@@ -22,6 +22,8 @@
 #include <string.h>
 #include <stdarg.h>
 
+#define DOLLARALL -0x7fffffff /* sentinel value for "$@" dollar arg */
+
 /* escape characters for saving */
 static char* strnescape(char *dest, const char *src, size_t outlen)
 {
@@ -202,7 +204,7 @@ void binbuf_text(t_binbuf *x, char *text, size_t size)
                     if(buf[2]==0) /* only expand A_DOLLAR $@ */
                     {
                         ap->a_type = A_DOLLAR;
-                        ap->a_w.w_symbol = gensym("@");
+                        ap->a_w.w_index = DOLLARALL;
                     } 
                     else /* there is no A_DOLLSYM $@ */
                     {
@@ -392,7 +394,7 @@ void binbuf_addbinbuf(t_binbuf *x, t_binbuf *y)
             break;
         case A_DOLLAR:
             //fprintf(stderr,"addbinbuf: dollar\n");
-            if(ap->a_w.w_symbol==gensym("@")){ /* JMZ: $@ expansion */
+            if(ap->a_w.w_index==DOLLARALL){ /* JMZ: $@ expansion */
                 SETSYMBOL(ap, gensym("$@"));
             } else {
                 sprintf(tbuf, "$%d", ap->a_w.w_index);
@@ -456,7 +458,7 @@ void binbuf_restore(t_binbuf *x, int argc, t_atom *argv)
             else if (!strcmp(str, "$@")) /* JMZ: $@ expansion */
             {
                 ap->a_type = A_DOLLAR;
-                ap->a_w.w_symbol = gensym("@");
+                ap->a_w.w_index = DOLLARALL;
             }
             else if ((str2 = strchr(str, '$')) && str2[1] >= '0'
                 && str2[1] <= '9')
@@ -694,7 +696,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
     {
         //fprintf(stderr, "count %d\n", count);
         if (at[count].a_type == A_DOLLAR &&
-            at[count].a_w.w_symbol==gensym("@"))
+            at[count].a_w.w_index==DOLLARALL)
         {
             //fprintf(stderr,"found @ count:%d ac:%d argc:%d ac+argc-1:%d\n",
             //    count, ac, argc, ac+argc-1);
@@ -765,6 +767,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
             if (!ac) break;
             if (at->a_type == A_DOLLAR)
             {
+                /* would it make sense to consider $@ here? */
                 if (at->a_w.w_index <= 0 || at->a_w.w_index > argc)
                 {
                     error("$%d: not enough arguments supplied",
@@ -839,7 +842,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
                 *msp = *at;
                 break;
             case A_DOLLAR:
-                if (at->a_w.w_symbol==gensym("@")) 
+                if (at->a_w.w_index==DOLLARALL)
                 { /* JMZ: $@ expansion */
                     int i;
                     //if(msp+argc >= ems) 
-- 
GitLab