Skip to content

Commit

Permalink
nvalue sanitisation project part 2: enum (re: 0686c70, d4adc8f)
Browse files Browse the repository at this point in the history
Unlike 'typeset -si' short int values, enum values (which are
glorified unsigned short ints) were still stored directly in the
nvalue union. As of this commit, they use pointers to allocated
memory like other values.

Notable changes:

src/cmd/ksh93/bltins/enum.c:
- Assign NV_INT16P (which uses value pointers) instead of NV_INT16
  attribute bits to enum variables when assigning or creating them.

src/cmd/ksh93/include/streval.h:
- struct lval: Rename opaquely named enum-related 'ptr' and 'eflag'
  members to 'enum_p' and 'isenum', respectively.

src/cmd/ksh93/sh/arith.c: arith():
- When comparing an enum's value using the == or != operators in an
  arithmetic expression, the rvalue (right-hand value) may be an
  enum value if the lvalue is a variable of an enum type. Update
  the code that handles this to no longer require the NV_NOFREE
  attribute on the temporary node, as short ints (including enums)
  now use pointers and allocated memory.
- Fix a pre-existing memory leak. In the old version of that code
  block, while the value of the temporary node was not freeable,
  the discipline function entry (nvfun) did need to be freed. Make
  sure everything is freed properly by calling _nv_unset().

src/cmd/ksh93/sh/array.c: nv_getsub():
- Fix enum subscripts to use the current method: malloc a value and
  assign the subscript numeric value via the pointer instead of
  directly to the nvalue union.

src/cmd/ksh93/tests/leaks.sh:
- Add test for the leak fixed in arith.c.
  • Loading branch information
McDutchie committed Nov 29, 2024
1 parent 0686c70 commit 0ebaab3
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 27 deletions.
6 changes: 3 additions & 3 deletions src/cmd/ksh93/bltins/enum.c
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
#include "shopt.h"
#include "defs.h"

#define ENUM_ID "enum (ksh 93u+m) 2022-03-05"
#define ENUM_ID "enum (ksh 93u+m) 2024-11-28"

const char sh_optenum[] =
"[-?\n@(#)$Id: " ENUM_ID " $\n]"
Expand Down Expand Up @@ -183,7 +183,7 @@ static void put_enum(Namval_t* np,const char *val,int flags,Namfun_t *fp)
n = strcmp(v,val);
if(n==0)
{
nv_putv(np, (char*)&i, NV_UINT16, fp);
nv_putv(np, (char*)&i, NV_UINT16P, fp);
return;
}
i++;
Expand Down Expand Up @@ -260,7 +260,7 @@ int b_enum(int argc, char** argv, Shbltin_t *context)
tp = nv_open(sfstruse(sh.strbuf), sh.var_tree, NV_VARNAME);
n = sz;
i = 0;
nv_onattr(tp, NV_UINT16);
nv_onattr(tp, NV_UINT16P);
nv_putval(tp, (char*)&i, NV_INTEGER);
nv_putsub(np, NULL, ARRAY_SCAN);
do
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/ksh93/include/streval.h
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ struct lval
char *ovalue;
Sfdouble_t (*fun)(Sfdouble_t,...);
const char *expr;
const void *ptr;
const void *enum_p; /* pointer to the lvalue's enum type */
int nosub;
char *sub;
short flag;
short nargs;
short emode;
short level;
short elen;
char eflag;
char isenum; /* set if the lvalue is of an enum type */
char isfloat;
};

