Commit 9d30a7cd authored by Jonathan Wilkes's avatar Jonathan Wilkes
Browse files

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.)
parent a5152c18
...@@ -6,6 +6,8 @@ ...@@ -6,6 +6,8 @@
#include <stdio.h> #include <stdio.h>
#include <string.h> #include <string.h>
#define DOLLARALL -0x7fffffff /* defined in m_binbuf.c, too. Consider merging */
/* convenience routines for checking and getting values of /* convenience routines for checking and getting values of
atoms. There's no "pointer" version since there's nothing atoms. There's no "pointer" version since there's nothing
safe to return if there's an error. */ safe to return if there's an error. */
...@@ -129,7 +131,7 @@ void atom_string(t_atom *a, char *buf, unsigned int bufsize) ...@@ -129,7 +131,7 @@ void atom_string(t_atom *a, char *buf, unsigned int bufsize)
} }
break; break;
case A_DOLLAR: case A_DOLLAR:
if(a->a_w.w_symbol == gensym("@")) if(a->a_w.w_index == DOLLARALL)
{ {
/* JMZ: $@ expansion */ /* JMZ: $@ expansion */
sprintf(buf, "$@"); sprintf(buf, "$@");
......
...@@ -22,6 +22,8 @@ ...@@ -22,6 +22,8 @@
#include <string.h> #include <string.h>
#include <stdarg.h> #include <stdarg.h>
#define DOLLARALL -0x7fffffff /* sentinel value for "$@" dollar arg */
/* escape characters for saving */ /* escape characters for saving */
static char* strnescape(char *dest, const char *src, size_t outlen) 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) ...@@ -202,7 +204,7 @@ void binbuf_text(t_binbuf *x, char *text, size_t size)
if(buf[2]==0) /* only expand A_DOLLAR $@ */ if(buf[2]==0) /* only expand A_DOLLAR $@ */
{ {
ap->a_type = 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 $@ */ else /* there is no A_DOLLSYM $@ */
{ {
...@@ -392,7 +394,7 @@ void binbuf_addbinbuf(t_binbuf *x, t_binbuf *y) ...@@ -392,7 +394,7 @@ void binbuf_addbinbuf(t_binbuf *x, t_binbuf *y)
break; break;
case A_DOLLAR: case A_DOLLAR:
//fprintf(stderr,"addbinbuf: dollar\n"); //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("$@")); SETSYMBOL(ap, gensym("$@"));
} else { } else {
sprintf(tbuf, "$%d", ap->a_w.w_index); sprintf(tbuf, "$%d", ap->a_w.w_index);
...@@ -456,7 +458,7 @@ void binbuf_restore(t_binbuf *x, int argc, t_atom *argv) ...@@ -456,7 +458,7 @@ void binbuf_restore(t_binbuf *x, int argc, t_atom *argv)
else if (!strcmp(str, "$@")) /* JMZ: $@ expansion */ else if (!strcmp(str, "$@")) /* JMZ: $@ expansion */
{ {
ap->a_type = A_DOLLAR; 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' else if ((str2 = strchr(str, '$')) && str2[1] >= '0'
&& str2[1] <= '9') && str2[1] <= '9')
...@@ -694,7 +696,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv) ...@@ -694,7 +696,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
{ {
//fprintf(stderr, "count %d\n", count); //fprintf(stderr, "count %d\n", count);
if (at[count].a_type == A_DOLLAR && 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", //fprintf(stderr,"found @ count:%d ac:%d argc:%d ac+argc-1:%d\n",
// count, ac, argc, ac+argc-1); // count, ac, argc, ac+argc-1);
...@@ -765,6 +767,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv) ...@@ -765,6 +767,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
if (!ac) break; if (!ac) break;
if (at->a_type == A_DOLLAR) 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) if (at->a_w.w_index <= 0 || at->a_w.w_index > argc)
{ {
error("$%d: not enough arguments supplied", error("$%d: not enough arguments supplied",
...@@ -839,7 +842,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv) ...@@ -839,7 +842,7 @@ void binbuf_eval(t_binbuf *x, t_pd *target, int argc, t_atom *argv)
*msp = *at; *msp = *at;
break; break;
case A_DOLLAR: case A_DOLLAR:
if (at->a_w.w_symbol==gensym("@")) if (at->a_w.w_index==DOLLARALL)
{ /* JMZ: $@ expansion */ { /* JMZ: $@ expansion */
int i; int i;
//if(msp+argc >= ems) //if(msp+argc >= ems)
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment