[Transforms] Allow non-regex Source in SymbolRewriter in case of using ExplicitRewriteDescriptor #154319
Conversation
|
@llvm/pr-subscribers-llvm-transforms Author: Dmitry Vasilyev (slydiman) ChangesExplicitRewriteDescriptor expects a non-regex Source. But unconditional verification that Source is a valid regex breaks this logic if the Source contains $ in the middle or some other special characters. Full diff: https://github.com/llvm/llvm-project/pull/154319.diff 1 Files Affected:
diff --git a/llvm/lib/Transforms/Utils/SymbolRewriter.cpp b/llvm/lib/Transforms/Utils/SymbolRewriter.cpp
index d52d52a9b7d3e..6319fd524ff0f 100644
--- a/llvm/lib/Transforms/Utils/SymbolRewriter.cpp
+++ b/llvm/lib/Transforms/Utils/SymbolRewriter.cpp
@@ -349,13 +349,7 @@ parseRewriteFunctionDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
KeyValue = Key->getValue(KeyStorage);
if (KeyValue == "source") {
- std::string Error;
-
Source = std::string(Value->getValue(ValueStorage));
- if (!Regex(Source).isValid(Error)) {
- YS.printError(Field.getKey(), "invalid regex: " + Error);
- return false;
- }
} else if (KeyValue == "target") {
Target = std::string(Value->getValue(ValueStorage));
} else if (KeyValue == "transform") {
@@ -379,12 +373,22 @@ parseRewriteFunctionDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
// TODO see if there is a more elegant solution to selecting the rewrite
// descriptor type
- if (!Target.empty())
+ if (!Target.empty()) {
DL->push_back(std::make_unique<ExplicitRewriteFunctionDescriptor>(
Source, Target, Naked));
- else
- DL->push_back(
- std::make_unique<PatternRewriteFunctionDescriptor>(Source, Transform));
+ return true;
+ }
+
+ {
+ std::string Error;
+ if (!Regex(Source).isValid(Error)) {
+ YS.printError(Descriptor, "invalid Source regex: " + Error);
+ return false;
+ }
+ }
+
+ DL->push_back(
+ std::make_unique<PatternRewriteFunctionDescriptor>(Source, Transform));
return true;
}
@@ -418,13 +422,7 @@ parseRewriteGlobalVariableDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
KeyValue = Key->getValue(KeyStorage);
if (KeyValue == "source") {
- std::string Error;
-
Source = std::string(Value->getValue(ValueStorage));
- if (!Regex(Source).isValid(Error)) {
- YS.printError(Field.getKey(), "invalid regex: " + Error);
- return false;
- }
} else if (KeyValue == "target") {
Target = std::string(Value->getValue(ValueStorage));
} else if (KeyValue == "transform") {
@@ -441,13 +439,23 @@ parseRewriteGlobalVariableDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
return false;
}
- if (!Target.empty())
+ if (!Target.empty()) {
DL->push_back(std::make_unique<ExplicitRewriteGlobalVariableDescriptor>(
Source, Target,
/*Naked*/ false));
- else
- DL->push_back(std::make_unique<PatternRewriteGlobalVariableDescriptor>(
- Source, Transform));
+ return true;
+ }
+
+ {
+ std::string Error;
+ if (!Regex(Source).isValid(Error)) {
+ YS.printError(Descriptor, "invalid Source regex: " + Error);
+ return false;
+ }
+ }
+
+ DL->push_back(std::make_unique<PatternRewriteGlobalVariableDescriptor>(
+ Source, Transform));
return true;
}
@@ -481,13 +489,7 @@ parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
KeyValue = Key->getValue(KeyStorage);
if (KeyValue == "source") {
- std::string Error;
-
Source = std::string(Value->getValue(ValueStorage));
- if (!Regex(Source).isValid(Error)) {
- YS.printError(Field.getKey(), "invalid regex: " + Error);
- return false;
- }
} else if (KeyValue == "target") {
Target = std::string(Value->getValue(ValueStorage));
} else if (KeyValue == "transform") {
@@ -504,13 +506,23 @@ parseRewriteGlobalAliasDescriptor(yaml::Stream &YS, yaml::ScalarNode *K,
return false;
}
- if (!Target.empty())
+ if (!Target.empty()) {
DL->push_back(std::make_unique<ExplicitRewriteNamedAliasDescriptor>(
Source, Target,
/*Naked*/ false));
- else
- DL->push_back(std::make_unique<PatternRewriteNamedAliasDescriptor>(
- Source, Transform));
+ return true;
+ }
+
+ {
+ std::string Error;
+ if (!Regex(Source).isValid(Error)) {
+ YS.printError(Descriptor, "invalid Source regex: " + Error);
+ return false;
+ }
+ }
+
+ DL->push_back(
+ std::make_unique<PatternRewriteNamedAliasDescriptor>(Source, Transform));
return true;
}
|
I would be happy to add tests. |
There's one here https://github.com/llvm/llvm-project/tree/main/llvm/test/SymbolRewriter |
Thanks. I have added the test. Please take a look. |
…g ExplicitRewriteDescriptor ExplicitRewriteDescriptor expects a non-regex Source. But unconditional verification that Source is a valid regex breaks this logic if the Source contains $ in the middle or some other special characters.
1d8381e to
794bf48
Compare
Done.
I wouldn't refactor this code. This code assumes 3rd party extension. Even the description at the beginning contains recommendations |
Do not check that Source is a valid regex in case of Target (explicit) transformation. Source may contain special symbols that may cause an incorrect
invalid regexerror.Note that source and exactly one of [Target, Transform] must be provided.
Target (explicit transformation): In this kind of ruleSourceis treated as a symbol name and is matched in its entirety.Targetfield will denote the symbol name to transform to.Transform (pattern transformation): This rule treatsSourceas a regex that should match the complete symbol name.Transformis a regex specifying the name to transform to.