[llvm] r358337 - [CommandLineParser] Add DefaultOption flag
Ilya Biryukov via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 15 07:03:13 PDT 2019
+llvm-commits <llvm-commits at lists.llvm.org>
On Mon, Apr 15, 2019 at 3:56 PM Ilya Biryukov <ibiryukov at google.com> wrote:
> Hi Don,
>
> This change produces a failure during our integrate. I'll have to revert
> the change to unbreak us. If you can provide a fix really quickly, let me
> know and I'll wait for it to land instead.
>
> You can reproduce it under asan by running:
> $ ninja SupportTests && unittests/Support/SupportTests
>
> Here's the corresponding asan trace:
> [ RUN ] CommandLineTest.SetDefautValue
> =================================================================
> ==167392==ERROR: AddressSanitizer: stack-buffer-overflow on address
> 0x7ffc680fecec at pc 0x7f2d8c7c3c51 bp 0x7ffc680fe980 sp 0x7ffc680fe978
> READ of size 4 at 0x7ffc680fecec thread T0
> #0 0x7f2d8c7c3c50 in size ...llvm/include/llvm/ADT/SmallPtrSet.h:92:35
> #1 0x7f2d8c7c3c50 in empty ...llvm/include/llvm/ADT/SmallPtrSet.h:91
> #2 0x7f2d8c7c3c50 in (anonymous
> namespace)::CommandLineParser::addOption(llvm::cl::Option*, bool)
> ...llvm/lib/Support/CommandLine.cpp:204
> #3 0x7f2d8c7ce9c3 in ParseCommandLineOptions
> ...llvm/lib/Support/CommandLine.cpp:1202:5
> #4 0x7f2d8c7ce9c3 in llvm::cl::ParseCommandLineOptions(int, char
> const* const*, llvm::StringRef, llvm::raw_ostream*, char const*)
> ...llvm/lib/Support/CommandLine.cpp:1139
> #5 0x743717 in (anonymous
> namespace)::CommandLineTest_SetDefautValue_Test::TestBody()
> ...llvm/unittests/Support/CommandLineTest.cpp:794:3
> #6 0x7f2d8cc53430 in
> HandleExceptionsInMethodIfSupported<testing::Test, void>
> ...llvm/utils/unittest/googletest/src/gtest.cc
> #7 0x7f2d8cc53430 in testing::Test::Run()
> ...llvm/utils/unittest/googletest/src/gtest.cc:2474
> #8 0x7f2d8cc57a35 in testing::TestInfo::Run()
> ...llvm/utils/unittest/googletest/src/gtest.cc:2656:11
> #9 0x7f2d8cc59270 in testing::TestCase::Run()
> ...llvm/utils/unittest/googletest/src/gtest.cc:2774:28
> #10 0x7f2d8cc7a210 in testing::internal::UnitTestImpl::RunAllTests()
> ...llvm/utils/unittest/googletest/src/gtest.cc:4649:43
> #11 0x7f2d8cc793d6 in
> HandleExceptionsInMethodIfSupported<testing::internal::UnitTestImpl, bool>
> ...llvm/utils/unittest/googletest/src/gtest.cc
> #12 0x7f2d8cc793d6 in testing::UnitTest::Run()
> ...llvm/utils/unittest/googletest/src/gtest.cc:4257
> #13 0x7f2d8ccf7256 in RUN_ALL_TESTS
> ...llvm/utils/unittest/googletest/include/gtest/gtest.h:2233:46
> #14 0x7f2d8ccf7256 in main
> ...llvm/utils/unittest/UnitTestMain/TestMain.cpp:50
> #15 0x7f2d8b2f52b0 in __libc_start_main
> (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)
> #16 0x4c8029 in _start
> (...build-asan/unittests/Support/SupportTests+0x4c8029)
>
>
>
>
> On Sat, Apr 13, 2019 at 6:53 PM Don Hinton via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: dhinton
>> Date: Sat Apr 13 09:55:28 2019
>> New Revision: 358337
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=358337&view=rev
>> Log:
>> [CommandLineParser] Add DefaultOption flag
>>
>> Summary: Add DefaultOption flag to CommandLineParser which provides a
>> default option or alias, but allows users to override it for some
>> other purpose as needed.
>>
>> Also, add `-h` as a default alias to `-help`, which can be seamlessly
>> overridden by applications like llvm-objdump and llvm-readobj which
>> use `-h` as an alias for other options.
>>
>> Reviewers: alexfh, klimek
>>
>> Reviewed By: klimek
>>
>> Subscribers: MaskRay, mehdi_amini, inglorion, dexonsmith, hiraditya,
>> llvm-commits, jhenderson, arphaman, cfe-commits
>>
>> Tags: #clang, #llvm
>>
>> Differential Revision: https://reviews.llvm.org/D59746
>>
>> Added:
>> llvm/trunk/test/Support/
>> llvm/trunk/test/Support/check-default-options.txt
>> Modified:
>> llvm/trunk/docs/CommandLine.rst
>> llvm/trunk/include/llvm/Support/CommandLine.h
>> llvm/trunk/lib/Support/CommandLine.cpp
>> llvm/trunk/tools/llvm-opt-report/OptReport.cpp
>> llvm/trunk/unittests/Support/CommandLineTest.cpp
>>
>> Modified: llvm/trunk/docs/CommandLine.rst
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CommandLine.rst?rev=358337&r1=358336&r2=358337&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/docs/CommandLine.rst (original)
>> +++ llvm/trunk/docs/CommandLine.rst Sat Apr 13 09:55:28 2019
>> @@ -128,6 +128,7 @@ this:
>> USAGE: compiler [options]
>>
>> OPTIONS:
>> + -h - Alias for -help
>> -help - display available options (-help-hidden for more)
>> -o <filename> - Specify output filename
>>
>> @@ -194,6 +195,7 @@ declarations above, the ``-help`` option
>> USAGE: compiler [options] <input file>
>>
>> OPTIONS:
>> + -h - Alias for -help
>> -help - display available options (-help-hidden for more)
>> -o <filename> - Specify output filename
>>
>> @@ -1251,6 +1253,14 @@ specify boolean properties that modify t
>> with ``cl::CommaSeparated``, this modifier only makes sense with a
>> `cl::list`_
>> option.
>>
>> +.. _cl::DefaultOption:
>> +
>> +* The **cl::DefaultOption** modifier is used to specify that the option
>> is a
>> + default that can be overridden by application specific parsers. For
>> example,
>> + the ``-help`` alias, ``-h``, is registered this way, so it can be
>> overridden
>> + by applications that need to use the ``-h`` option for another purpose,
>> + either as a regular option or an alias for another option.
>> +
>> .. _response files:
>>
>> Response files
>>
>> Modified: llvm/trunk/include/llvm/Support/CommandLine.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Support/CommandLine.h?rev=358337&r1=358336&r2=358337&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/include/llvm/Support/CommandLine.h (original)
>> +++ llvm/trunk/include/llvm/Support/CommandLine.h Sat Apr 13 09:55:28 2019
>> @@ -175,7 +175,10 @@ enum MiscFlags { // Miscella
>> // If this is enabled, multiple letter options are allowed to bunch
>> together
>> // with only a single hyphen for the whole group. This allows
>> emulation
>> // of the behavior that ls uses for example: ls -la === ls -l -a
>> - Grouping = 0x08
>> + Grouping = 0x08,
>> +
>> + // Default option
>> + DefaultOption = 0x10
>> };
>>
>>
>> //===----------------------------------------------------------------------===//
>> @@ -270,7 +273,7 @@ class Option {
>> unsigned Value : 2;
>> unsigned HiddenFlag : 2; // enum OptionHidden
>> unsigned Formatting : 2; // enum FormattingFlags
>> - unsigned Misc : 4;
>> + unsigned Misc : 5;
>> unsigned Position = 0; // Position of last occurrence of the
>> option
>> unsigned AdditionalVals = 0; // Greater than 0 for multi-valued option.
>>
>> @@ -306,6 +309,7 @@ public:
>> bool hasArgStr() const { return !ArgStr.empty(); }
>> bool isPositional() const { return getFormattingFlag() ==
>> cl::Positional; }
>> bool isSink() const { return getMiscFlags() & cl::Sink; }
>> + bool isDefaultOption() const { return getMiscFlags() &
>> cl::DefaultOption; }
>>
>> bool isConsumeAfter() const {
>> return getNumOccurrencesFlag() == cl::ConsumeAfter;
>> @@ -382,7 +386,7 @@ public:
>> }
>>
>> inline int getNumOccurrences() const { return NumOccurrences; }
>> - inline void reset() { NumOccurrences = 0; }
>> + void reset();
>> };
>>
>>
>> //===----------------------------------------------------------------------===//
>> @@ -1732,7 +1736,10 @@ class alias : public Option {
>> error("cl::alias must have argument name specified!");
>> if (!AliasFor)
>> error("cl::alias must have an cl::aliasopt(option) specified!");
>> + if (!Subs.empty())
>> + error("cl::alias must not have cl::sub(), aliased option's
>> cl::sub() will be used!");
>> Subs = AliasFor->Subs;
>> + Category = AliasFor->Category;
>> addArgument();
>> }
>>
>>
>> Modified: llvm/trunk/lib/Support/CommandLine.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Support/CommandLine.cpp?rev=358337&r1=358336&r2=358337&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/lib/Support/CommandLine.cpp (original)
>> +++ llvm/trunk/lib/Support/CommandLine.cpp Sat Apr 13 09:55:28 2019
>> @@ -98,6 +98,11 @@ public:
>> // This collects additional help to be printed.
>> std::vector<StringRef> MoreHelp;
>>
>> + // This collects Options added with the cl::DefaultOption flag. Since
>> they can
>> + // be overridden, they are not added to the appropriate SubCommands
>> until
>> + // ParseCommandLineOptions actually runs.
>> + SmallVector<Option*, 4> DefaultOptions;
>> +
>> // This collects the different option categories that have been
>> registered.
>> SmallPtrSet<OptionCategory *, 16> RegisteredOptionCategories;
>>
>> @@ -146,6 +151,11 @@ public:
>> void addOption(Option *O, SubCommand *SC) {
>> bool HadErrors = false;
>> if (O->hasArgStr()) {
>> + // If it's a DefaultOption, check to make sure it isn't already
>> there.
>> + if (O->isDefaultOption() &&
>> + SC->OptionsMap.find(O->ArgStr) != SC->OptionsMap.end())
>> + return;
>> +
>> // Add argument to the argument map!
>> if (!SC->OptionsMap.insert(std::make_pair(O->ArgStr, O)).second) {
>> errs() << ProgramName << ": CommandLine Error: Option '" <<
>> O->ArgStr
>> @@ -185,7 +195,12 @@ public:
>> }
>> }
>>
>> - void addOption(Option *O) {
>> + void addOption(Option *O, bool ProcessDefaultOption = false) {
>> + if (!ProcessDefaultOption && O->isDefaultOption()) {
>> + DefaultOptions.push_back(O);
>> + return;
>> + }
>> +
>> if (O->Subs.empty()) {
>> addOption(O, &*TopLevelSubCommand);
>> } else {
>> @@ -201,8 +216,12 @@ public:
>> OptionNames.push_back(O->ArgStr);
>>
>> SubCommand &Sub = *SC;
>> - for (auto Name : OptionNames)
>> - Sub.OptionsMap.erase(Name);
>> + auto End = Sub.OptionsMap.end();
>> + for (auto Name : OptionNames) {
>> + auto I = Sub.OptionsMap.find(Name);
>> + if (I != End && I->getValue() == O)
>> + Sub.OptionsMap.erase(I);
>> + }
>>
>> if (O->getFormattingFlag() == cl::Positional)
>> for (auto Opt = Sub.PositionalOpts.begin();
>> @@ -266,8 +285,13 @@ public:
>> if (O->Subs.empty())
>> updateArgStr(O, NewName, &*TopLevelSubCommand);
>> else {
>> - for (auto SC : O->Subs)
>> - updateArgStr(O, NewName, SC);
>> + if (O->isInAllSubCommands()) {
>> + for (auto SC : RegisteredSubCommands)
>> + updateArgStr(O, NewName, SC);
>> + } else {
>> + for (auto SC : O->Subs)
>> + updateArgStr(O, NewName, SC);
>> + }
>> }
>> }
>>
>> @@ -366,6 +390,13 @@ void Option::setArgStr(StringRef S) {
>> ArgStr = S;
>> }
>>
>> +void Option::reset() {
>> + NumOccurrences = 0;
>> + setDefault();
>> + if (isDefaultOption())
>> + removeArgument();
>> +}
>> +
>> // Initialise the general option category.
>> OptionCategory llvm::cl::GeneralCategory("General options");
>>
>> @@ -1167,6 +1198,10 @@ bool CommandLineParser::ParseCommandLine
>> auto &SinkOpts = ChosenSubCommand->SinkOpts;
>> auto &OptionsMap = ChosenSubCommand->OptionsMap;
>>
>> + for (auto O: DefaultOptions) {
>> + addOption(O, true);
>> + }
>> +
>> if (ConsumeAfterOpt) {
>> assert(PositionalOpts.size() > 0 &&
>> "Cannot specify cl::ConsumeAfter without a positional
>> argument!");
>> @@ -2146,6 +2181,9 @@ static cl::opt<HelpPrinterWrapper, true,
>> cl::location(WrappedNormalPrinter), cl::ValueDisallowed,
>> cl::cat(GenericCategory), cl::sub(*AllSubCommands));
>>
>> +static cl::alias HOpA("h", cl::desc("Alias for -help"),
>> cl::aliasopt(HOp),
>> + cl::DefaultOption);
>> +
>> static cl::opt<HelpPrinterWrapper, true, parser<bool>>
>> HHOp("help-hidden", cl::desc("Display all available options"),
>> cl::location(WrappedHiddenPrinter), cl::Hidden,
>> cl::ValueDisallowed,
>>
>> Added: llvm/trunk/test/Support/check-default-options.txt
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Support/check-default-options.txt?rev=358337&view=auto
>>
>> ==============================================================================
>> --- llvm/trunk/test/Support/check-default-options.txt (added)
>> +++ llvm/trunk/test/Support/check-default-options.txt Sat Apr 13 09:55:28
>> 2019
>> @@ -0,0 +1,18 @@
>> +# RUN: llvm-objdump -help-hidden %t | FileCheck
>> --check-prefix=CHECK-OBJDUMP %s
>> +# RUN: llvm-readobj -help-hidden %t | FileCheck
>> --check-prefix=CHECK-READOBJ %s
>> +# RUN: llvm-tblgen -help-hidden %t | FileCheck
>> --check-prefix=CHECK-TBLGEN %s
>> +# RUN: llvm-opt-report -help-hidden %t | FileCheck
>> --check-prefix=CHECK-OPT-RPT %s
>> +# RUN: llvm-dwarfdump -help-hidden %t | FileCheck
>> --check-prefix=CHECK-DWARF %s
>> +# RUN: llvm-dwarfdump -h %t | FileCheck --check-prefix=CHECK-DWARF-H %s
>> +
>> +
>> +# CHECK-OBJDUMP: -h - Alias for --section-headers
>> +# CHECK-READOBJ: -h - Alias for --file-headers
>> +# CHECK-TBLGEN: -h - Alias for -help
>> +# CHECK-OPT-RPT: -h - Alias for -help
>> +# CHECK-DWARF: -h - Alias for -help
>> +
>> +# llvm-dwarfdump declares `-h` option and prints special help in that
>> case,
>> +# which is weird, but makes for a good test, i.e., shows the default `-h`
>> +# wasn't used.
>> +# CHECK-DWARF-H-NOT: -help-list - Display list of available options
>> (-help-list-hidden for more)
>>
>> Modified: llvm/trunk/tools/llvm-opt-report/OptReport.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-opt-report/OptReport.cpp?rev=358337&r1=358336&r2=358337&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/tools/llvm-opt-report/OptReport.cpp (original)
>> +++ llvm/trunk/tools/llvm-opt-report/OptReport.cpp Sat Apr 13 09:55:28
>> 2019
>> @@ -36,8 +36,6 @@
>> using namespace llvm;
>> using namespace llvm::yaml;
>>
>> -static cl::opt<bool> Help("h", cl::desc("Alias for -help"), cl::Hidden);
>> -
>> // Mark all our options with this category, everything else (except for
>> -version
>> // and -help) will be hidden.
>> static cl::OptionCategory
>> @@ -440,11 +438,6 @@ int main(int argc, const char **argv) {
>> "A tool to generate an optimization report from YAML optimization"
>> " record files.\n");
>>
>> - if (Help) {
>> - cl::PrintHelpMessage();
>> - return 0;
>> - }
>> -
>> LocationInfoTy LocationInfo;
>> if (!readLocationInfo(LocationInfo))
>> return 1;
>>
>> Modified: llvm/trunk/unittests/Support/CommandLineTest.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Support/CommandLineTest.cpp?rev=358337&r1=358336&r2=358337&view=diff
>>
>> ==============================================================================
>> --- llvm/trunk/unittests/Support/CommandLineTest.cpp (original)
>> +++ llvm/trunk/unittests/Support/CommandLineTest.cpp Sat Apr 13 09:55:28
>> 2019
>> @@ -620,6 +620,67 @@ TEST(CommandLineTest, GetRegisteredSubco
>> }
>> }
>>
>> +TEST(CommandLineTest, DefaultOptions) {
>> + cl::ResetCommandLineParser();
>> +
>> + StackOption<std::string> Bar("bar", cl::sub(*cl::AllSubCommands),
>> + cl::DefaultOption);
>> + StackOption<std::string, cl::alias> Bar_Alias(
>> + "b", cl::desc("Alias for -bar"), cl::aliasopt(Bar),
>> cl::DefaultOption);
>> +
>> + StackOption<bool> Foo("foo", cl::init(false),
>> cl::sub(*cl::AllSubCommands),
>> + cl::DefaultOption);
>> + StackOption<bool, cl::alias> Foo_Alias("f", cl::desc("Alias for -foo"),
>> + cl::aliasopt(Foo),
>> cl::DefaultOption);
>> +
>> + StackSubCommand SC1("sc1", "First Subcommand");
>> + // Override "-b" and change type in sc1 SubCommand.
>> + StackOption<bool> SC1_B("b", cl::sub(SC1), cl::init(false));
>> + StackSubCommand SC2("sc2", "Second subcommand");
>> + // Override "-foo" and change type in sc2 SubCommand. Note that this
>> does not
>> + // affect "-f" alias, which continues to work correctly.
>> + StackOption<std::string> SC2_Foo("foo", cl::sub(SC2));
>> +
>> + const char *args0[] = {"prog", "-b", "args0 bar string", "-f"};
>> + EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args0) / sizeof(char
>> *), args0,
>> + StringRef(), &llvm::nulls()));
>> + EXPECT_TRUE(Bar == "args0 bar string");
>> + EXPECT_TRUE(Foo);
>> + EXPECT_FALSE(SC1_B);
>> + EXPECT_TRUE(SC2_Foo.empty());
>> +
>> + cl::ResetAllOptionOccurrences();
>> +
>> + const char *args1[] = {"prog", "sc1", "-b", "-bar", "args1 bar
>> string", "-f"};
>> + EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args1) / sizeof(char
>> *), args1,
>> + StringRef(), &llvm::nulls()));
>> + EXPECT_TRUE(Bar == "args1 bar string");
>> + EXPECT_TRUE(Foo);
>> + EXPECT_TRUE(SC1_B);
>> + EXPECT_TRUE(SC2_Foo.empty());
>> + for (auto *S : cl::getRegisteredSubcommands()) {
>> + if (*S) {
>> + EXPECT_EQ("sc1", S->getName());
>> + }
>> + }
>> +
>> + cl::ResetAllOptionOccurrences();
>> +
>> + const char *args2[] = {"prog", "sc2", "-b", "args2 bar string",
>> + "-f", "-foo", "foo string"};
>> + EXPECT_TRUE(cl::ParseCommandLineOptions(sizeof(args2) / sizeof(char
>> *), args2,
>> + StringRef(), &llvm::nulls()));
>> + EXPECT_TRUE(Bar == "args2 bar string");
>> + EXPECT_TRUE(Foo);
>> + EXPECT_FALSE(SC1_B);
>> + EXPECT_TRUE(SC2_Foo == "foo string");
>> + for (auto *S : cl::getRegisteredSubcommands()) {
>> + if (*S) {
>> + EXPECT_EQ("sc2", S->getName());
>> + }
>> + }
>> +}
>> +
>> TEST(CommandLineTest, ArgumentLimit) {
>> std::string args(32 * 4096, 'a');
>> EXPECT_FALSE(llvm::sys::commandLineFitsWithinSystemLimits("cl",
>> args.data()));
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> --
> Regards,
> Ilya Biryukov
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20190415/944342cc/attachment.html>
More information about the llvm-commits
mailing list