Expand Down
26 changes: 15 additions & 11 deletions src/cmd/ksh93/sh/arith.c
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,9 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
}
}
nv_putval(np, (char*)&n, NV_LDOUBLE);
if(lvalue->eflag)
lvalue->ptr = nv_hasdisc(np,&ENUM_disc);
lvalue->eflag = 0;
if(lvalue->isenum)
lvalue->enum_p = nv_hasdisc(np,&ENUM_disc);
lvalue->isenum = 0;
lvalue->value = (char*)np;
/*
* The result (r) of an assignment is its value (n), cast to the type of the variable
Expand Down Expand Up @@ -435,7 +435,7 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
else
{
char lastbase=0, *val = xp, oerrno = errno;
lvalue->eflag = 0;
lvalue->isenum = 0;
errno = 0;
r = strtonll(val,&str, &lastbase,-1);
if(lastbase==8 && *val=='0' && !sh_isoption(sh.bltinfun==b_let ? SH_LETOCTAL : SH_POSIX))
Expand Down Expand Up @@ -504,20 +504,24 @@ static Sfdouble_t arith(const char **ptr, struct lval *lvalue, int type, Sfdoubl
return 0;
}
lvalue->ovalue = (char*)np;
if(lvalue->eflag)
lvalue->ptr = nv_hasdisc(np,&ENUM_disc);
else if((Namfun_t*)lvalue->ptr && !nv_hasdisc(np,&ENUM_disc) && !nv_isattr(np,NV_INTEGER))
if(lvalue->isenum)
{
lvalue->enum_p = nv_hasdisc(np,&ENUM_disc);
lvalue->isenum = 0;
}
else if(lvalue->enum_p && !nv_hasdisc(np,&ENUM_disc) && !nv_isattr(np,NV_INTEGER))
{
/* convert the enum rvalue of the lvalue's enum type to its number */
Namval_t *mp,node;
mp = ((Namfun_t*)lvalue->ptr)->type;
mp = ((Namfun_t*)lvalue->enum_p)->type;
memset(&node,0,sizeof(node));
nv_clone(mp,&node,0);
nv_offattr(&node,NV_RDONLY|NV_NOFREE);
nv_putval(&node,np->nvname,0);
if(nv_isattr(&node,NV_NOFREE))
return r=nv_getnum(&node);
r = nv_getnum(&node);
_nv_unset(&node,0);
return r;
}
lvalue->eflag = 0;
if(((lvalue->emode&2) || lvalue->level>1 || sh_isoption(SH_NOUNSET)) && nv_isnull(np) && !nv_isattr(np,NV_INTEGER))
{
*ptr = nv_name(np);
Expand Down
10 changes: 6 additions & 4 deletions src/cmd/ksh93/sh/array.c
Original file line number Diff line number Diff line change
Expand Up @@ -1593,9 +1593,11 @@ char *nv_getsub(Namval_t* np)
if(is_associative(ap))
return (char*)((*ap->header.fun)(np,NULL,NV_ANAME));
if(ap->xp)
{
{ /* enum subscript */
np = nv_namptr(ap->xp,0);
np->nvalue.s = ap->cur;
if(!np->nvalue.sp)
np->nvalue.sp = malloc(sizeof(uint16_t));
*((uint16_t*)np->nvalue.sp) = ap->cur;
return nv_getval(np);
}
if((dot = ap->cur)==0)
Expand Down Expand Up @@ -1755,11 +1757,11 @@ void *nv_associative(Namval_t *np,const char *sp,int mode)
if(sh.subshell)
sh_assignok(np,1);
/*
* For enum types (NV_UINT16 with discipline ENUM_disc), nelem should not
* For enum types (NV_UINT16P with discipline ENUM_disc), nelem should not
* increase or 'unset' will fail to completely unset such an array.
*/
if((!ap->header.scope || !nv_search(sp,dtvnext(ap->header.table),0))
&& !(type==NV_UINT16 && nv_hasdisc(np, &ENUM_disc)))
&& !(type==NV_UINT16P && nv_hasdisc(np, &ENUM_disc)))
ap->header.nelem++;
if(nv_isnull(mp))
{
Expand Down
14 changes: 7 additions & 7 deletions src/cmd/ksh93/sh/streval.c
Original file line number Diff line number Diff line change
Expand Up @@ -162,8 +162,8 @@ Sfdouble_t arith_exec(Arith_t *ep)
node.value = 0;
node.nosub = 0;
node.sub = 0;
node.ptr = 0;
node.eflag = 0;
node.enum_p = 0;
node.isenum = 0;
if(sh.arithrecursion++ >= MAXLEVEL)
{
arith_error(e_recursive,ep->expr,ep->emode);
Expand Down Expand Up @@ -266,7 +266,7 @@ Sfdouble_t arith_exec(Arith_t *ep)
c = 0;
break;
case A_ENUM:
node.eflag = 1;
node.isenum = 1;
continue;
case A_ASSIGNOP:
node.nosub = lastsub;
Expand All @@ -282,10 +282,10 @@ Sfdouble_t arith_exec(Arith_t *ep)
node.value = (char*)dp;
node.flag = c;
if(lastval)
node.eflag = 1;
node.ptr = 0;
node.isenum = 1;
node.enum_p = 0;
num = (*ep->fun)(&ptr,&node,ASSIGN,num);
if(lastval && node.ptr)
if(lastval && node.enum_p)
{
Sfdouble_t r;
node.flag = 0;
Expand Down Expand Up @@ -479,7 +479,7 @@ Sfdouble_t arith_exec(Arith_t *ep)
lastval = 0;
if(c&T_BINARY)
{
node.ptr = 0;
node.enum_p = 0;
sp--,tp--;
type |= (*tp!=0);
}
Expand Down
8 changes: 8 additions & 0 deletions src/cmd/ksh93/tests/leaks.sh
Original file line number Diff line number Diff line change
Expand Up @@ -423,5 +423,13 @@ DO
unset baz
DONE

# ======
TEST title='assigning and comparing enum rvalue in arithmetic'
enum Test1_t=(lorem ipsum dolor sit amet consectetur adipiscing elit curabitur scelerisque massa nec diam fermentum tempor)
Test1_t foo
DO
(((foo = amet) && foo == amet && foo != fermentum))
DONE

# ======
exit $((Errors<125?Errors:125))

0 comments on commit 0ebaab3

Please sign in to comment.