Skip to content

Commit

Permalink
Rollup merge of rust-lang#60038 - michaelwoerister:pgo-updates-2, r=a…
Browse files Browse the repository at this point in the history
…lexcrichton

Add codegen test for PGO instrumentation.

This PR adds a codegen test that makes sure that LLVM actually generates instrumentation code when we enable PGO instrumentation in `rustc`.

The second commit updates a test case to the new commandline option syntax introduced in rust-lang#59874. Without the fix the test still works, but it confusingly creates a directory called `test.profraw`, which usually is the name of the _file_ where profiling data is collected.
  • Loading branch information
Centril authored Apr 25, 2019
2 parents a552beb + ff976fe commit bb892be
Show file tree
Hide file tree
Showing 14 changed files with 71 additions and 43 deletions.
4 changes: 2 additions & 2 deletions src/bootstrap/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1268,11 +1268,11 @@ impl Step for Compiletest {
builder.add_rust_test_threads(&mut cmd);

if builder.config.sanitizers {
cmd.env("SANITIZER_SUPPORT", "1");
cmd.env("RUSTC_SANITIZER_SUPPORT", "1");
}

if builder.config.profiler {
cmd.env("PROFILER_SUPPORT", "1");
cmd.env("RUSTC_PROFILER_SUPPORT", "1");
}

cmd.env("RUST_TEST_TMPDIR", builder.out.join("tmp"));
Expand Down
20 changes: 20 additions & 0 deletions src/test/codegen/pgo-instrumentation.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
// Test that `-Zpgo-gen` creates expected instrumentation artifacts in LLVM IR.

// needs-profiler-support
// compile-flags: -Z pgo-gen -Ccodegen-units=1

// CHECK: @__llvm_profile_raw_version =
// CHECK: @__profc_{{.*}}pgo_instrumentation{{.*}}some_function{{.*}} = private global
// CHECK: @__profd_{{.*}}pgo_instrumentation{{.*}}some_function{{.*}} = private global
// CHECK: @__profc_{{.*}}pgo_instrumentation{{.*}}main{{.*}} = private global
// CHECK: @__profd_{{.*}}pgo_instrumentation{{.*}}main{{.*}} = private global
// CHECK: @__llvm_profile_filename = {{.*}}"default_%m.profraw\00"{{.*}}

#[inline(never)]
fn some_function() {

}

fn main() {
some_function();
}
4 changes: 2 additions & 2 deletions src/test/run-make-fulldeps/pgo-gen-lto/Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# needs-profiler-support

-include ../tools.mk

all:
ifeq ($(PROFILER_SUPPORT),1)
$(RUSTC) -Copt-level=3 -Clto=fat -Z pgo-gen="$(TMPDIR)" test.rs
$(call RUN,test) || exit 1
[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
endif
6 changes: 3 additions & 3 deletions src/test/run-make-fulldeps/pgo-gen-no-imp-symbols/Makefile
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
# needs-profiler-support

-include ../tools.mk

all:
ifeq ($(PROFILER_SUPPORT),1)
$(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)/test.profraw" --emit=llvm-ir test.rs
$(RUSTC) -O -Ccodegen-units=1 -Z pgo-gen="$(TMPDIR)" --emit=llvm-ir test.rs
# We expect symbols starting with "__llvm_profile_".
$(CGREP) "__llvm_profile_" < $(TMPDIR)/test.ll
# We do NOT expect the "__imp_" version of these symbols.
$(CGREP) -v "__imp___llvm_profile_" < $(TMPDIR)/test.ll # 64 bit
$(CGREP) -v "__imp____llvm_profile_" < $(TMPDIR)/test.ll # 32 bit
endif
4 changes: 2 additions & 2 deletions src/test/run-make-fulldeps/pgo-gen/Makefile
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
# needs-profiler-support

-include ../tools.mk

all:
ifeq ($(PROFILER_SUPPORT),1)
$(RUSTC) -g -Z pgo-gen="$(TMPDIR)" test.rs
$(call RUN,test) || exit 1
[ -e "$(TMPDIR)"/default_*.profraw ] || (echo "No .profraw file"; exit 1)
endif
4 changes: 2 additions & 2 deletions src/test/run-make-fulldeps/profile/Makefile
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
# needs-profiler-support

-include ../tools.mk

all:
ifeq ($(PROFILER_SUPPORT),1)
$(RUSTC) -g -Z profile test.rs
$(call RUN,test) || exit 1
[ -e "$(TMPDIR)/test.gcno" ] || (echo "No .gcno file"; exit 1)
[ -e "$(TMPDIR)/test.gcda" ] || (echo "No .gcda file"; exit 1)
endif
6 changes: 2 additions & 4 deletions src/test/run-make-fulldeps/sanitizer-address/Makefile
Original file line number Diff line number Diff line change
@@ -1,15 +1,15 @@
# needs-sanitizer-support

-include ../tools.mk

LOG := $(TMPDIR)/log.txt

# NOTE the address sanitizer only supports x86_64 linux and macOS

ifeq ($(TARGET),x86_64-apple-darwin)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)
EXTRA_RUSTFLAG=-C rpath
else
ifeq ($(TARGET),x86_64-unknown-linux-gnu)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)

