Skip to content

Commit 9441ea9

Browse files
andyclaude
andcommitted
TaskJob::getChildren(): fix iterator-invalidation UB, likely real cause of "internal files never show"
Same bug shape as the 16 sites fixed earlier today (GUIValues.C, DavEDSI.C, etc., all found via that session's own audit): both filter loops here called ret->erase(resItor) without reseating resItor from erase()'s return value. erase() invalidates the iterator it's given; the for loop's own next condition check then read a dangling iterator -- undefined behavior, not just on the entry being filtered out but for the rest of the iteration. This is a strong, independent explanation for a longstanding report: internal files (Inputs/Outputs, and without ECCE_DEVELOPER set, Parameters/Props) have reportedly never actually displayed even when "Show ECCE Internal Files" is on and nothing crashes -- unrelated to the tree-reentrancy crashes fixed earlier today. Silently corrupting the iteration right after the first filtered-out entry would explain exactly that: whatever child happened to land next in the vector could be silently dropped from the result too, not just the one meant to be filtered. Audited the same erase(iterator)-without-reseat pattern tree-wide again; found one more site (Session.C:171) but it's safe (erase() immediately followed by break, never touching the iterator again). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 parent 8e847ff commit 9441ea9

1 file changed

Lines changed: 16 additions & 3 deletions

File tree

‎src/dsm/edsiimpl/TaskJob.C‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -122,11 +122,24 @@ vector<Resource*> * TaskJob::getChildren(bool refresh)
122122
vector<Resource*>::iterator resItor;
123123

124124
// Hide the Parameters and Props folder if developer flag is not set
125+
//
126+
// Both loops here used to call ret->erase(resItor) without reseating
127+
// resItor from erase()'s return value -- erase() invalidates the
128+
// iterator it's given, so the *next* loop check (resItor != ret->end())
129+
// read a dangling iterator, undefined behavior. Confirmed as the real
130+
// explanation for internal files (and, without ECCE_DEVELOPER set,
131+
// Parameters/Props) never actually appearing even once "Show ECCE
132+
// Internal Files" is on and nothing crashes: this UB was silently
133+
// dropping whatever child happened to land right after the erased
134+
// entry from the rest of the iteration, not just the entry meant to
135+
// be filtered. Same bug shape as 16 other sites fixed earlier today
136+
// (GUIValues.C, DavEDSI.C, etc.) -- reseat from erase()'s return value
137+
// instead of a separate resItor++ outside the erase branch.
125138
if (userFlag) {
126139
for (resItor = ret->begin(); resItor != ret->end();) {
127140
if ((*resItor)->getName() == "Parameters" ||
128141
(*resItor)->getName() == "Props" ) {
129-
ret->erase(resItor);
142+
resItor = ret->erase(resItor);
130143
} else {
131144
resItor++;
132145
}
@@ -137,11 +150,11 @@ vector<Resource*> * TaskJob::getChildren(bool refresh)
137150
for (resItor = ret->begin(); resItor != ret->end();) {
138151
if ((*resItor)->getName() == "Inputs" ||
139152
(*resItor)->getName() == "Outputs" ) {
140-
ret->erase(resItor);
153+
resItor = ret->erase(resItor);
141154
}
142155
else if (!userFlag && ((*resItor)->getName() == "Parameters" ||
143156
(*resItor)->getName() == "Props" )) {
144-
ret->erase(resItor);
157+
resItor = ret->erase(resItor);
145158
} else {
146159
resItor++;
147160
}

0 commit comments

Comments
 (0)