[Clang][Driver][Test] Created test for unsupported driver options

[GeorgeKA]

Closes #119190

Created generate_unsupported_in_drivermode.py which generates Lit regression test that validate that options are only exposed to intended driver modes.

The options and driver modes are parsed from Options.td, whose path should be provided on the command line. See
clang/include/clang/Driver/Options.td

The path to the TableGen executable can optionally be provided. Otherwise, the script will search for it.

Proposed workflow

• A change is made to clang/include/clang/Driver/Options.td
• Regenerate the Lit tests that validate exposure of options to intended driver modes
  python generate_unsupported_in_drivermode.py --llvm-bin <path>/llvm-project/build/bin --llvm-tblgen llvm-tblgen ../include/clang/Driver/Options.td
• Run the Lit tests to check exposure
  llvm-lit -v clang/test/Driver/unsupported_in_drivermode.c
llvm-lit -v clang/test/Driver/flang/unsupported_in_drivermode.c
llvm-lit -v flang/test/Driver/unsupported_in_flang_fc1.f90

[github-actions]

✅ With the latest revision this PR passed the Python code formatter.

[Maetveis]

Hi, Thanks for working on this issue. I wanted to preface this by feel free to ignore this during the holidays, zero pressure. I like to interact with the community in my free time, but that obviously does not need to extend to everyone :).

What kind of failures are you seeing in the generated test? There shouldn't be any afaict if the script works correctly, as Options.td IS the source where the parsing code for the various drivers are generated from. Are you perhaps missing the case where there is no visibility specified at all and it should be assumed to be DefaultVisibility, i.e. the gcc compatible clang driver?

[GeorgeKA]

@Maetveis

All of the Visibility fields appear to be not null, but that's a good check to add anyway. Thanks. After adding it though, the same failures occur. Here's an example. Maybe there's a gap in my understanding:

The script generates tests for both flang & clang -cc1as when looking at the O_flag option. cc1as outputs the unexpected error message, but flang does not.

$ clang -cc1as -O_flag -help 2>&1 | less
clang -cc1as: error: unknown argument: '-O_flag'

$ flang -O_flag -help 2>&1 | less
<regular help message>

These are the steps I'm taking for manual validation:

1. Generate JSON dump for Options.td
   llvm-tblgen -I <llvm repo>/llvm/include <llvm repo>/clang/include/clang/Driver/Options.td -dump-json | jq . > Options.td.json