# Apparently there are very specific Linux kernels, notably the one that's
# currently on Travis CI, which contain a buggy commit that triggers failures in
Expand All @@ -23,7 +23,5 @@ endif
endif

all:
ifeq ($(ASAN_SUPPORT),1)
$(RUSTC) -g -Z sanitizer=address -Z print-link-args $(EXTRA_RUSTFLAG) overflow.rs | $(CGREP) librustc_asan
$(TMPDIR)/overflow 2>&1 | $(CGREP) stack-buffer-overflow
endif
10 changes: 4 additions & 6 deletions src/test/run-make-fulldeps/sanitizer-cdylib-link/Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# needs-sanitizer-support
# only-x86_64
# only-linux

-include ../tools.mk

LOG := $(TMPDIR)/log.txt
Expand All @@ -7,16 +11,10 @@ LOG := $(TMPDIR)/log.txt
# are compiled with address sanitizer, and we assert that a fault in the cdylib
# is correctly detected.

ifeq ($(TARGET),x86_64-unknown-linux-gnu)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)

# See comment in sanitizer-address/Makefile for why this is here
EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
endif

all:
ifeq ($(ASAN_SUPPORT),1)
$(RUSTC) -g -Z sanitizer=address --crate-type cdylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
$(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) $(EXTRA_RUSTFLAG) -llibrary program.rs
LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
endif
10 changes: 4 additions & 6 deletions src/test/run-make-fulldeps/sanitizer-dylib-link/Makefile
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
# needs-sanitizer-support
# only-x86_64
# only-linux

-include ../tools.mk

LOG := $(TMPDIR)/log.txt
Expand All @@ -7,16 +11,10 @@ LOG := $(TMPDIR)/log.txt
# are compiled with address sanitizer, and we assert that a fault in the dylib
# is correctly detected.

ifeq ($(TARGET),x86_64-unknown-linux-gnu)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)

# See comment in sanitizer-address/Makefile for why this is here
EXTRA_RUSTFLAG=-C relocation-model=dynamic-no-pic
endif

all:
ifeq ($(ASAN_SUPPORT),1)
$(RUSTC) -g -Z sanitizer=address --crate-type dylib --target $(TARGET) $(EXTRA_RUSTFLAG) library.rs
$(RUSTC) -g -Z sanitizer=address --crate-type bin --target $(TARGET) $(EXTRA_RUSTFLAG) -llibrary program.rs
LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
endif
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
# needs-sanitizer-support

-include ../tools.mk

# NOTE the address sanitizer only supports x86_64 linux and macOS

ifeq ($(TARGET),x86_64-apple-darwin)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)
EXTRA_RUSTFLAG=-C rpath
else
ifeq ($(TARGET),x86_64-unknown-linux-gnu)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)
EXTRA_RUSTFLAG=
endif
endif

all:
ifeq ($(ASAN_SUPPORT),1)
$(RUSTC) -Z sanitizer=address --crate-type proc-macro --target $(TARGET) hello.rs 2>&1 | $(CGREP) '-Z sanitizer'
endif
3 changes: 1 addition & 2 deletions src/test/run-make-fulldeps/sanitizer-leak/Makefile
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
-include ../tools.mk

