Skip to content

Commit 62e4a77

Browse files
snps-riyazserge-sans-paille
authored and
serge-sans-paille
committed
[Support] Fix for two issues with clearing of the internal storage for cl::bits
This patch fixes two issues with clearing of the internal storage for cl::bits 1. The internal bits storage for cl::bits is uninitialized. This is a problem if a cl::bits option is not defined with static lifetime. 2. ResetAllOptionOccurrences does not reset cl::bits options. The latter is also discussed in: https://lists.llvm.org/pipermail/llvm-dev/2021-February/148299.html Differential Revision: https://reviews.llvm.org/D119066
1 parent fda2926 commit 62e4a77

File tree

2 files changed

+22
-7
lines changed

2 files changed

+22
-7
lines changed

llvm/include/llvm/Support/CommandLine.h

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1694,7 +1694,7 @@ template <class DataType, class StorageClass> class bits_storage {
16941694
unsigned *Location = nullptr; // Where to store the bits...
16951695

16961696
template <class T> static unsigned Bit(const T &V) {
1697-
unsigned BitPos = reinterpret_cast<unsigned>(V);
1697+
unsigned BitPos = static_cast<unsigned>(V);
16981698
assert(BitPos < sizeof(unsigned) * CHAR_BIT &&
16991699
"enum exceeds width of bit vector!");
17001700
return 1 << BitPos;
@@ -1719,6 +1719,11 @@ template <class DataType, class StorageClass> class bits_storage {
17191719

17201720
unsigned getBits() { return *Location; }
17211721

1722+
void clear() {
1723+
if (Location)
1724+
*Location = 0;
1725+
}
1726+
17221727
template <class T> bool isSet(const T &V) {
17231728
return (*Location & Bit(V)) != 0;
17241729
}
@@ -1728,10 +1733,10 @@ template <class DataType, class StorageClass> class bits_storage {
17281733
// This makes us exactly compatible with the bits in all cases that it is used.
17291734
//
17301735
template <class DataType> class bits_storage<DataType, bool> {
1731-
unsigned Bits; // Where to store the bits...
1736+
unsigned Bits{0}; // Where to store the bits...
17321737

17331738
template <class T> static unsigned Bit(const T &V) {
1734-
unsigned BitPos = (unsigned)V;
1739+
unsigned BitPos = static_cast<unsigned>(V);
17351740
assert(BitPos < sizeof(unsigned) * CHAR_BIT &&
17361741
"enum exceeds width of bit vector!");
17371742
return 1 << BitPos;
@@ -1742,6 +1747,8 @@ template <class DataType> class bits_storage<DataType, bool> {
17421747

17431748
unsigned getBits() { return Bits; }
17441749

1750+
void clear() { Bits = 0; }
1751+
17451752
template <class T> bool isSet(const T &V) { return (Bits & Bit(V)) != 0; }
17461753
};
17471754

@@ -1788,7 +1795,7 @@ class bits : public Option, public bits_storage<DataType, Storage> {
17881795
void printOptionValue(size_t /*GlobalWidth*/, bool /*Force*/) const override {
17891796
}
17901797

1791-
void setDefault() override {}
1798+
void setDefault() override { bits_storage<DataType, Storage>::clear(); }
17921799

17931800
void done() {
17941801
addArgument();

llvm/unittests/Support/CommandLineTest.cpp

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1931,20 +1931,27 @@ TEST(CommandLineTest, ConsumeAfterTwoPositionals) {
19311931
TEST(CommandLineTest, ResetAllOptionOccurrences) {
19321932
cl::ResetCommandLineParser();
19331933

1934-
// -option [sink] input [args]
1934+
// -option -enableA -enableC [sink] input [args]
19351935
StackOption<bool> Option("option");
1936+
enum Vals { ValA, ValB, ValC };
1937+
StackOption<Vals, cl::bits<Vals>> Bits(
1938+
cl::values(clEnumValN(ValA, "enableA", "Enable A"),
1939+
clEnumValN(ValB, "enableB", "Enable B"),
1940+
clEnumValN(ValC, "enableC", "Enable C")));
19361941
StackOption<std::string, cl::list<std::string>> Sink(cl::Sink);
19371942
StackOption<std::string> Input(cl::Positional);
19381943
StackOption<std::string, cl::list<std::string>> ExtraArgs(cl::ConsumeAfter);
19391944

1940-
const char *Args[] = {"prog", "-option", "-unknown", "input", "-arg"};
1945+
const char *Args[] = {"prog", "-option", "-enableA", "-enableC",
1946+
"-unknown", "input", "-arg"};
19411947

19421948
std::string Errs;
19431949
raw_string_ostream OS(Errs);
1944-
EXPECT_TRUE(cl::ParseCommandLineOptions(5, Args, StringRef(), &OS));
1950+
EXPECT_TRUE(cl::ParseCommandLineOptions(7, Args, StringRef(), &OS));
19451951
EXPECT_TRUE(OS.str().empty());
19461952

19471953
EXPECT_TRUE(Option);
1954+
EXPECT_EQ((1u << ValA) | (1u << ValC), Bits.getBits());
19481955
EXPECT_EQ(1u, Sink.size());
19491956
EXPECT_EQ("-unknown", Sink[0]);
19501957
EXPECT_EQ("input", Input);
@@ -1953,6 +1960,7 @@ TEST(CommandLineTest, ResetAllOptionOccurrences) {
19531960

19541961
cl::ResetAllOptionOccurrences();
19551962
EXPECT_FALSE(Option);
1963+
EXPECT_EQ(0u, Bits.getBits());
19561964
EXPECT_EQ(0u, Sink.size());
19571965
EXPECT_EQ(0, Input.getNumOccurrences());
19581966
EXPECT_EQ(0u, ExtraArgs.size());

0 commit comments

Comments
 (0)