2. Navigate to "O_flag": {
3. "Group" is null, so no extra visibilities to check
4. "Visibility" includes DefaultVis, CC1Option, & FC1Option, which leaves unsupported Visibilities as CC1AsOption, CLOption, DXCOption, FlangOption, so the script generates run lines & checks for these four.

For reference, full list of drivers/visibilities include:

"CC1AsOption"
"CC1Option"
"CLOption"
"DXCOption"
"DefaultVis"
"FC1Option"
"FlangOption"

[Maetveis]

@Maetveis

All of the Visibility fields appear to be not null, but that's a good check to add anyway. Thanks. After adding it though, the same failures occur. Here's an example. Maybe there's a gap in my understanding:

The script generates tests for both flang & clang -cc1as when looking at the O_flag option. cc1as outputs the unexpected error message, but flang does not.

That looks to be by design, because you are using the -help, the drivers meant to be user facing (clang, clang-cl and flang) don't report unknown options with -help. A user might tack on a -help after their broken command line when they are looking for the exact spelling of their option so it makes sense that it is not an error to have unknown options when mixed with -help.

Simply you shouldn't be using -help, instead passing options to perform a real compilation on the command line like clang-cl <tested-options> -### -x c++ -c - < /dev/null should work better.

The other issues is that -O_flag is parsed as -O _flag for drivers that support -O, in runtime longer valid options have preference over shorter ones, in this case I think if there is a supported joined option (Kind == JoinedClass) that is a prefix of an unsupported option you now way to test the unsupported option so it should be skipped.

[GeorgeKA]

@Maetveis
Do you have any thoughts on the following two groups of options that are unexpectedly succeeding?
I'll note that I did come across some options for which the driver appears to correct user error by adding the appropriate driver option, like testing "-fexperimental-sanitize-metadata=atomics" with the default driver and the driver adding -cc1, but I'm not aware of a supported -cc1 option that outputs the final list of options used like -### does for the default driver.

1. "fheinous-gnu-extensions" & "fcuda-approx-transcendentals" with -cc1
   Both of these unexpectedly succeed with clang -cc1. Notably, they both have aliases that are supported for -cc1, (ex: fheinous-gnu-extensions), but those aliases expectedly fail when used with -cc1, which suggests aliases aren't the issue.

Ex: clang -cc1 -fheinous-gnu-extensions -x c++ - < /dev/null 2>&1

2. "mno-strict-align" & "mstrict-align" with -cc1
   Both of these unexpectedly succeed with clang -cc1. Both of these have no alias. They're part of the m_Group and the CompileOnly_Group, which don't include -cc1 for visibility.

Ex: clang -cc1 -mno-strict-align -x c++ - < /dev/null 2>&1

[Maetveis]

Sorry I was caught up with work, and didn't find the time to look at this until now.

@Maetveis Do you have any thoughts on the following two groups of options that are unexpectedly succeeding? I'll note that I did come across some options for which the driver appears to correct user error by adding the appropriate driver option, like testing "-fexperimental-sanitize-metadata=atomics" with the default driver and the driver adding -cc1

For -fexperimental-sanitize-metadata=atomics I think it should be hidden by -fexperimental-sanitize-metadata=<anything> (Which is driver only). Maybe your search logic for hidden options has bugs, but I didn't dive into it to check.

1. "fheinous-gnu-extensions" & "fcuda-approx-transcendentals" with -cc1
   Both of these unexpectedly succeed with clang -cc1. Notably, they both have aliases that are supported for -cc1, (ex: fheinous-gnu-extensions), but those aliases expectedly fail when used with -cc1, which suggests aliases aren't the issue.

Ex: clang -cc1 -fheinous-gnu-extensions -x c++ - < /dev/null 2>&1

2. "mno-strict-align" & "mstrict-align" with -cc1
   Both of these unexpectedly succeed with clang -cc1. Both of these have no alias. They're part of the m_Group and the CompileOnly_Group, which don't include -cc1 for visibility.

Ex: clang -cc1 -mno-strict-align -x c++ - < /dev/null 2>&1

These all produce errors for me. Are you sure you're testing everything right? You have an up-to-date clang build for example? The testing tool should give you an output like llvm-lit: (...)/llvm/utils/lit/lit/llvm/config.py:506: note: using clang (...)/build/bin/clang that should be pointing into the llvm build folder.

[GeorgeKA]

Ahh, thanks. You're right, I had environment issues. Now that Lit is pointing to an up to date clang, the exceptions I mentioned properly fail. That's a relief.

Fyi, I think all that's left before it's ready for review is handling for aliases. I'll try and add that later today or tomorrow.

[GeorgeKA]

Hi @Maetveis . This is ready for review.

There are a few things that I came across while making the script that I should highlight :

1. clang -cc1 test directory

test/Driver/lit.local.cfg says, '("%clang_cc1", """*** Do not use 'clang -cc1' in Driver tests. ***""")', yet I see various tests in that directory that do so, like linker-wrapper.c. If there's no opposition, I'll leave the cc1 tests in the main Lit test file, Driver/unsupported_in_drivermode.c.

2. flang vs flang -fc1

As per clang/test/Driver/flang/flang.f90, "flang -fc1 is invoked when in --driver-mode=flang", so I disabled the plain flang tests since there's no distinction.

3. Testing on Windows

I disabled the tests on Windows given the following recursion error that occurs on Windows regression machines. (It has some odd formatting. Pasting in a text editor makes viewing it easier.)

	^[_bk;t=1737572755765^GPASS: Clang :: Driver/fsanitize-memory-param-retval.c (11017 of 21306)^M                                                                                                             ^[_bk;t=1737572756104^G******************** TEST 'Clang :: Driver/flang/unsupported_in_flang.f90' FAILED ********************^M                                                                             ^[_bk;t=1737572756104^G******************** TEST 'Clang :: Driver/flang/unsupported_in_flang.f90' FAILED ********************^M                                                                             ^[_bk;t=1737572756104^GTraceback (most recent call last):^M                                                                                                                                                 ^[_bk;t=1737572756104^GTraceback (most recent call last):^M                                                                                                                                                 ^[_bk;t=1737572756104^GTraceback (most recent call last):^M                                                                                                                                                 ^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\formats\shtest.py", line 29, in execute^M
	^[_bk;t=1737572756104^G    return lit.TestRunner.executeShTest(^M
	^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 2326, in executeShTest^M                                                                                                   ^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 2326, in executeShTest^M                                                                                                   ^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 2326, in executeShTest^M                                                                                                   ^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 2326, in executeShTest^M                                                                                                   ^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 2326, in executeShTest^M                                                                                                   ^[_bk;t=1737572756104^G    res = executeScriptInternal(^M
	^[_bk;t=1737572756104^G  File "C:\ws\src\llvm\utils\lit\lit\TestRunner.py", line 1110, in executeScriptInternal^M
	^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    exitCode, timeoutInfo = executeShCmd(^M                                                                                                                                          ^[_bk;t=1737572756104^G    res = _executeShCmd(cmd.lhs, shenv, results, timeoutHelper)^M                                                                                                                    ^[_bk;t=1737572756104^G    res = _executeShCmd(cmd.lhs, shenv, results, timeoutHelper)^M                                                                                                                    ^[_bk;t=1737572756104^G    res = _executeShCmd(cmd.lhs, shenv, results, timeoutHelper)^M                                                                                                                    ^[_bk;t=1737572756104^G    if timeoutHelper.timeoutReached():^M                                                                                                                                             ^[_bk;t=1737572756104^G    if timeoutHelper.timeoutReached():^M

[Maetveis]

Thanks, this is starting to look right. I'm sorry for the long response time, I could not justify spending work time on the review this time, and my weekends have been somewhat busy.

I'll try to get some other reviewers to look at this too, to approve of the proposed workflow when updating Options.td. I would appreciate if you could make a summary of that as a comment, but if not I'll do it later.

[GeorgeKA]

@Maetveis, referring to the proposed workflow review comment, not sure if you meant to leave a comment in the script, or here in the review. I'll leave it here and in the description for now, but let me know if you mean the script.

Proposed workflow

• A change is made to clang/include/clang/Driver/Options.td
• Regenerate the Lit tests that validate exposure of options to intended driver modes
  python generate_unsupported_in_drivermode.py --llvm-bin <path>/llvm-project/build/bin --llvm-tblgen llvm-tblgen ../include/clang/Driver/Options.td
• Run the Lit tests to check exposure
  llvm-lit -v clang/test/Driver/unsupported_in_drivermode.c
llvm-lit -v clang/test/Driver/flang/unsupported_in_drivermode.c
llvm-lit -v flang/test/Driver/unsupported_in_flang_fc1.f90

[GeorgeKA]

• I should draw attention to a change I made when it comes to option prefixes. Put simply, I had to switch to using prefix "-" instead of "/" because of different error output for "/". I describe it here.

• Secondly, I had to add handling for matching option names, which was distinguished before by differing prefixes. Described here

• Also, I moved the flang fc1 testing to flang/test/Driver/unsupported_in_flang_fc1.f90 in order to use %flang_fc1

[Maetveis]

Hey @GeorgeKA, there's still some unaddressed comments from my previous review, I thought you were still working on it, so I did not want to post more comments.

I realize the latency I'm taking to respond is dragging this out quite a bit. Also it seems like the good first issue label might have been inaccurate, I really didn't expect so much complexity. I'd be happy to help with implementing some of my suggestions if you are okay with that. Github allows to do this by opening a pull request against this branch in your fork of the repository.

[GeorgeKA]

Hi @Maetveis.

there's still some unaddressed comments from my previous review, I thought you were still working on it

Oh my mistake. I didn't get a notification, so I didn't see the most recent comments.

Also it seems like the good first issue label might have been inaccurate, I really didn't expect so much complexity.

Don't sweat it. I appreciate the opportunity.

I'd be happy to help with implementing some of my suggestions if you are okay with that. Github allows to do this by opening a pull request against this branch in your fork of the repository.

Fine by me.

[GeorgeKA]

Ping @Maetveis
Any more thoughts on this one? I think I've addressed the existing comments.