Skip to content

Fix: strict-aliasing violations in treeshr #2983

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
joshStillerman wants to merge 4 commits into
base: alpha
Choose a base branch
from
+35 −33
Open

Fix: strict-aliasing violations in treeshr #2983

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
047bbb9
Fix: strict-aliasing violations in treeshr
joshStillerman Nov 6, 2025
9b26d4e
remove bogus comment about volatile
joshStillerman Nov 6, 2025
6e4747c
Another co-pilot suggested fix
joshStillerman Nov 6, 2025
fc7ce43
use sizeof(var) instead of sizeof var
joshStillerman Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion treeshr/TreeAddNode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ int _TreeAddNode(void *dbid, char const *name, int *nid_out, char usage)
&scratch_nci, &ncilocked);
if (STATUS_OK)
{
if (_TreeIsOn(dblist, *(int *)&parent_nid) & 1)
if (_TreeIsOn(dblist, parent_nid) & 1)
new_nci.flags &= (unsigned)~NciM_PARENT_STATE;
else
new_nci.flags |= NciM_PARENT_STATE;
Expand Down
4 changes: 2 additions & 2 deletions treeshr/TreeDeleteNode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -223,8 +223,8 @@ extern void _TreeDeleteNodeExecute(void *dbid)
while (_TreeDeleteNodeGetNid(dbid, (int *)&nid) & 1)
{
int found = 0;
_TreeRemoveNodesTags(dbid, *(int *)&nid);
_TreeSetNoSubtree(dbid, *(int *)&nid);
_TreeRemoveNodesTags(dbid, nid_to_int(&nid));
_TreeSetNoSubtree(dbid, nid_to_int(&nid));
node = nid_to_node(dblist, &nid);
parent = parent_of(0, node);
if (child_of(0, parent) == node)
Expand Down
17 changes: 10 additions & 7 deletions treeshr/TreeGetNci.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ extern void **TreeCtx();
static char *Treename(PINO_DATABASE *dblist, int nid_in)
{
TREE_INFO *info;
NID nid = *(NID *)&nid_in;
NID nid = int_to_nid(nid_in);
unsigned int treenum;
for (info = dblist->tree_info, treenum = 0; info && treenum < nid.tree;
info = info->next_info)
Expand Down Expand Up @@ -220,7 +220,7 @@ int TreeGetNci(int nid_in, struct nci_itm *nci_itm)
void *dbid = *TreeCtx();
INIT_STATUS_AS TreeSUCCESS;
PINO_DATABASE *dblist = (PINO_DATABASE *)dbid;
NID nid = *(NID *)&nid_in;
NID nid = int_to_nid(nid_in);
int node_number;
TREE_INFO *info;
NCI_ITM *itm;
Expand Down Expand Up @@ -449,7 +449,7 @@ int TreeGetNci(int nid_in, struct nci_itm *nci_itm)
for (node = child_of(dblist, node); node; count++,
node = brother_of(dblist, node) ? brother_of(dblist, node) : 0)
;
*(int *)(itm->pointer) = count;
memcpy(itm->pointer, &count, sizeof(count));
break;
case NciNUMBER_OF_MEMBERS:
break_on_no_node;
Expand All @@ -459,7 +459,7 @@ int TreeGetNci(int nid_in, struct nci_itm *nci_itm)
for (node = member_of(node); node; count++,
node = brother_of(dblist, node) ? brother_of(dblist, node) : 0)
;
*(int *)(itm->pointer) = count;
memcpy(itm->pointer, &count, sizeof(count));
break;
case NciNUMBER_OF_ELTS:
break_on_no_node;
Expand All @@ -468,7 +468,7 @@ int TreeGetNci(int nid_in, struct nci_itm *nci_itm)
for (count = 0; swapint16(&cng_node->conglomerate_elt) > count;
count++, cng_node++)
;
*(int *)(itm->pointer) = count;
memcpy(itm->pointer, &count, sizeof(count));
break;
case NciCHILDREN_NIDS:
{
Expand Down Expand Up @@ -665,7 +665,10 @@ int TreeGetNci(int nid_in, struct nci_itm *nci_itm)
*(NID *)itm->pointer = out_nid;
}
else
*(int *)itm->pointer = 0;
{
int zero = 0;
memcpy(itm->pointer, &zero, sizeof(zero));
}
break;
}
case NciDTYPE_STR:
Expand Down Expand Up @@ -766,7 +769,7 @@ static char *getPath(PINO_DATABASE *dblist, NODE *node, int remove_tree_refs)
for (info = dblist->tree_info, i = 0; info && i < nid.tree;
i++, info = info->next_info)
;
if ((tag = _TreeFindNodeTags((void *)dblist, *(int *)&nid, &ctx)) != NULL)
if ((tag = _TreeFindNodeTags((void *)dblist, nid_to_int(&nid), &ctx)) != NULL)
{
string[0] = '\\';
string[1] = '\0';
Expand Down
2 changes: 1 addition & 1 deletion treeshr/TreeGetRecord.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,7 @@ typedef ARRAY(struct descriptor *) array_dsc;
int TreeMakeNidsLocal(struct descriptor *dsc_ptr, int nid)
{
int status = 1;
unsigned char tree = ((NID *)&nid)->tree;
unsigned char tree = int_to_nid(nid).tree;
if (dsc_ptr == NULL)
status = 1;
else
Expand Down
12 changes: 4 additions & 8 deletions treeshr/TreeOpen.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1175,22 +1175,18 @@ static void SubtreeNodeConnect(PINO_DATABASE *dblist, NODE *parent,
NODE *subtreetop)
{
NID child_nid, parent_nid = {0, 0};
Copy link
Contributor

@heidthecamp heidthecamp Nov 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while we're making changes here, should we move each of these on their own line?

/*
* make brother_nid volatile so that the optimizer does not
* decide to optimize it out and replace it with a zero.
*/
volatile NID brother_nid = {0,0};
NID brother_nid = {0,0};
NODE *brother = brother_of(dblist, parent);
parent->usage = TreeUSAGE_SUBTREE_REF;
subtreetop->usage = TreeUSAGE_SUBTREE_TOP;
node_to_nid(dblist, subtreetop, &child_nid);
node_to_nid(dblist, parent_of(dblist, parent), &parent_nid);
parent->child = *(int *)&child_nid;
parent->child = nid_to_int(&child_nid);
if (brother) {
node_to_nid(dblist, brother_of(dblist, parent), &brother_nid);
}
subtreetop->brother = *(int *)&brother_nid;
subtreetop->parent = *(int *)&parent_nid;
subtreetop->brother = nid_to_int(&brother_nid);
subtreetop->parent = nid_to_int(&parent_nid);
memcpy(subtreetop->name, parent->name, sizeof(subtreetop->name));
return;
}
Expand Down
4 changes: 2 additions & 2 deletions treeshr/TreeRenameNode.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,8 +256,8 @@ static int FixParentState(PINO_DATABASE *dblist, NODE *parent_ptr,
in the flag longword and the parent state argument
to SET_PARENT_STATE are negative boolean logic.
****************************************************/
parent_state = _TreeIsOn(dblist, *(int *)&parent_nid) & 1;
status = _TreeGetNci(dblist, *(int *)&child_nid, child_itm_list);
parent_state = _TreeIsOn(dblist, nid_to_int(&parent_nid)) & 1;
status = _TreeGetNci(dblist, nid_to_int(&child_nid), child_itm_list);
if (STATUS_OK)
{
child_parent_state = ((child_flags & NciM_PARENT_STATE) == 0);
Expand Down
27 changes: 15 additions & 12 deletions treeshr/treeshrp.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -396,17 +396,6 @@ typedef struct
unsigned char rfa[6];
} RFA;

#ifdef RFA_MACROS
#define RfaToSeek(rfa) \
(((*(unsigned int *)rfa - 1) * 512) + \
(*(unsigned short *)&((char *)rfa)[4] & 0x1ff))
#define SeekToRfa(seek, rfa) \
{ \
*(unsigned int *)rfa = (unsigned int)(seek / 512 + 1); \
*(unsigned short *)&(((char *)rfa)[4]) = (unsigned short)(seek % 512); \
}
#endif

/****************************************
RECORD_HEADER
VFC portion of file.
Expand Down Expand Up @@ -642,6 +631,20 @@ typedef struct pino_database
void *dispatch_table; /* pointer to dispatch table generated by dispatch/build */
} PINO_DATABASE;

static inline int nid_to_int(NID *nid)
{
int result;
memcpy(&result, nid, sizeof(int));
return result;
}

static inline NID int_to_nid(int nid_int)
{
NID nid;
memcpy(&nid, &nid_int, sizeof(int));
return nid;
}

static inline NODE *nid_to_node(PINO_DATABASE *dbid, NID *nid)
{
TREE_INFO *info;
Expand Down Expand Up @@ -793,7 +796,7 @@ static inline int node_to_nid(PINO_DATABASE *dbid, NODE *node, NID *nid_out)
nid.tree = 0;
if (nid_out)
*nid_out = nid;
return *(int *)&nid;
return nid_to_int(&nid);
}

/******************************************
Expand Down