Add a "-gmodules" category to the test suite.

[tfiala]

Summary

This is a work-in-progress patch for adding a "-gmodules" category to the testsuite. It adds clang module debugging as a configuration alongside .dSYM and DWO to ensure that module debug info is getting broader coverage. When enabled it will add "-fmodules -gmodules" to CFLAGS (but not CXXFLAGS). It will not invoke dsymutil, since dsymutil undoes most of the module-related intricacies.

In the current form, this passes the test suite on Darwin, and it should also work on any other platform that uses clang as the host compiler.

I do have a couple of remaining questions regarding the implementation:

• Is there a way to detect that clang is the host compiler, so we can disable the category on platforms that use a different compiler?
• The "-fmodules" and "-gmodules" are only well supported by clang-3.9 and later. Is there a good way to test for this in is_supported_on_platform() or somewhere else?

[tfiala]

I think we can limit the tests to skip unless clang of a certain version, which will probably take care of it. I'll see if I can get the right decorator incantation for this. (It's a bit complicated by the fact that official Apple clang releases show up as the much larger version numbers, while the same clang built on the local machine will show up with the LLVM.org version numbers).

[labath]

I am glad to see more testing of the modules debugging. I have a couple of small comments though:

• -fmodules: Why is it not being added to CXXFLAGS? Is this how clang is supposed to be invoked? (I am not very familiar clang modules)
• there is a @skipUnlessClangModules decorator in decorators.py. As far as I can see, this patch should now make it obsolete. It seems that it can be removed and all invocations replaced with add_test_categories(["gmodules"])

And one meta-comment not directly related to this patch:
We already run most of the tests two times. Now we will be doing it once more, which will increase the test times even more. I think it's important for each debug info format to have good coverage, but I also feel that there are tests which have nothing to do with debug info (or their connection to debug info is only very peripheral), and it does not make to sense to slow down the tests runs by running those tests so many times. We already have a (not very elegant, but working) mechanism to avoid this (NO_DEBUG_INFO_TESTCASE member). I propose that we be more aggressive in using it for new tests which do not specifically test debug info. Also when looking at existing tests, we should re-evaluate whether the test really needs to be run that many times (right now, the largest candidate that comes to mind is TestConcurrentEvents, but I am sure there are others I can't think of by name now).

packages/Python/lldbsuite/test/lldbtest.py

1488 ↗

