This is an archive of the discontinued LLVM Phabricator instance.

Differential D42277

Use test-specific module caches to avoid stale header conflicts
ClosedPublic

Authored by vsk on Jan 18 2018, 7:03 PM.

Details

Reviewers
aprantl
davide
jasonmolenda
labath
Commits
rG801362f067ae: Use test-specific module caches to avoid stale header conflicts
rLLDB323450: Use test-specific module caches to avoid stale header conflicts
rL323450: Use test-specific module caches to avoid stale header conflicts
Summary

Stale global module caches cause problems for the bots. The modules
become invalid when clang headers are updated by version control, and
tests which use these modules fail to compile, e.g:

fatal error: file '.../__stddef_max_align_t.h' has been modified since the module file '/var/.../Darwin.pcm' was built
note: please rebuild precompiled header '/var/.../Darwin.pcm'

Eventually we should transition to having just a single module cache to speed
tests up. This patch should be just enough to fix the spurious bot failures due
to stale caches.

rdar://36479805

Diff Detail

Repository
rL LLVM

Event Timeline

vsk created this revision.Jan 18 2018, 7:03 PM
Herald added a subscriber: eraman. · View Herald TranscriptJan 18 2018, 7:03 PM
aprantl added a comment.Jan 18 2018, 7:36 PM

Thanks, this is a great start!

