Skip to content

Commit

Permalink
[Core] BuildEngine should not be copyable.
Browse files Browse the repository at this point in the history
 - This also fixes a bug in the performance tests in which the engine *was*
   being copied, which lead to a use after free.

 - <rdar://problem/35571840> 3 test assertions in -[CorePerfTests testBuildEngineDependencyScanningOn2DMatrix]
  • Loading branch information
ddunbar committed Dec 1, 2017
1 parent 303a89b commit f138390
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 11 deletions.
14 changes: 10 additions & 4 deletions include/llbuild/Core/BuildEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// This source file is part of the Swift.org open source project
//
// Copyright (c) 2014 - 2015 Apple Inc. and the Swift project authors
// Copyright (c) 2014 - 2017 Apple Inc. and the Swift project authors
// Licensed under Apache License v2.0 with Runtime Library Exception
//
// See http://swift.org/LICENSE.txt for license information
Expand All @@ -13,16 +13,18 @@
#ifndef LLBUILD_CORE_BUILDENGINE_H
#define LLBUILD_CORE_BUILDENGINE_H

#include "llbuild/Basic/Compiler.h"

#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"

#include <cstdint>
#include <functional>
#include <memory>
#include <string>
#include <utility>
#include <vector>

#include "llvm/ADT/StringRef.h"
#include "llvm/ADT/Twine.h"

namespace llbuild {
namespace core {

Expand Down Expand Up @@ -237,6 +239,10 @@ class BuildEngineDelegate {
class BuildEngine {
void *impl;

// Copying is disabled.
BuildEngine(const BuildEngine&) LLBUILD_DELETED_FUNCTION;
void operator=(const BuildEngine&) LLBUILD_DELETED_FUNCTION;

public:
/// Create a build engine with the given delegate.
explicit BuildEngine(BuildEngineDelegate& delegate);
Expand Down
21 changes: 14 additions & 7 deletions perftests/Xcode/PerfTests/CorePerfTests.mm
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ @implementation CorePerfTests

#pragma mark - "buildengine ack" Performance Tests

- (void)measurePerformance:(std::function<void()>)body {
[self measureBlock:^() {
body();
}];
}

- (void)testBuildEngineBasicPerf {
// Test the timing of 'buildengine ack 3 14'.
//
Expand Down Expand Up @@ -153,7 +159,8 @@ virtual void error(const Twine& message) override {
abort();
}
} Delegate;
__block core::BuildEngine Engine(Delegate);
core::BuildEngine Engine(Delegate);

int LastInputValue = 0;
for (int i = 1; i <= M; ++i) {
char Name[32];
Expand Down Expand Up @@ -187,7 +194,7 @@ virtual void error(const Twine& message) override {
Engine.build("i1");

// Measure the null build time.
[self measureBlock:^{
[self measurePerformance: [&] {
auto Result = IntFromValue(Engine.build("i1"));
(void)Result;
assert(Result == LastInputValue);
Expand Down Expand Up @@ -233,7 +240,7 @@ virtual void error(const Twine& message) override {
abort();
}
} Delegate;
__block core::BuildEngine Engine(Delegate);
core::BuildEngine Engine(Delegate);
int LastInputValue = 0;
for (int i = 1; i <= M; ++i) {
// Compute the total number of groups at this depth.
Expand Down Expand Up @@ -274,7 +281,7 @@ virtual void error(const Twine& message) override {
Engine.build("i1,1");

// Measure the null build time.
[self measureBlock:^{
[self measurePerformance: [&] {
auto Result = IntFromValue(Engine.build("i1,1"));
(void)Result;
assert(Result == LastInputValue);
Expand All @@ -295,7 +302,7 @@ - (void)testBuildEngineDependencyScanningOn2DMatrix {
//
// This is an easy to construct synthetic graph which is scalable and has
// sharing.
int M = 1000, N = 1000; // Use a graph of 1 million nodes.
int M = 100, N = 100; // Use a graph of 1 million nodes.

// Set up the build rules.
struct MatrixDelegate : public BuildEngineDelegate {
Expand All @@ -317,7 +324,7 @@ virtual void error(const Twine& message) override {
abort();
}
} Delegate;
__block core::BuildEngine Engine(Delegate);
core::BuildEngine Engine(Delegate);
int LastInputValue = 0;
for (int i = 1; i <= M; ++i) {
for (int j = 1; j <= N; ++j) {
Expand Down Expand Up @@ -376,7 +383,7 @@ virtual void error(const Twine& message) override {
Engine.build("i1,1");

// Measure the null build time.
[self measureBlock:^{
[self measurePerformance: [&] {
auto Result = IntFromValue(Engine.build("i1,1"));
(void)Result;
assert(Result == LastInputValue);
Expand Down

0 comments on commit f138390

Please sign in to comment.