Skip to content
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

Jump to bottom

ICU-22934 Limit the number of resursive call #3230

Merged
merged 1 commit into from
Oct 2, 2024
Merged

ICU-22934 Limit the number of resursive call #3230

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 41 additions & 15 deletions icu4c/source/common/rbbinode.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,26 +193,40 @@ void RBBINode::NRDeleteNode(RBBINode *node) {
// references in preparation for generating the DFA tables.
//
//-------------------------------------------------------------------------
RBBINode *RBBINode::cloneTree() {
constexpr int kRecursiveDepthLimit = 3500;
RBBINode *RBBINode::cloneTree(UErrorCode &status, int depth) {
if (U_FAILURE(status)) {
return nullptr;
}
// If the depth of the stack is too deep, we return U_INPUT_TOO_LONG_ERROR
// to avoid stack overflow crash.
if (depth > kRecursiveDepthLimit) {
status = U_INPUT_TOO_LONG_ERROR;
return nullptr;
}
RBBINode *n;

if (fType == RBBINode::varRef) {
// If the current node is a variable reference, skip over it
// and clone the definition of the variable instead.
n = fLeftChild->cloneTree();
n = fLeftChild->cloneTree(status, depth+1);
} else if (fType == RBBINode::uset) {
n = this;
} else {
n = new RBBINode(*this);
// Check for null pointer.
if (n != nullptr) {
if (fLeftChild != nullptr) {
n->fLeftChild = fLeftChild->cloneTree();
n->fLeftChild->fParent = n;
n->fLeftChild = fLeftChild->cloneTree(status, depth+1);
if (U_SUCCESS(status)) {
n->fLeftChild->fParent = n;
}
}
if (fRightChild != nullptr) {
n->fRightChild = fRightChild->cloneTree();
n->fRightChild->fParent = n;
n->fRightChild = fRightChild->cloneTree(status, depth+1);
if (U_SUCCESS(status)) {
n->fRightChild->fParent = n;
}
}
}
}
Expand All @@ -239,7 +253,6 @@ RBBINode *RBBINode::cloneTree() {
// nested references are handled by cloneTree(), not here.
//
//-------------------------------------------------------------------------
constexpr int kRecursiveDepthLimit = 3500;
RBBINode *RBBINode::flattenVariables(UErrorCode& status, int depth) {
if (U_FAILURE(status)) {
return this;
Expand All @@ -251,7 +264,7 @@ RBBINode *RBBINode::flattenVariables(UErrorCode& status, int depth) {
return this;
}
if (fType == varRef) {
RBBINode *retNode = fLeftChild->cloneTree();
RBBINode *retNode = fLeftChild->cloneTree(status, depth+1);
if (retNode != nullptr) {
retNode->fRuleRoot = this->fRuleRoot;
retNode->fChainIn = this->fChainIn;
Expand Down Expand Up @@ -280,19 +293,30 @@ RBBINode *RBBINode::flattenVariables(UErrorCode& status, int depth) {
// the left child of the uset node.
//
//-------------------------------------------------------------------------
void RBBINode::flattenSets() {
void RBBINode::flattenSets(UErrorCode &status, int depth) {
if (U_FAILURE(status)) {
return;
}
// If the depth of the stack is too deep, we return U_INPUT_TOO_LONG_ERROR
// to avoid stack overflow crash.
if (depth > kRecursiveDepthLimit) {
status = U_INPUT_TOO_LONG_ERROR;
return;
}
U_ASSERT(fType != setRef);

if (fLeftChild != nullptr) {
if (fLeftChild->fType==setRef) {
RBBINode *setRefNode = fLeftChild;
RBBINode *usetNode = setRefNode->fLeftChild;
RBBINode *replTree = usetNode->fLeftChild;
fLeftChild = replTree->cloneTree();
fLeftChild->fParent = this;
fLeftChild = replTree->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
fLeftChild->fParent = this;
}
delete setRefNode;
} else {
fLeftChild->flattenSets();
fLeftChild->flattenSets(status, depth+1);
}
}

Expand All @@ -301,11 +325,13 @@ void RBBINode::flattenSets() {
RBBINode *setRefNode = fRightChild;
RBBINode *usetNode = setRefNode->fLeftChild;
RBBINode *replTree = usetNode->fLeftChild;
fRightChild = replTree->cloneTree();
fRightChild->fParent = this;
fRightChild = replTree->cloneTree(status, depth+1);
if (U_FAILURE(status)) {
fRightChild->fParent = this;
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks wrong. Did you intend to return on failure?

}
delete setRefNode;
} else {
fRightChild->flattenSets();
fRightChild->flattenSets(status, depth+1);
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions icu4c/source/common/rbbinode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,9 @@ class RBBINode : public UMemory {
~RBBINode();
static void NRDeleteNode(RBBINode *node);

RBBINode *cloneTree();
RBBINode *cloneTree(UErrorCode &status, int depth=0);
RBBINode *flattenVariables(UErrorCode &status, int depth=0);
void flattenSets();
void flattenSets(UErrorCode &status, int depth=0);
void findNodes(UVector *dest, RBBINode::NodeType kind, UErrorCode &status);

#ifdef RBBI_DEBUG
Expand Down
2 changes: 1 addition & 1 deletion icu4c/source/common/rbbitblb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ void RBBITableBuilder::buildForwardTable() {
// Replace all references to UnicodeSets with the tree for the equivalent
// expression.
//
fTree->flattenSets();
fTree->flattenSets(*fStatus, 0);
#ifdef RBBI_DEBUG
if (fRB->fDebugEnv && uprv_strstr(fRB->fDebugEnv, "stree")) {
RBBIDebugPuts("\nParse tree after flattening Unicode Set references.");
Expand Down