(On Diff #56351)

Shouldn't this be gmodules_test_method?

[aprantl]

In D19998#423277, @labath wrote:

I am glad to see more testing of the modules debugging. I have a couple of small comments though:

• -fmodules: Why is it not being added to CXXFLAGS? Is this how clang is supposed to be invoked? (I am not very familiar clang modules)

C++ modules are still a work in progress and not supported on all platforms (particularly on Darwin due to the way the system module maps interact with libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the platforms where C++ modules work well (such as Linux) on the other hand, module debugging hasn't been productized so far. Due to the way module debugging reuses DWO mechanisms I don't expect it to work without some fine-tuning.

• there is a @skipUnlessClangModules decorator in decorators.py. As far as I can see, this patch should now make it obsolete. It seems that it can be removed and all invocations replaced with add_test_categories(["gmodules"])

Good catch. I didn't notice that!

And one meta-comment not directly related to this patch:
We already run most of the tests two times. Now we will be doing it once more, which will increase the test times even more. I think it's important for each debug info format to have good coverage, but I also feel that there are tests which have nothing to do with debug info (or their connection to debug info is only very peripheral), and it does not make to sense to slow down the tests runs by running those tests so many times. We already have a (not very elegant, but working) mechanism to avoid this (NO_DEBUG_INFO_TESTCASE member). I propose that we be more aggressive in using it for new tests which do not specifically test debug info. Also when looking at existing tests, we should re-evaluate whether the test really needs to be run that many times (right now, the largest candidate that comes to mind is TestConcurrentEvents, but I am sure there are others I can't think of by name now).

That sounds like an all-around good idea, but probably out of scope for this patch.

[tfiala]

I propose that we be more aggressive in using it for new tests which do not specifically test debug info.

I totally agree, but I also caution that, as a debugger, the heart of much of what we do does use debug info. So we need to be careful to make sure that we don't disable the fan-out across all debug styles if we are in fact using debug info.

[labath]

In D19998#423541, @aprantl wrote:

In D19998#423277, @labath wrote:

I am glad to see more testing of the modules debugging. I have a couple of small comments though:

• -fmodules: Why is it not being added to CXXFLAGS? Is this how clang is supposed to be invoked? (I am not very familiar clang modules)

C++ modules are still a work in progress and not supported on all platforms (particularly on Darwin due to the way the system module maps interact with libc++, see https://llvm.org/bugs/show_bug.cgi?id=26928 for examples). On the platforms where C++ modules work well (such as Linux) on the other hand, module debugging hasn't been productized so far. Due to the way module debugging reuses DWO mechanisms I don't expect it to work without some fine-tuning.

Thank you for the comprehensive explanation. This information is interesting. Does that mean that in case of tests with c++ source code, there will be no difference in the test executables between modules and non-modules versions of the tests? I am wondering whether we should avoid running the modules tests in this case... I just did a quick count and we seem to have 129 C source files vs. 262 C++ files, so the difference is not actually as big as I expected, but it is still a substantial amount of test time going to waste. Maybe it's not that important though, we can possibly optimize that later, if it turns out to be necessary.

And one meta-comment not directly related to this patch:
We already run most of the tests two times. Now we will be doing it once more, which will increase the test times even more. I think it's important for each debug info format to have good coverage, but I also feel that there are tests which have nothing to do with debug info (or their connection to debug info is only very peripheral), and it does not make to sense to slow down the tests runs by running those tests so many times. We already have a (not very elegant, but working) mechanism to avoid this (NO_DEBUG_INFO_TESTCASE member). I propose that we be more aggressive in using it for new tests which do not specifically test debug info. Also when looking at existing tests, we should re-evaluate whether the test really needs to be run that many times (right now, the largest candidate that comes to mind is TestConcurrentEvents, but I am sure there are others I can't think of by name now).

That sounds like an all-around good idea, but probably out of scope for this patch.

Yes, I am definitely not requesting you make any changes like that here. I using just testing the waters about what people think about this idea. It's a bit of a hijack though, so sorry about that. We can move the discussion outside, if it starts to get more involved.

In D19998#423562, @tfiala wrote:

I propose that we be more aggressive in using it for new tests which do not specifically test debug info.

I totally agree, but I also caution that, as a debugger, the heart of much of what we do does use debug info. So we need to be careful to make sure that we don't disable the fan-out across all debug styles if we are in fact using debug info.

I guess the question here is what do we consider as "using the debug info". To take TestConcurrentEvents as an example, it certainly *uses* debug info, in the sense that it sets a breakpoint at a certain line, and sets some variables, but I don't think this is what the test is about. It is about testing that the debugger handles the situation where there are multiple events happening in the target concurrently, which should not be related to the debug info at all. Any failure it this test which would be caused by a debug info problem should already be caught by some other test which actually targets these things.

At least, that's my opinion...

packages/Python/lldbsuite/test/plugins/builder_base.py

144 ↗

(On Diff #56351)

shouldn't this be buildModules or something?

[tfiala]

Hi Adrian,

I think this has stalled. What do we need to resolve now?

[aprantl]

I still need some expertise on how to disable the configuration on platforms with older versions of clang (or different compilers like gcc).

[tfiala]

Oh right, I recall there was an action item on me.

I'll catch up with you on that in a bit.

[tfiala]

Adrian,

I'm going to take a look at this with you today. I'm reviewing all the comments on this so far.

-Todd

[tfiala]

So at this point, it looks like we're just missing a mechanism to check for gmodules support, and then add it to the supported_categories list if it is available.

I'll have a look a that.

This might be the time where we want to break out an apple-clang vs. llvm.org-clang when we talk about version numbers. We're asking for trouble when we treat both as the same, yet they have drastically different version numbers that will eventually collide as llvm.org version numbers increase.

packages/Python/lldbsuite/test/lldbtest.py

1488 ↗

(On Diff #56351)

I agree with @labath

packages/Python/lldbsuite/test/plugins/builder_base.py

144 ↗

(On Diff #56351)

buildGModules() sounds right here, particularly since it matches the define being passed in. And adjust the comment below to reflect.

[zturner]

Just chiming in to say modules is unsupported on Windows even with clang,
so please make sure the gmodules support function takes this into account

[tfiala]

In D19998#437820, @zturner wrote:

Just chiming in to say modules is unsupported on Windows even with clang,
so please make sure the gmodules support function takes this into account

Okay, thanks Zachary. We'll make sure it doesn't somehow get enabled there.

[tfiala]

gmodules-test-support-v2.diff6 KBDownload

I just uploaded a modified patch. This patch does nothing more than fix invalid behavior in the initial patch, as it had several issues that would cause runtime errors.

With the adjusted patch, which is based against lldb r270570, I see the following failures on OS X 10.11.5 with Xcode 7.3.1, building test inferiors with the in-tree clang (with both compiler-rt and libcxx from top of tree):

FAIL: test_NSString_expr_commands_gmodules (lang/objc/foundation/TestObjCMethods2.py)
FAIL: test_break_gmodules (lang/objc/foundation/TestRuntimeTypes.py)
FAIL: test_c_global_variables_gmodules (lang/c/global_variables/TestGlobalVariables.py)
ERROR: test_top_level_expressions_gmodules (expression_command/top-level/TestTopLevelExprs.py)

As it stands right now, it looks like it will always try to run gmodules. I'll adjust that now per previous comments.

[tfiala]

gmodules-test-support-v3.diff13 KBDownload

The attached patch contains an implementation that properly checks for gmodules support and runs clean on OS X. I filed a few bugs and marked a few tests as failing with gmodules debug info in this patch.

I'm going to give it a run on Linux and see if we have more tests that need to be marked xfail with gmodules support.

@aprantl, can you update the patch sets with the latest patch? It is built against r270593.

[tfiala]

gmodules-test-support-v4.diff14 KBDownload

Patch updated. TestDeadStrip.py required xfailing for Ubuntu with gmodules debug info.

[tfiala]

gmodules-test-support-v5.diff16 KBDownload

This is the final patch for the way I'd like to see this. It breaks out gmodules checking into a separate support module (in lldbsuite/support/gmodules.py). The skipUnless for gmodules now uses this mechanism, and the one previous gmodules-specific test is now set to run only for gmodules debug info.

@aprantl, if you can put up this patch set here, then we're good to get a final pass on the review from others.

[zturner]

In is_compiler_clang_with_gmodules you will need to explicitly return false if the target is Windows, because clang help will still show the command line option as being valid, even though it doesn't work properly.

[tfiala]

In D19998#438529, @zturner wrote:

In is_compiler_clang_with_gmodules you will need to explicitly return false if the target is Windows, because clang help will still show the command line option as being valid, even though it doesn't work properly.

Ah okay.

Two code paths call that, one is guarded against Windows, but the very last change for that one single gmodules-only test uses the decorator that doesn't guard. (I had mis-expected that -gmodules would just be missing entirely from the Windows end).

Adrian is about to put up the patch set. I'll catch him and we'll modify that before it goes up.

[tfiala]

gmodules-test-support-v6.diff16 KBDownload

Updated gmodules.is_compiler_clang_with_gmodules() to guard on Windows with:

def _gmodules_supported_internal():
    compiler = os.path.basename(compiler_path)
    if "clang" not in compiler:
        return False
    elif os.name == "nt":
        # gmodules support is broken on Windows
        return False
    else:
        # Check the compiler help for the -gmodules option.
        clang_help = os.popen("%s --help" % compiler_path).read()
        return GMODULES_HELP_REGEX.search(clang_help, re.DOTALL) is not None

[zturner]

In D19998#438586, @tfiala wrote:

gmodules-test-support-v6.diff16 KBDownload

Updated gmodules.is_compiler_clang_with_gmodules() to guard on Windows with:

def _gmodules_supported_internal():
    compiler = os.path.basename(compiler_path)
    if "clang" not in compiler:
        return False
    elif os.name == "nt":
        # gmodules support is broken on Windows
        return False
    else:
        # Check the compiler help for the -gmodules option.
        clang_help = os.popen("%s --help" % compiler_path).read()
        return GMODULES_HELP_REGEX.search(clang_help, re.DOTALL) is not None

So I asked some of the guys here, and they said modules debug info (in particular -gmodules) will not work anywhere but OSX.

[tfiala]

In D19998#438614, @zturner wrote:

In D19998#438586, @tfiala wrote:

gmodules-test-support-v6.diff16 KBDownload

Updated gmodules.is_compiler_clang_with_gmodules() to guard on Windows with:

def _gmodules_supported_internal():
    compiler = os.path.basename(compiler_path)
    if "clang" not in compiler:
        return False
    elif os.name == "nt":
        # gmodules support is broken on Windows
        return False
    else:
        # Check the compiler help for the -gmodules option.
        clang_help = os.popen("%s --help" % compiler_path).read()
        return GMODULES_HELP_REGEX.search(clang_help, re.DOTALL) is not None

So I asked some of the guys here, and they said modules debug info (in particular -gmodules) will not work anywhere but OSX.

I don't think that's right. I ran all these tests on Linux, and our debug info guy (Adrian Prantl) thinks it is better supported on Linux than OS X. ? I'll let Adrian weigh in on that though. I got all but the one dead code stripping test to run fine on Ubuntu 14.04 x86_64.

[aprantl]

Update with Todds most recent infrastructure additions.

[aprantl]

In D19998#438666, @tfiala wrote:

In D19998#438614, @zturner wrote:

So I asked some of the guys here, and they said modules debug info (in particular -gmodules) will not work anywhere but OSX.

I don't think that's right. I ran all these tests on Linux, and our debug info guy (Adrian Prantl) thinks it is better supported on Linux than OS X. ? I'll let Adrian weigh in on that though. I got all but the one dead code stripping test to run fine on Ubuntu 14.04 x86_64.

Clang modules themselves (-fmodules) can be thought of as almost a build system / preprocessor feature that does not or rather can not have any effect on the debugger at all. Darwin (OS X, iOS, ...) supports clang modules for C and Objective-C very well. C++ clang modules are not supported on Darwin. On Linux clang modules are supported, including C++ modules. I believe I also heard of people using clang modules on FreeBSD. On Windows, clang can build and use modules, but I don't think anyone is using/testing this actively.

Clang module debugging (-fmodules -gmodules) affects the debugger (debug info for module types is emitted only once in the compiled module and referred to by reference in the object files). It is fully supported on Darwin. On Linux, can in theory support it (there may be some unresolved bugs with DWOs), but nobody tested this so far and this patch would make it possible to figure out where we're at. I think tests should be XFAIL'ed on an individual basis and considered bugs if they don't work on Linux.

• adrian

[zturner]

Should this be disabled by default unless explicitly requested? Seems like
"run the entire test suite N times" should be opt in, not opt out.

If its opt in, I don't mind removing the os check for Windows.

[labath]

I think we should make one more iteration on this, as there is still some room for improvement.

In D19998#438756, @zturner wrote:

Should this be disabled by default unless explicitly requested? Seems like
"run the entire test suite N times" should be opt in, not opt out.

I think it makes sense to run multiple variants of some tests, as "gmodules" is a regular debugger feature and we want to make sure we support that feature. What I think is a problem here is the "whole test suite" part, as I think a lot of tests (think, all process control and thread tests) don't (or shouldn't) depend on debug info (see discussion early on in this thread). My feeling is that we should start restricting the duplication more aggressively, and then it won't matter that some tests get run multiple times.

That said, if you as the windows maintainer say that you don't want to support gmodules debugging on windows at the moment, then I think it's fine to skip those tests on this platform.

packages/Python/lldbsuite/support/gmodules.py

19 ↗

(On Diff #58369)

This diff looks broken. This appears to be a new file (I certainly don't have it in my tree), and it is being shown as a diff. Could you reupload a correct diff against the current top of tree?

22 ↗

(On Diff #58369)

This check should be folded into test_categories.is_supported_on_platform. (Also, it is incorrect as it should presumably be checking against the target platform and not the host.)

packages/Python/lldbsuite/test/decorators.py

527 ↗

(On Diff #58369)

This whole decorator can be removed. It's invocations can be replaced with add_test_categories(["gmodules"]) (cf. lldbtest.py:1453, it will avoid duplicating the test if it is already annotated with categories).

packages/Python/lldbsuite/test/lang/cpp/gmodules/TestWithModuleDebugging.py

13 ↗

(On Diff #58369)

replace these two decorators with add_test_categories(["gmodules"])

packages/Python/lldbsuite/test/lldbinline.py

148 ↗

(On Diff #58369)

buildGModules() ?

[tfiala]

@aprantl, the lldbsuite/support/gmodules.py file didn't make it into your patch set here. (It was the top file in the -v6 diff).

I'm going to adjust per comments above. @labath, see question on add_test_categories.

I may end up filing this as a separate review since I'm WFH and we can probably iterate faster without getting @aprantl to have to keep putting up patch sets for me. (I didn't see a way for phabricator to allow multiple people to put up a diff set on the same review - if that existed I'd use that).

@zturner, with the changes @labath suggested, that one decorator that was used by that one test will go away, and then the is_supported-style check for gmodules (as currently written) will not permit gmodules on Windows. It is set to support macosx, ios, linux and freebsd.

packages/Python/lldbsuite/support/gmodules.py

19 ↗

(On Diff #58369)

Yes it is missing from the patch set. It is in the -v6.diff file I attached earlier (top entry in the diff). We'll get this updated.

22 ↗

(On Diff #58369)

It actually is in is_supported_on_platform (by virtue of windows not being included).

I had misunderstood your earlier comment on how add_categories worked. I did not know that I could make a debuginfo-specific test work by adding the category. That makes sense now, but I had kept the other decorator around only because I didn't see this as an option.

I get it now. Good idea.

packages/Python/lldbsuite/test/decorators.py

527 ↗

(On Diff #58369)

Yes, I see now. I did not understand add_test_categories(["gmodules"]) was a valid way to mark a test as gmodules-only.

It does look a little weird that we need to do:
@no_debug_info_test
@add_test_categories(["gmodules"])

I wonder if we want a combined decorator that is effectively:
@valid_debug_info([...])

that specifies that this test is only valid for those debug info entries specified. Then the entry becomes a single decorator:
@valid_debug_info(["gmodules"])

What do you think?

packages/Python/lldbsuite/test/lang/cpp/gmodules/TestWithModuleDebugging.py

13 ↗

(On Diff #58369)

Yup.

Well we would need @no_debug_info_test still, wouldn't we? Otherwise we'll fan out to all the debug info variants? (Or is add_test_categories() replace all test categories? I assumed it was additive since it starts with "add_", in which case I'd expect we'd still have all the normal debug info entries show up).

packages/Python/lldbsuite/test/lldbinline.py

148 ↗

(On Diff #58369)

Yeah that's wrong. Good catch.

That also means the testing I did on OS X and Linux failed to do real gmodule debugging for inline tests. I'll need to rerun.

[aprantl]

Todd, I'm abandoning the review. This should allow you to claim it as your own so you can iterate quicker. Thanks!

[tfiala]

Commandeering.

Thanks, Adrian!

[tfiala]

@aprantl, the lldbsuite/support/gmodules.py file didn't make it into your patch set here. (It was the top file in the -v6 diff).

Totally incorrect. The top file is the new file, but it is malformed and is showing a diff rather than the whole file. Not sure exactly how I produced that diff, but that is something I'm about to rectify. I'll put up a fresh patch set after I address the issues in the review. (It may take a while as I will also be testing with adjustments for the inline tests, that did not use the gmodules build steps --- regular tests did, just not inline ones, which we happen to have a lot of).

[tfiala]

Just updating the patch set to what I had intended to give Adrian in r6. This does not yet contain any of the suggestions from the latest round.

[labath]

packages/Python/lldbsuite/test/decorators.py

527 ↗

(On Diff #58369)

You shouldn't need both decorators. The test multiplicator will first check whether whether the test is already explicitly annotated with some debug info category (lldbtest.py:1446) and multiply only into the categories that were not annotated.

Now that I look at it closer, I see that the code there is not entirely correct in case you annotate a test with two categories (it will create two tests, but both of them will be annotated with both categories). However, this is a topic for another change, and should not prevent you from using it now.

So, yes, you should only need one decorator, and things should just work. As for the current syntax, I am open to making any changes to it, but I want to avoid having two ways of doing this in parallel.

[tfiala]

Yep, I see how it works now.

I'll do a quick follow-up review after this to get that other issue with the debug info multiplicator (you may have just coined a word ;-)) fixed since I'm already in this code.

Just finishing testing it on OS X.

[tfiala]

I also stuck a print in to verify the gmodules build was being run everywhere I expected (i.e. for normal tests, inline tests, and the gmodules-explicit tests).

[tfiala]

Adjustments per review, removal of decorator, fixup of
incorrect build method call in inline tests.

[tfiala]

Remove the now-unneeded second check for Windows.

[tfiala]

I believe this now accounts for everything we discussed changing.

[tfiala]

Marked all code comments as done.

[zturner]

Going to try this out on Windows and see how it goes.

[tfiala]

Okay, thanks Zachary.

[tfiala]

Timing info for Ubuntu 15.10:

Description	Time (seconds)(mm:ss)
Ubuntu 15.10 6-core, 24 GB RAM, TOT clang, no gmodules	377s (6:17)
Ubuntu 15.10 6-core, 24 GB RAM, TOT clang, with gmodules	517s (8:37)

Net increase: 37%.

No new failures with Ubuntu 15.10 x86_64 with the inline test fix.

[zturner]

No problems here, I'd wait for Pavel for additional thoughts before going in.

[tfiala]

In D19998#439698, @zturner wrote:

No problems here, I'd wait for Pavel for additional thoughts before going in.

Yep that's fine. I am in no hurry to get this in.

[tfiala]

Updated timings, now with OS X:

Configuration	Time (seconds)(mm:ss)
Ubuntu 15.10 6-core, 24 GB RAM, TOT clang, no gmodules	377 (6:17)
Ubuntu 15.10 6-core, 24 GB RAM, TOT clang, with gmodules	517 (8:37)
OS X 11.5 8-core, 32 GB RAM, Xcode 7.3.1 clang, no gmodules	361 (6:01)
OS X 11.5 8-core, 32 GB RAM, Xcode 7.3.1 clang, with gmodules	500 (8:20)

Configuration	Net increase (%)
Ubuntu 15.10 6-core, 24 GB RAM, TOT clang	37
OS X 11.5 8-core, 32 GB RAM, Xcode 7.3.1 clang	39

[labath]

Looks great now. Thanks.

packages/Python/lldbsuite/test/test_categories.py

62 ↗

(On Diff #58459)

So, one way to achieve that would be to move the logic that decides whether to run these tests to a later stage -- to check the conditions during test running, not test generation. This would make this behave the same way as all the other skip decorators -- the test would be skipped instead of "not generated at all".

But I don't think we should do this now.

lldb/trunk/packages/Python/lldbsuite/support/gmodules.py

+from __future__ import absolute_import
+from __future__ import print_function
+
+# System modules
+import os
+import re
+
+
+GMODULES_SUPPORT_MAP = {}
+GMODULES_HELP_REGEX = re.compile(r"\s-gmodules\s")
+
+
+def is_compiler_clang_with_gmodules(compiler_path):
+    # Before computing the result, check if we already have it cached.
+    if compiler_path in GMODULES_SUPPORT_MAP:
+        return GMODULES_SUPPORT_MAP[compiler_path]
+
+    def _gmodules_supported_internal():
+        compiler = os.path.basename(compiler_path)
+        if "clang" not in compiler:
+            return False
+        else:
+            # Check the compiler help for the -gmodules option.
+            clang_help = os.popen("%s --help" % compiler_path).read()
+            return GMODULES_HELP_REGEX.search(clang_help, re.DOTALL) is not None
+
+    GMODULES_SUPPORT_MAP[compiler_path] = _gmodules_supported_internal()
+    return GMODULES_SUPPORT_MAP[compiler_path]
+
+

lldb/trunk/packages/Python/lldbsuite/test/decorators.py

def is_compiler_clang_with_thread_sanitizer(self):
         cmd = "echo 'int main() {}' | %s -x c -o %s -" % (compiler_path, f.name)
         if os.popen(cmd).close() != None:
             return None  # The compiler cannot compile at all, let's *not* skip the test
         cmd = "echo 'int main() {}' | %s -fsanitize=thread -x c -o %s -" % (compiler_path, f.name)
         if os.popen(cmd).close() != None:
             return "Compiler cannot compile with -fsanitize=thread"
         return None
     return skipTestIfFn(is_compiler_clang_with_thread_sanitizer)(func)
-
-def skipUnlessClangModules():
-    """Decorate the item to skip test unless Clang -gmodules flag is supported."""
-    def is_compiler_clang_with_gmodules(self):
-        compiler_path = self.getCompiler()
-        compiler = os.path.basename(compiler_path)
-        if compiler != "clang":
-            return "Test requires clang as compiler"
-        clang_help = os.popen("%s --help" % (compiler_path)).read()
-        match = re.match(".* -gmodules ", clang_help, re.DOTALL)
-        return "Clang version doesn't support -gmodules flag" if not match else None
-    return skipTestIfFn(is_compiler_clang_with_gmodules)

lldb/trunk/packages/Python/lldbsuite/test/expression_command/top-level/TestTopLevelExprs.py

def run_dummy(self):
 
         lldbutil.run_break_set_by_file_and_line (self, "dummy.cpp", self.dummy_line, num_expected_locations=1, loc_exact=False)
 
         self.runCmd("run", RUN_SUCCEEDED)
 
     @add_test_categories(['pyapi'])
     @expectedFailureAndroid(api_levels=[21, 22], bugnumber="llvm.org/pr27787")
     @expectedFailureAll(oslist=["linux"], archs=["arm", "aarch64"], bugnumber="llvm.org/pr27787")
+    @expectedFailureAll(oslist=["macosx"], debug_info="gmodules", bugnumber="llvm.org/pr27864")
     @skipIf(oslist=["windows"]) # Error in record layout on Windows
     def test_top_level_expressions(self):
         self.build_and_run()
 
         resultFromCode = self.frame().EvaluateExpression("doTest()").GetValueAsUnsigned()
 
         self.runCmd("kill")
 

lldb/trunk/packages/Python/lldbsuite/test/functionalities/dead-strip/TestDeadStrip.py

 from lldbsuite.test import lldbutil
 
 class DeadStripTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24778")
     @expectedFailureAll(debug_info="dwo", bugnumber="llvm.org/pr25087")
+    @expectedFailureAll(oslist=["linux"], debug_info="gmodules", bugnumber="llvm.org/pr27865")
     @skipIfFreeBSD # The -dead_strip linker option isn't supported on FreeBSD versions of ld.
     def test(self):
         """Test breakpoint works correctly with dead-code stripping."""
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Break by function name f1 (live code).

lldb/trunk/packages/Python/lldbsuite/test/lang/c/global_variables/TestGlobalVariables.py

def setUp(self):
         # Call super's setUp().
         TestBase.setUp(self)
         # Find the line number to break inside main().
         self.source = 'main.c'
         self.line = line_number(self.source, '// Set break point at this line.')
         self.shlib_names = ["a"]
 
     @expectedFailureAll(oslist=["windows"], bugnumber="llvm.org/pr24764")
-    @expectedFailureAll("llvm.org/pr25872", oslist=["macosx"], debug_info="dwarf")
+    @expectedFailureAll("llvm.org/pr25872", oslist=["macosx"], debug_info=["dwarf", "gmodules"])
     def test_c_global_variables(self):
         """Test 'frame variable --scope --no-args' which omits args and shows scopes."""
         self.build()
 
         # Create a target by the debugger.
         target = self.dbg.CreateTarget("a.out")
         self.assertTrue(target, VALID_TARGET)
 

lldb/trunk/packages/Python/lldbsuite/test/lang/cpp/gmodules/TestWithModuleDebugging.py

 import lldb, os
 from lldbsuite.test.decorators import *
 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 class TestWithGmodulesDebugInfo(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
+    @add_test_categories(["gmodules"])
     @expectedFailureAll(bugnumber="llvm.org/pr27412")
-    @skipUnlessClangModules()
     def test_specialized_typedef_from_pch(self):
-        clang_help = os.popen("clang --help").read()
-        match = re.match(".* -gmodules ", clang_help, re.DOTALL)
-        if not match:
-            self.skipTest("Clang version doesn't support -gmodules flag")
-            return
-
         self.build()
         cwd = os.getcwd()
 
         src_file = os.path.join(cwd, "main.cpp")
         src_file_spec = lldb.SBFileSpec(src_file)
         self.assertTrue(src_file_spec.IsValid(), "breakpoint file")
 
         # Get the path of the executable

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestObjCMethods2.py

def test_NSArray_expr_commands(self):
         self.expect("expression (int)[array2 count]",
             patterns = ["\(int\) \$.* = 3"])
         self.expect("expression (int)array1.count",
             patterns = ["\(int\) \$.* = 3"])
         self.expect("expression (int)array2.count",
             patterns = ["\(int\) \$.* = 3"])
         self.runCmd("process continue")
 
+    @expectedFailureAll(oslist=["macosx"], debug_info="gmodules", bugnumber="llvm.org/pr27861")
     def test_NSString_expr_commands(self):
         """Test expression commands for NSString."""
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
 
         # Break inside Test_NSString:
         line = self.lines[2]

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/foundation/TestRuntimeTypes.py

 from lldbsuite.test.lldbtest import *
 from lldbsuite.test import lldbutil
 
 @skipUnlessDarwin
 class RuntimeTypesTestCase(TestBase):
 
     mydir = TestBase.compute_mydir(__file__)
 
+    @expectedFailureAll(oslist=["macosx"], debug_info="gmodules", bugnumber="llvm.org/pr27862")
     def test_break(self):
         """Test setting objc breakpoints using '_regexp-break' and 'breakpoint set'."""
         if self.getArchitecture() != 'x86_64':
             self.skipTest("This only applies to the v2 runtime")
 
         self.build()
         exe = os.path.join(os.getcwd(), "a.out")
         self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)

lldb/trunk/packages/Python/lldbsuite/test/lldbinline.py

 from __future__ import print_function
 from __future__ import absolute_import
 
 # System modules
 import os
 
 # Third-party modules
 
 # LLDB modules
 import lldb
 from .lldbtest import *
+from . import configuration
 from . import lldbutil
 from .decorators import *
 
 def source_type(filename):
     _, extension = os.path.splitext(filename)
     return {
         '.c' : 'C_SOURCES',
         '.cpp' : 'CXX_SOURCES',
def __test_with_dwarf(self):
         self.do_test()
 
     def __test_with_dwo(self):
         self.using_dsym = False
         self.BuildMakefile()
         self.buildDwo()
         self.do_test()
 
+    def __test_with_gmodules(self):
+        self.using_dsym = False
+        self.BuildMakefile()
+        self.buildGModules()
+        self.do_test()
+
     def execute_user_command(self, __command):
         exec(__command, globals(), locals())
 
     def do_test(self):
         exe_name = "a.out"
         exe = os.path.join(os.getcwd(), exe_name)
         source_files = [ f for f in os.listdir(os.getcwd()) if source_type(f) ]
         target = self.dbg.CreateTarget(exe)
def MakeInlineTest(__file, __globals, decorators=None):
     InlineTest.mydir = TestBase.compute_mydir(__file)
 
     test_name, _ = os.path.splitext(file_basename)
     # Build the test case
     test = type(test_name, (InlineTest,), {'using_dsym': None})
     test.name = test_name
 
     target_platform = lldb.DBG.GetSelectedPlatform().GetTriple().split('-')[2]
-    if test_categories.is_supported_on_platform("dsym", target_platform):
+    if test_categories.is_supported_on_platform("dsym", target_platform, configuration.compilers):
         test.test_with_dsym = ApplyDecoratorsToFunction(test._InlineTest__test_with_dsym, decorators)
-    if test_categories.is_supported_on_platform("dwarf", target_platform):
+    if test_categories.is_supported_on_platform("dwarf", target_platform, configuration.compilers):
         test.test_with_dwarf = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwarf, decorators)
-    if test_categories.is_supported_on_platform("dwo", target_platform):
+    if test_categories.is_supported_on_platform("dwo", target_platform, configuration.compilers):
         test.test_with_dwo = ApplyDecoratorsToFunction(test._InlineTest__test_with_dwo, decorators)
+    if test_categories.is_supported_on_platform("gmodules", target_platform, configuration.compilers):
+        test.test_with_gmodules = ApplyDecoratorsToFunction(test._InlineTest__test_with_gmodules, decorators)
 
     # Add the test case to the globals, and hide InlineTest
     __globals.update({test_name : test})
 
     # Keep track of the original test filename so we report it
     # correctly in test results.
     test.test_filename = __file
     return test
 

lldb/trunk/packages/Python/lldbsuite/test/lldbtest.py

class Base(unittest2.TestCase):
 
     def buildDwo(self, architecture=None, compiler=None, dictionary=None, clean=True):
         """Platform specific way to build binaries with dwarf maps."""
         module = builder_module()
         dictionary = lldbplatformutil.finalize_build_dictionary(dictionary)
         if not module.buildDwo(self, architecture, compiler, dictionary, clean):
             raise Exception("Don't know how to build binary with dwo")
 
+    def buildGModules(self, architecture=None, compiler=None, dictionary=None, clean=True):
+        """Platform specific way to build binaries with gmodules info."""
+        module = builder_module()
+        if not module.buildGModules(self, architecture, compiler, dictionary, clean):
+            raise Exception("Don't know how to build binary with gmodules")
+
     def buildGo(self):
         """Build the default go binary.
         """
         system([[which('go'), 'build -gcflags "-N -l" -o a.out main.go']])
 
     def signBinary(self, binary_path):
         if sys.platform.startswith("darwin"):
             codesign_cmd = "codesign --force --sign lldb_codesign %s" % (binary_path)
def __new__(cls, name, bases, attrs):
 
                 # If any debug info categories were explicitly tagged, assume that list to be
                 # authoritative.  If none were specified, try with all debug info formats.
                 all_dbginfo_categories = set(test_categories.debug_info_categories)
                 categories = set(getattr(attrvalue, "categories", [])) & all_dbginfo_categories
                 if not categories:
                     categories = all_dbginfo_categories
 
-                supported_categories = [x for x in categories 
-                                        if test_categories.is_supported_on_platform(x, target_platform)]
+                supported_categories = [x for x in categories
+                                        if test_categories.is_supported_on_platform(
+                                            x, target_platform, configuration.compilers)]
                 if "dsym" in supported_categories:
                     @decorators.add_test_categories(["dsym"])
                     @wraps(attrvalue)
                     def dsym_test_method(self, attrvalue=attrvalue):
                         self.debug_info = "dsym"
                         return attrvalue(self)
                     dsym_method_name = attrname + "_dsym"
                     dsym_test_method.__name__ = dsym_method_name
def __new__(cls, name, bases, attrs):
                     @decorators.add_test_categories(["dwo"])
                     @wraps(attrvalue)
                     def dwo_test_method(self, attrvalue=attrvalue):
                         self.debug_info = "dwo"
                         return attrvalue(self)
                     dwo_method_name = attrname + "_dwo"
                     dwo_test_method.__name__ = dwo_method_name
                     newattrs[dwo_method_name] = dwo_test_method
+
+                if "gmodules" in supported_categories:
+                    @decorators.add_test_categories(["gmodules"])
+                    @wraps(attrvalue)
+                    def gmodules_test_method(self, attrvalue=attrvalue):
+                        self.debug_info = "gmodules"
+                        return attrvalue(self)
+                    gmodules_method_name = attrname + "_gmodules"
+                    gmodules_test_method.__name__ = gmodules_method_name
+                    newattrs[gmodules_method_name] = gmodules_test_method
+
             else:
                 newattrs[attrname] = attrvalue
         return super(LLDBTestCaseFactory, cls).__new__(cls, name, bases, newattrs)
 
 # Setup the metaclass for this class to change the list of the test methods when a new class is loaded
 @add_metaclass(LLDBTestCaseFactory)
 class TestBase(Base):
     """
def build(self, architecture=None, compiler=None, dictionary=None, clean=True):
         if self.debug_info is None:
             return self.buildDefault(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dsym":
             return self.buildDsym(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dwarf":
             return self.buildDwarf(architecture, compiler, dictionary, clean)
         elif self.debug_info == "dwo":
             return self.buildDwo(architecture, compiler, dictionary, clean)
+        elif self.debug_info == "gmodules":
+            return self.buildGModules(architecture, compiler, dictionary, clean)
         else:
             self.fail("Can't build for debug info: %s" % self.debug_info)
 
     def run_platform_command(self, cmd):
         platform = self.dbg.GetSelectedPlatform()
         shell_command = lldb.SBPlatformShellCommand(cmd)
         err = platform.Run(shell_command)
         return (err, shell_command.GetStatus(), shell_command.GetOutput())

lldb/trunk/packages/Python/lldbsuite/test/make/Makefile.rules

 else
   CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(CFLAGS_EXTRAS) $(TRIPLE_CFLAGS)
 endif
 
 ifeq "$(MAKE_DWO)" "YES"
   CFLAGS += -gsplit-dwarf
 endif
 
+ifeq "$(MAKE_GMODULES)" "YES"
+  CFLAGS += -fmodules -gmodules
+endif
+
 CXXFLAGS += -std=c++11
-CXXFLAGS += $(CFLAGS)
+# FIXME: C++ modules aren't supported on all platforms.
+CXXFLAGS += $(subst -fmodules,, $(CFLAGS))
 LD = $(CC)
 LDFLAGS ?= $(CFLAGS)
 LDFLAGS += $(LD_EXTRAS)
 ifeq (,$(filter $(OS), Windows_NT Android))
   ifneq (,$(filter YES,$(ENABLE_THREADS)))
     LDFLAGS += -pthread
   endif
 endif

 #----------------------------------------------------------------------
 # Make the precompiled header and compile C++ sources against it
 #----------------------------------------------------------------------
 
 #ifneq "$(PCH_OUTPUT)" ""
 $(PCH_OUTPUT) : $(PCH_CXX_SOURCE)
   $(CXX) $(CXXFLAGS) -x c++-header -o $(PCH_OUTPUT) $(PCH_CXX_SOURCE)
 %.o : %.cpp $(PCH_OUTPUT)
-  $(CXX) $(PCHFLAGS) $(CXXFLAGS) $(CFLAGS) -c -o $@ $<
+  $(CXX) $(PCHFLAGS) $(CXXFLAGS) -c -o $@ $<
 #endif
 
 #----------------------------------------------------------------------
 # Automatic variables based on items already entered. Below we create
 # an object's lists from the list of sources by replacing all entries
 # that end with .c with .o, and we also create a list of prerequisite
 # files by replacing all .c files with .d.
 #----------------------------------------------------------------------

lldb/trunk/packages/Python/lldbsuite/test/plugins/builder_base.py

def buildDwo(sender=None, architecture=None, compiler=None, dictionary=None, clean=True):
     if clean:
         commands.append([getMake(), "clean", getCmdLine(dictionary)])
     commands.append([getMake(), "MAKE_DSYM=NO", "MAKE_DWO=YES", getArchSpec(architecture), getCCSpec(compiler), getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle building dwo.
     return True
 
+def buildGModules(sender=None, architecture=None, compiler=None, dictionary=None, clean=True):
+    """Build the binaries with dwarf debug info."""
+    commands = []
+    if clean:
+        commands.append([getMake(), "clean", getCmdLine(dictionary)])
+    commands.append([getMake(), "MAKE_DSYM=NO", "MAKE_GMODULES=YES", getArchSpec(architecture), getCCSpec(compiler), getCmdLine(dictionary)])
+
+    lldbtest.system(commands, sender=sender)
+    # True signifies that we can handle building with gmodules.
+    return True
+
 def cleanup(sender=None, dictionary=None):
     """Perform a platform-specific cleanup after the test."""
     #import traceback
     #traceback.print_stack()
     commands = []
     if os.path.isfile("Makefile"):
         commands.append([getMake(), "clean", getCmdLine(dictionary)])
 
     runBuildCommands(commands, sender=sender)
     # True signifies that we can handle cleanup.
     return True

lldb/trunk/packages/Python/lldbsuite/test/test_categories.py

 """
 Provides definitions for various lldb test categories
 """
 
 from __future__ import absolute_import
 from __future__ import print_function
 
 # System modules
 import sys
 
 # Third-party modules
 
 # LLDB modules
+from lldbsuite.support import gmodules
+
 
 debug_info_categories = [
-    'dwarf', 'dwo', 'dsym'
+    'dwarf', 'dwo', 'dsym', 'gmodules'
 ]
 
 all_categories = {
     'dataformatters': 'Tests related to the type command and the data formatters subsystem',
     'dwarf'         : 'Tests that can be run with DWARF debug information',
     'dwo'           : 'Tests that can be run with DWO debug information',
     'dsym'          : 'Tests that can be run with DSYM debug information',
+    'gmodules'      : 'Tests that can be run with -gmodules debug information',
     'expression'    : 'Tests related to the expression parser',
     'objc'          : 'Tests related to the Objective-C programming language support',
     'pyapi'         : 'Tests related to the Python API',
     'basic_process' : 'Basic process execution sniff tests.',
     'cmdline'       : 'Tests related to the LLDB command-line interface',
     'dyntype'       : 'Tests related to dynamic type support',
     'stresstest'    : 'Tests related to stressing lldb limits',
     'flakey'        : 'Flakey test cases, i.e. tests that do not reliably pass at each execution',
     'lldb-mi'       : 'lldb-mi tests'
 }
 
 def unique_string_match(yourentry, list):
     candidate = None
     for item in list:
         if not item.startswith(yourentry):
             continue
         if candidate:
             return None
         candidate = item
     return candidate
 
-def is_supported_on_platform(category, platform):
+
+def is_supported_on_platform(category, platform, compiler_paths):
     if category == "dwo":
         # -gsplit-dwarf is not implemented by clang on Windows.
         return platform in ["linux", "freebsd"]
     elif category == "dsym":
         return platform in ["darwin", "macosx", "ios"]
+    elif category == "gmodules":
+        # First, check to see if the platform can even support gmodules.
+        if platform not in ["linux", "freebsd", "darwin", "macosx", "ios"]:
+            return False
+        # If all compilers specified support gmodules, we'll enable it.
+        for compiler_path in compiler_paths:
+            if not gmodules.is_compiler_clang_with_gmodules(compiler_path):
+                # Ideally in a multi-compiler scenario during a single test run, this would
+                # allow gmodules on compilers that support it and not on ones that don't.
+                # However, I didn't see an easy way for all the callers of this to know
+                # the compiler being used for a test invocation.  As we tend to run with
+                # a single compiler per test run, this shouldn't be a major issue.
+                return False
+        return True
     return True
 
 def validate(categories, exact_match):
     """
     For each category in categories, ensure that it's a valid category (if exact_match is false,
     unique prefixes are also accepted). If a category is invalid, print a message and quit.
        If all categories are valid, return the list of categories. Prefixes are expanded in the
        returned list.