packages/Python/lldbsuite/test/make/Makefile.rules
240 ↗(On Diff #130538)

Is it safe that this is a relative path?

242 ↗(On Diff #130538)

Should we also specify -fcxxmodules here? Or will that break some bots with older clangs?

vsk updated this revision to Diff 130566.Jan 19 2018, 12:56 AM
  • Upload a diff with context.
packages/Python/lldbsuite/test/make/Makefile.rules
240 ↗(On Diff #130538)

I admit it's not great, but it's at least in keeping with the other targets we define in this Makefile (like a.out, the dsym, etc). As I'm writing this, I've realized I did not upload a diff with context, so this might not have been apparent!

242 ↗(On Diff #130538)

I've tried to avoid changing the flags used by each Makefile when possible. The only exception is my removal of "$(subst -fmodules,, $(CFLAGS))" in CXXFLAGS, which I needed to do because the substitution would make us pass "-cache-path=foo".

We might consider adding in -fcxxmodules as a followup (maybe just in CXXFLAGS?).

labath added a comment.Jan 19 2018, 2:59 AM

I don't know much about clang modules, but the change seems reasonable to me.

vsk updated this revision to Diff 130686.Jan 19 2018, 1:51 PM

Per an offline comment by Adrian, include -gmodules in the mandatory set of module flags.

vsk updated this revision to Diff 131000.Jan 22 2018, 7:26 PM
aprantl added a comment.Jan 23 2018, 6:31 AM

Skip tests which fail when -fmodules is passed (https://bugs.llvm.org/show_bug.cgi?id=36048).

Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Why is this necessary?

vsk added a comment.Jan 23 2018, 10:51 AM
In D42277#985002, @aprantl wrote:

Skip tests which fail when -fmodules is passed (https://bugs.llvm.org/show_bug.cgi?id=36048).

Wait.. this patch is not supposed to change the set of tests that get -fmodules passed to them. It should only add -fmodules-cache-path to tests that do pass -fmodules. Why is this necessary?

It's not necessary, but we should make this change. It falls out of removing this FIXME:

# FIXME: C++ modules aren't supported on all platforms.
CXXFLAGS += $(subst -fmodules,, $(CFLAGS))

If we keep it around, we'd need an extra hack to strip out "-fmodules-cache-path=module-cache" as well. Without that we'd pass "-cache-path=module-cache", which clang would reject.

Another benefit of making this change is that it lets us know which tests are actually failing with modules enabled, instead of hiding them.

aprantl added a comment.Jan 23 2018, 11:18 AM

Okay got it. I'll investigate the PR separately.

vsk added a comment.Jan 24 2018, 7:09 PM

Is this OK to commit?

aprantl accepted this revision.Jan 25 2018, 9:17 AM
This revision is now accepted and ready to land.Jan 25 2018, 9:17 AM
Closed by commit rL323450: Use test-specific module caches to avoid stale header conflicts (authored by vedantk). · Explain WhyJan 25 2018, 10:02 AM
This revision was automatically updated to reflect the committed changes.
Herald added a subscriber: llvm-commits. · View Herald TranscriptJan 25 2018, 10:02 AM

Revision Contents

PathSize
lldb/
trunk/
packages/
Python/
lldbsuite/
test/
functionalities/
data-formatter/
data-formatter-cpp/
2 lines
data-formatter-globals/
3 lines
data-formatter-stl/
libcxx/
list/
2 lines
vector/
2 lines
lang/
objc/
modules-auto-import/
2 lines
2 lines
modules-incomplete/
2 lines
2 lines
modules-inline-functions/
2 lines
2 lines
make/
12 lines

Diff 131475

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-cpp/TestDataFormatterCpp.py

Show All 20 Lines def setUp(self):
# Call super's setUp(). # Call super's setUp().
TestBase.setUp(self) TestBase.setUp(self)
# Find the line number to break at. # Find the line number to break at.
self.line = line_number('main.cpp', '// Set break point at this line.') self.line = line_number('main.cpp', '// Set break point at this line.')
@expectedFailureAll( @expectedFailureAll(
oslist=["windows"], oslist=["windows"],
bugnumber="llvm.org/pr24462: Data formatters have problems on Windows") bugnumber="llvm.org/pr24462: Data formatters have problems on Windows")
@skipIf(debug_info="gmodules",
bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
def test_with_run_command(self): def test_with_run_command(self):
"""Test that that file and class static variables display correctly.""" """Test that that file and class static variables display correctly."""
self.build() self.build()
self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True) self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
▲ Show 20 Lines • Show All 262 Lines • Show Last 20 Lines

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-globals/TestDataFormatterGlobals.py

""" """
Test lldb data formatter subsystem. Test lldb data formatter subsystem.
""" """
from __future__ import print_function from __future__ import print_function
import os import os
import time import time
import lldb import lldb
from lldbsuite.test.lldbtest import * from lldbsuite.test.lldbtest import *
from lldbsuite.test.decorators import *
import lldbsuite.test.lldbutil as lldbutil import lldbsuite.test.lldbutil as lldbutil
class GlobalsDataFormatterTestCase(TestBase): class GlobalsDataFormatterTestCase(TestBase):
mydir = TestBase.compute_mydir(__file__) mydir = TestBase.compute_mydir(__file__)
def setUp(self): def setUp(self):
# Call super's setUp(). # Call super's setUp().
TestBase.setUp(self) TestBase.setUp(self)
# Find the line number to break at. # Find the line number to break at.
self.line = line_number('main.cpp', '// Set break point at this line.') self.line = line_number('main.cpp', '// Set break point at this line.')
@skipIf(debug_info="gmodules",
bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
def test_with_run_command(self): def test_with_run_command(self):
"""Test that that file and class static variables display correctly.""" """Test that that file and class static variables display correctly."""
self.build() self.build()
self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True) self, "main.cpp", self.line, num_expected_locations=1, loc_exact=True)
Show All 39 Lines

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/list/TestDataFormatterLibcxxList.py

Show All 25 Lines def setUp(self):
self.line2 = line_number('main.cpp', self.line2 = line_number('main.cpp',
'// Set second break point at this line.') '// Set second break point at this line.')
self.line3 = line_number('main.cpp', self.line3 = line_number('main.cpp',
'// Set third break point at this line.') '// Set third break point at this line.')
self.line4 = line_number('main.cpp', self.line4 = line_number('main.cpp',
'// Set fourth break point at this line.') '// Set fourth break point at this line.')
@add_test_categories(["libc++"]) @add_test_categories(["libc++"])
@skipIf(debug_info="gmodules",
bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
def test_with_run_command(self): def test_with_run_command(self):
"""Test that that file and class static variables display correctly.""" """Test that that file and class static variables display correctly."""
self.build() self.build()
self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.cpp", self.line, num_expected_locations=-1) self, "main.cpp", self.line, num_expected_locations=-1)
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
▲ Show 20 Lines • Show All 174 Lines • Show Last 20 Lines

lldb/trunk/packages/Python/lldbsuite/test/functionalities/data-formatter/data-formatter-stl/libcxx/vector/TestDataFormatterLibcxxVector.py

Show All 12 Lines
from lldbsuite.test import lldbutil from lldbsuite.test import lldbutil
class LibcxxVectorDataFormatterTestCase(TestBase): class LibcxxVectorDataFormatterTestCase(TestBase):
mydir = TestBase.compute_mydir(__file__) mydir = TestBase.compute_mydir(__file__)
@add_test_categories(["libc++"]) @add_test_categories(["libc++"])
@skipIf(debug_info="gmodules",
bugnumber="https://bugs.llvm.org/show_bug.cgi?id=36048")
def test_with_run_command(self): def test_with_run_command(self):
"""Test that that file and class static variables display correctly.""" """Test that that file and class static variables display correctly."""
self.build() self.build()
self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET) self.runCmd("file " + self.getBuildArtifact("a.out"), CURRENT_EXECUTABLE_SET)
bkpt = self.target().FindBreakpointByID( bkpt = self.target().FindBreakpointByID(
lldbutil.run_break_set_by_source_regexp( lldbutil.run_break_set_by_source_regexp(
self, "break here")) self, "break here"))
▲ Show 20 Lines • Show All 157 Lines • Show Last 20 Lines

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-auto-import/Makefile

LEVEL = ../../../make LEVEL = ../../../make
OBJC_SOURCES := main.m OBJC_SOURCES := main.m
CFLAGS += -fmodules -gmodules -g CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
include $(LEVEL)/Makefile.rules include $(LEVEL)/Makefile.rules

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-auto-import/TestModulesAutoImport.py

Show All 20 Linesclass ObjCModulesAutoImportTestCase(TestBase):
def setUp(self): def setUp(self):
# Call super's setUp(). # Call super's setUp().
TestBase.setUp(self) TestBase.setUp(self)
# Find the line number to break inside main(). # Find the line number to break inside main().
self.line = line_number('main.m', '// Set breakpoint 0 here.') self.line = line_number('main.m', '// Set breakpoint 0 here.')
@skipUnlessDarwin @skipUnlessDarwin
@skipIf(macos_version=["<", "10.12"]) @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self): def test_expr(self):
self.build() self.build()
exe = self.getBuildArtifact("a.out") exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
# Break inside the foo function which takes a bar_ptr argument. # Break inside the foo function which takes a bar_ptr argument.
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.m", self.line, num_expected_locations=1, loc_exact=True) self, "main.m", self.line, num_expected_locations=1, loc_exact=True)
Show All 16 Lines

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-incomplete/Makefile

LEVEL = ../../../make LEVEL = ../../../make
OBJC_SOURCES := main.m myModule.m OBJC_SOURCES := main.m myModule.m
include $(LEVEL)/Makefile.rules include $(LEVEL)/Makefile.rules
CFLAGS += -fmodules -I$(PWD) CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)
LDFLAGS += -framework Foundation LDFLAGS += -framework Foundation

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-incomplete/TestIncompleteModules.py

Show All 18 Linesclass IncompleteModulesTestCase(TestBase):
def setUp(self): def setUp(self):
# Call super's setUp(). # Call super's setUp().
TestBase.setUp(self) TestBase.setUp(self)
# Find the line number to break inside main(). # Find the line number to break inside main().
self.line = line_number('main.m', '// Set breakpoint 0 here.') self.line = line_number('main.m', '// Set breakpoint 0 here.')
@skipUnlessDarwin @skipUnlessDarwin
@unittest2.expectedFailure("rdar://20416388") @unittest2.expectedFailure("rdar://20416388")
@skipIf(macos_version=["<", "10.12"]) @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self): def test_expr(self):
self.build() self.build()
exe = self.getBuildArtifact("a.out") exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
# Break inside the foo function which takes a bar_ptr argument. # Break inside the foo function which takes a bar_ptr argument.
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.m", self.line, num_expected_locations=1, loc_exact=True) self, "main.m", self.line, num_expected_locations=1, loc_exact=True)
Show All 32 Lines

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/Makefile

LEVEL = ../../../make LEVEL = ../../../make
C_SOURCES := myModule.c C_SOURCES := myModule.c
OBJC_SOURCES := main.m OBJC_SOURCES := main.m
include $(LEVEL)/Makefile.rules include $(LEVEL)/Makefile.rules
CFLAGS += -fmodules -I$(PWD) CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS) -I$(PWD)

lldb/trunk/packages/Python/lldbsuite/test/lang/objc/modules-inline-functions/TestModulesInlineFunctions.py

Show All 21 Linesclass ModulesInlineFunctionsTestCase(TestBase):
def setUp(self): def setUp(self):
# Call super's setUp(). # Call super's setUp().
TestBase.setUp(self) TestBase.setUp(self)
# Find the line number to break inside main(). # Find the line number to break inside main().
self.line = line_number('main.m', '// Set breakpoint here.') self.line = line_number('main.m', '// Set breakpoint here.')
@skipUnlessDarwin @skipUnlessDarwin
@skipIf(macos_version=["<", "10.12"]) @skipIf(macos_version=["<", "10.12"], debug_info=no_match(["gmodules"]))
def test_expr(self): def test_expr(self):
self.build() self.build()
exe = self.getBuildArtifact("a.out") exe = self.getBuildArtifact("a.out")
self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET) self.runCmd("file " + exe, CURRENT_EXECUTABLE_SET)
# Break inside the foo function which takes a bar_ptr argument. # Break inside the foo function which takes a bar_ptr argument.
lldbutil.run_break_set_by_file_and_line( lldbutil.run_break_set_by_file_and_line(
self, "main.m", self.line, num_expected_locations=1, loc_exact=True) self, "main.m", self.line, num_expected_locations=1, loc_exact=True)
Show All 22 Lines

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

