Skip to content

Commit 2c20995

Browse files
authored
[RISCV] Detect duplicate extensions in parseNormalizedArchString. (#91416)
This detects the same extension name being added twice. Mostly I'm worried about the case that the same string appears with two different versions. We will only preserve one of the versions. We could allow the same version to be repeated, but that doesn't seem useful at the moment. I've updated addExtension to use map::emplace instead of map::operator[]. This means we only keep the first version if there are duplicates. Previously we kept the last version, but that shouldn't matter now that we don't allow duplicates. parseArchString already doesn't allow duplicates.
1 parent 812c302 commit 2c20995

File tree

3 files changed

+14
-4
lines changed

3 files changed

+14
-4
lines changed

llvm/include/llvm/TargetParser/RISCVISAInfo.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class RISCVISAInfo {
8787

8888
RISCVISAUtils::OrderedExtensionMap Exts;
8989

90-
void addExtension(StringRef ExtName, RISCVISAUtils::ExtensionVersion Version);
90+
bool addExtension(StringRef ExtName, RISCVISAUtils::ExtensionVersion Version);
9191

9292
Error checkDependency();
9393

llvm/lib/TargetParser/RISCVISAInfo.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,9 +159,9 @@ findDefaultVersion(StringRef ExtName) {
159159
return std::nullopt;
160160
}
161161

162-
void RISCVISAInfo::addExtension(StringRef ExtName,
162+
bool RISCVISAInfo::addExtension(StringRef ExtName,
163163
RISCVISAUtils::ExtensionVersion Version) {
164-
Exts[ExtName.str()] = Version;
164+
return Exts.emplace(ExtName, Version).second;
165165
}
166166

167167
static StringRef getExtensionTypeDesc(StringRef Ext) {
@@ -492,7 +492,9 @@ RISCVISAInfo::parseNormalizedArchString(StringRef Arch) {
492492
"'" + Twine(ExtName[0]) +
493493
"' must be followed by a letter");
494494

495-
ISAInfo->addExtension(ExtName, {MajorVersion, MinorVersion});
495+
if (!ISAInfo->addExtension(ExtName, {MajorVersion, MinorVersion}))
496+
return createStringError(errc::invalid_argument,
497+
"duplicate extension '" + ExtName + "'");
496498
}
497499
ISAInfo->updateImpliedLengths();
498500
return std::move(ISAInfo);

llvm/unittests/TargetParser/RISCVISAInfoTest.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,14 @@ TEST(ParseNormalizedArchString, RejectsBadX) {
7878
}
7979
}
8080

81+
TEST(ParseNormalizedArchString, DuplicateExtension) {
82+
for (StringRef Input : {"rv64i2p0_a2p0_a1p0"}) {
83+
EXPECT_EQ(
84+
toString(RISCVISAInfo::parseNormalizedArchString(Input).takeError()),
85+
"duplicate extension 'a'");
86+
}
87+
}
88+
8189
TEST(ParseNormalizedArchString, AcceptsValidBaseISAsAndSetsXLen) {
8290
auto MaybeRV32I = RISCVISAInfo::parseNormalizedArchString("rv32i2p0");
8391
ASSERT_THAT_EXPECTED(MaybeRV32I, Succeeded());

0 commit comments

Comments
 (0)