# needs-sanitizer-support
# only-linux
# only-x86_64
# ignore-test
# FIXME(#46126) ThinLTO for libstd broke this test

all:
ifdef SANITIZER_SUPPORT
$(RUSTC) -C opt-level=1 -g -Z sanitizer=leak -Z print-link-args leak.rs | $(CGREP) librustc_lsan
$(TMPDIR)/leak 2>&1 | $(CGREP) 'detected memory leaks'
endif
3 changes: 1 addition & 2 deletions src/test/run-make-fulldeps/sanitizer-memory/Makefile
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
-include ../tools.mk

# needs-sanitizer-support
# only-linux
# only-x86_64

all:
ifdef SANITIZER_SUPPORT
$(RUSTC) -g -Z sanitizer=memory -Z print-link-args uninit.rs | $(CGREP) librustc_msan
$(TMPDIR)/uninit 2>&1 | $(CGREP) use-of-uninitialized-value
endif
13 changes: 5 additions & 8 deletions src/test/run-make-fulldeps/sanitizer-staticlib-link/Makefile
Original file line number Diff line number Diff line change
@@ -1,18 +1,15 @@
# needs-sanitizer-support
# only-x86_64
# only-linux

-include ../tools.mk

# This test builds a staticlib, then an executable that links to it.
# The staticlib and executable both are compiled with address sanitizer,
# The staticlib and executable both are compiled with address sanitizer,
# and we assert that a fault in the staticlib is correctly detected.

ifeq ($(TARGET),x86_64-unknown-linux-gnu)
ASAN_SUPPORT=$(SANITIZER_SUPPORT)
EXTRA_RUSTFLAG=
endif

all:
ifeq ($(ASAN_SUPPORT),1)
$(RUSTC) -g -Z sanitizer=address --crate-type staticlib --target $(TARGET) library.rs
$(CC) program.c $(call STATICLIB,library) $(call OUT_EXE,program) $(EXTRACFLAGS) $(EXTRACXXFLAGS)
LD_LIBRARY_PATH=$(TMPDIR) $(TMPDIR)/program 2>&1 | $(CGREP) stack-buffer-overflow
endif

21 changes: 21 additions & 0 deletions src/tools/compiletest/src/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,9 @@ impl EarlyProps {
}
}

let rustc_has_profiler_support = env::var_os("RUSTC_PROFILER_SUPPORT").is_some();
let rustc_has_sanitizer_support = env::var_os("RUSTC_SANITIZER_SUPPORT").is_some();

iter_header(testfile, None, &mut |ln| {
// we should check if any only-<platform> exists and if it exists
// and does not matches the current platform, skip the test
Expand Down Expand Up @@ -116,6 +119,16 @@ impl EarlyProps {
config.parse_needs_matching_clang(ln) {
props.ignore = Ignore::Ignore;
}

if !rustc_has_profiler_support &&
config.parse_needs_profiler_support(ln) {
props.ignore = Ignore::Ignore;
}

if !rustc_has_sanitizer_support &&
config.parse_needs_sanitizer_support(ln) {
props.ignore = Ignore::Ignore;
}
}

if (config.mode == common::DebugInfoGdb || config.mode == common::DebugInfoBoth) &&
Expand Down Expand Up @@ -748,6 +761,14 @@ impl Config {
self.parse_name_directive(line, "needs-matching-clang")
}

fn parse_needs_profiler_support(&self, line: &str) -> bool {
self.parse_name_directive(line, "needs-profiler-support")
}

fn parse_needs_sanitizer_support(&self, line: &str) -> bool {
self.parse_name_directive(line, "needs-sanitizer-support")
}

/// Parses a name-value directive which contains config-specific information, e.g., `ignore-x86`
/// or `normalize-stderr-32bit`.
fn parse_cfg_name_directive(&self, line: &str, prefix: &str) -> ParsedNameDirective {
Expand Down

0 comments on commit bb892be

Please sign in to comment.