Show First 20 Lines • Show All 231 Lines • ▼ Show 20 Lines
else else
CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS) CFLAGS_NO_DEBUG = -O0 $(ARCHFLAG)$(ARCH) $(FRAMEWORK_INCLUDES) $(ARCH_CFLAGS) $(CFLAGS_EXTRAS)
endif endif
ifeq "$(MAKE_DWO)" "YES" ifeq "$(MAKE_DWO)" "YES"
CFLAGS += -gsplit-dwarf CFLAGS += -gsplit-dwarf
endif endif
CLANG_MODULE_CACHE_DIR := module-cache
MANDATORY_MODULE_BUILD_CFLAGS := -fmodules -gmodules -fmodules-cache-path=$(CLANG_MODULE_CACHE_DIR)
ifeq "$(MAKE_GMODULES)" "YES" ifeq "$(MAKE_GMODULES)" "YES"
CFLAGS += -fmodules -gmodules CFLAGS += $(MANDATORY_MODULE_BUILD_CFLAGS)
endif endif
CXXFLAGS += -std=c++11 CXXFLAGS += -std=c++11 $(CFLAGS)
# FIXME: C++ modules aren't supported on all platforms.
CXXFLAGS += $(subst -fmodules,, $(CFLAGS))
LD = $(CC) LD = $(CC)
LDFLAGS ?= $(CFLAGS) LDFLAGS ?= $(CFLAGS)
LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS) LDFLAGS += $(LD_EXTRAS) $(ARCH_LDFLAGS)
ifeq (,$(filter $(OS), Windows_NT Android Darwin)) ifeq (,$(filter $(OS), Windows_NT Android Darwin))
ifneq (,$(filter YES,$(ENABLE_THREADS))) ifneq (,$(filter YES,$(ENABLE_THREADS)))
LDFLAGS += -pthread LDFLAGS += -pthread
endif endif
endif endif
▲ Show 20 Lines • Show All 376 Lines • ▼ Show 20 Lines
.SUFFIXES: .mm .o .SUFFIXES: .mm .o
.mm.o: .mm.o:
$(CXX) $(CXXFLAGS) -c $< $(CXX) $(CXXFLAGS) -c $<
.PHONY: clean .PHONY: clean
dsym: $(DSYM) dsym: $(DSYM)
all: $(EXE) $(DSYM) all: $(EXE) $(DSYM)
clean:: clean::
$(RM) $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(RM) -rf $(OBJECTS) $(PREREQS) $(PREREQS:.d=.d.tmp) $(DWOS) $(ARCHIVE_NAME) $(ARCHIVE_OBJECTS) $(CLANG_MODULE_CACHE_DIR)
ifneq "$(DYLIB_NAME)" "" ifneq "$(DYLIB_NAME)" ""
$(RM) -r $(DYLIB_FILENAME).dSYM $(RM) -r $(DYLIB_FILENAME).dSYM
$(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug $(RM) $(DYLIB_OBJECTS) $(DYLIB_PREREQS) $(DYLIB_PREREQS:.d=.d.tmp) $(DYLIB_DWOS) $(DYLIB_FILENAME) $(DYLIB_FILENAME).debug
endif endif
ifneq "$(PCH_OUTPUT)" "" ifneq "$(PCH_OUTPUT)" ""
$(RM) $(PCH_OUTPUT) $(RM) $(PCH_OUTPUT)
endif endif
ifneq "$(DSYM)" "" ifneq "$(DSYM)" ""
Show All 27 Lines