From 875f05828d95251abe7c943d79399a3b1c80db12 Mon Sep 17 00:00:00 2001 From: Igor Kudrin Date: Fri, 1 Mar 2019 09:20:56 +0000 Subject: [PATCH] [CommandLine] Do not crash if an option has both ValueRequired and Grouping. If an option, which requires a value, has a `cl::Grouping` formatting modifier, it works well as far as it is used at the end of a group, or as a separate argument. However, if the option appears accidentally in the middle of a group, the program just crashes. This patch prints an error message instead. Differential Revision: https://reviews.llvm.org/D58499 llvm-svn: 355184 --- llvm/docs/CommandLine.rst | 3 ++- llvm/lib/Support/CommandLine.cpp | 11 +++++++---- llvm/unittests/Support/CommandLineTest.cpp | 22 ++++++++++++++++++++++ 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/llvm/docs/CommandLine.rst b/llvm/docs/CommandLine.rst index 9a6a196b431c..8f3207ef0832 100644 --- a/llvm/docs/CommandLine.rst +++ b/llvm/docs/CommandLine.rst @@ -1172,7 +1172,8 @@ As usual, you can only specify one of these arguments at most. ``ls``) that have lots of single letter arguments, but only require a single dash. For example, the '``ls -labF``' command actually enables four different options, all of which are single letters. Note that **cl::Grouping** options - cannot have values. + can have values only if they are used separately or at the end of the groups. + It is a runtime error if such an option is used elsewhere in the group. The CommandLine library does not restrict how you use the **cl::Prefix** or **cl::Grouping** modifiers, but it is possible to specify ambiguous argument diff --git a/llvm/lib/Support/CommandLine.cpp b/llvm/lib/Support/CommandLine.cpp index b0c92b7c72c4..8b279baf8d60 100644 --- a/llvm/lib/Support/CommandLine.cpp +++ b/llvm/lib/Support/CommandLine.cpp @@ -671,10 +671,13 @@ HandlePrefixedOrGroupedOption(StringRef &Arg, StringRef &Value, StringRef OneArgName = Arg.substr(0, Length); Arg = Arg.substr(Length); - // Because ValueRequired is an invalid flag for grouped arguments, - // we don't need to pass argc/argv in. - assert(PGOpt->getValueExpectedFlag() != cl::ValueRequired && - "Option can not be cl::Grouping AND cl::ValueRequired!"); + if (PGOpt->getValueExpectedFlag() == cl::ValueRequired) { + ErrorParsing |= PGOpt->error("may not occur within a group!"); + return nullptr; + } + + // Because the value for the option is not required, we don't need to pass + // argc/argv in. int Dummy = 0; ErrorParsing |= ProvideOption(PGOpt, OneArgName, StringRef(), 0, nullptr, Dummy); diff --git a/llvm/unittests/Support/CommandLineTest.cpp b/llvm/unittests/Support/CommandLineTest.cpp index f9841f580b1c..4839a31f0c13 100644 --- a/llvm/unittests/Support/CommandLineTest.cpp +++ b/llvm/unittests/Support/CommandLineTest.cpp @@ -1128,4 +1128,26 @@ TEST(CommandLineTest, PrefixOptions) { EXPECT_TRUE(MacroDefs.front().compare("HAVE_FOO") == 0); } +TEST(CommandLineTest, GroupingWithValue) { + cl::ResetCommandLineParser(); + + StackOption OptF("f", cl::Grouping, cl::desc("Some flag")); + StackOption OptV("v", cl::Grouping, + cl::desc("Grouping option with a value")); + + // Should be possible to use an option which requires a value + // at the end of a group. + const char *args1[] = {"prog", "-fv", "val1"}; + EXPECT_TRUE( + cl::ParseCommandLineOptions(3, args1, StringRef(), &llvm::nulls())); + EXPECT_TRUE(OptF); + EXPECT_STREQ("val1", OptV.c_str()); + cl::ResetAllOptionOccurrences(); + + // Should not crash if it is accidentally used elsewhere in the group. + const char *args2[] = {"prog", "-vf", "val2"}; + EXPECT_FALSE( + cl::ParseCommandLineOptions(3, args2, StringRef(), &llvm::nulls())); +} + } // anonymous namespace -- 2.34.1