Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Coverage #303

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Coverage #303

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
25f733b
add support for extra_files
svkf Oct 17, 2017
cee58f7
make a few changes for the protocol 38
svkf Oct 18, 2017
809a9cc
update a few variables that shadowed others (noticed in travis build)
svkf Oct 18, 2017
45da679
fixup issues with mac c++
svkf Oct 18, 2017
ca30b0f
fixup more bind issues
svkf Oct 18, 2017
b67a60d
update typo in comment
svkf Oct 18, 2017
7fd5057
Merge branch 'master' into coverage
HenryMiller1 Oct 31, 2017
dd748ea
Merge branch 'master' into coverage
HenryMiller1 Oct 31, 2017
3d2f477
update to latest protocol and fix other issues
svkf Jan 22, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 30 additions & 7 deletions client/arg.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,6 @@ bool analyse_argv(const char * const *argv, CompileJob &job, bool icerun, list<s
{
ArgumentsList args;
string ofile;
string dwofile;

#if CLIENT_DEBUG > 1
trace() << "scanning arguments" << endl;
Expand All @@ -195,6 +194,8 @@ bool analyse_argv(const char * const *argv, CompileJob &job, bool icerun, list<s
bool seen_mf = false;
bool seen_md = false;
bool seen_split_dwarf = false;
bool seen_coverage = false;
bool seen_save_temps = false;
// if rewriting includes and precompiling on remote machine, then cpp args are not local
Argument_Type Arg_Cpp = compiler_only_rewrite_includes(job) ? Arg_Rest : Arg_Local;

Expand Down Expand Up @@ -328,19 +329,27 @@ bool analyse_argv(const char * const *argv, CompileJob &job, bool icerun, list<s
}
} else if (!strcmp(a, "-S")) {
seen_s = true;
} else if (!strcmp(a, "-fprofile-arcs")
|| !strcmp(a, "-ftest-coverage")
|| !strcmp(a, "-frepo")
} else if (!strcmp(a, "-frepo")
|| !strcmp(a, "-fprofile-generate")
|| !strcmp(a, "-fprofile-use")
|| !strcmp(a, "-save-temps")
|| !strcmp(a, "-save-temps=cwd")
|| !strcmp(a, "--save-temps")
|| !strcmp(a, "-fbranch-probabilities")) {
log_info() << "compiler will emit profile info (argument " << a << "); building locally" << endl;
always_local = true;
args.append(a, Arg_Local);
} else if (!strcmp(a, "-gsplit-dwarf")) {
seen_split_dwarf = true;
args.append(a, Arg_Rest);
} else if (!strcmp(a, "-fprofile-arcs")
|| !strcmp(a, "-ftest-coverage")
|| !strcmp(a, "--coverage")) {
seen_coverage = true;
args.append(a, Arg_Rest);
} else if (!strcmp(a, "-save-temps=obj")) {
seen_save_temps = true;
args.append(a, Arg_Rest);
} else if (str_equal(a, "-x")) {
args.append(a, Arg_Rest);
bool unsupported = true;
Expand Down Expand Up @@ -590,9 +599,6 @@ bool analyse_argv(const char * const *argv, CompileJob &job, bool icerun, list<s
args.append("-S", Arg_Remote);
} else {
args.append("-c", Arg_Remote);
if (seen_split_dwarf) {
job.setDwarfFissionEnabled(true);
}
}

if (!always_local) {
Expand Down Expand Up @@ -720,6 +726,23 @@ bool analyse_argv(const char * const *argv, CompileJob &job, bool icerun, list<s

job.setFlags(args);
job.setOutputFile(ofile);
if (seen_split_dwarf) {
// need to do after the output file is set */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove the c style comment end - this is a C++ comment

log_info() << "Seen SplitDwarf" << std::endl;
job.setExtraOutputFileEnum(CompileJob::eExFile_dwarfFission);
}
if (seen_coverage) {
// we really only need this to test if the return protocol will work
log_info() << "Seen Coverage" << std::endl;
job.setExtraOutputFileEnum(CompileJob::eExFile_coverage);
}
if (seen_save_temps) {
// we really only need this to test if the return protocol will work
log_info() << "Save save-temps=obj" << std::endl;
job.setExtraOutputFileEnum(CompileJob::eExFile_st_i);
job.setExtraOutputFileEnum(CompileJob::eExFile_st_ii);
job.setExtraOutputFileEnum(CompileJob::eExFile_st_s);
}

#if CLIENT_DEBUG
trace() << "scanned result: local args=" << concat_args(job.localFlags())
Expand Down
4 changes: 0 additions & 4 deletions client/local.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,10 +228,6 @@ int build_local(CompileJob &job, MsgChannel *local_daemon, struct rusage *used)
arguments.push_back(compiler_name);
appendList(arguments, job.allFlags());

if (job.dwarfFissionEnabled()) {
arguments.push_back("-gsplit-dwarf");
}

if (!job.inputFile().empty()) {
arguments.push_back(job.inputFile());
}
Expand Down
73 changes: 35 additions & 38 deletions client/remote.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -590,16 +590,25 @@ static int build_remote_int(CompileJob &job, UseCSMsg *usecs, MsgChannel *local_
}
}

bool have_dwo_file = crmsg->have_dwo_file;
uint32_t extra_files = crmsg->extra_files;
//log_info() << "extra_files: " << extra_files << std::endl;
delete crmsg;

assert(!job.outputFile().empty());

if (status == 0) {
receive_file(job.outputFile(), cserver);
if (have_dwo_file) {
string dwo_output = job.outputFile().substr(0, job.outputFile().find_last_of('.')) + ".dwo";
receive_file(dwo_output, cserver);
uint32_t i(0);
while (extra_files)
{
++i;
extra_files >>= 1;
if (extra_files & 0x01)
{
std::string file(job.ExtraOutputFileEnum((CompileJob::ExFileEnum)i));
log_info() << "get extra file [" << i << "] = " << file << std::endl;
receive_file(file, cserver);
}
}
}

Expand Down Expand Up @@ -704,11 +713,10 @@ maybe_build_local(MsgChannel *local_daemon, UseCSMsg *usecs, CompileJob &job,
struct stat st;

msg.out_uncompressed = 0;
if (!stat(job.outputFile().c_str(), &st)) {
msg.out_uncompressed += st.st_size;
}
if (!stat((job.outputFile().substr(0, job.outputFile().find_last_of('.')) + ".dwo").c_str(), &st)) {
msg.out_uncompressed += st.st_size;
for (auto i : job.outputFiles()) {
if (stat(i.second.c_str(), &st)) {
msg.out_uncompressed += st.st_size;
}
}

msg.user_msec = ru.ru_utime.tv_sec * 1000 + ru.ru_utime.tv_usec / 1000;
Expand Down Expand Up @@ -738,6 +746,10 @@ static int minimalRemoteVersion( const CompileJob& job)
version = max(version, 35);
}

if (job.extraOutputEnabled()) {
version = max(version, 38);
}

return version;
}

Expand All @@ -746,7 +758,6 @@ int build_remote(CompileJob &job, MsgChannel *local_daemon, const Environments &
srand(time(0) + getpid());

int torepeat = 1;
bool has_split_dwarf = job.dwarfFissionEnabled();

// older compilers do not support the options we need to make it reproducible
#if defined(__GNUC__) && ( ( (__GNUC__ == 3) && (__GNUC_MINOR__ >= 3) ) || (__GNUC__ >=4) )
Expand Down Expand Up @@ -942,10 +953,9 @@ int build_remote(CompileJob &job, MsgChannel *local_daemon, const Environments &
if (-1 == ::unlink(jobs[0].outputFile().c_str())){
log_perror("unlink outputFile failed") << "\t" << jobs[0].outputFile() << endl;
}
if (has_split_dwarf) {
string dwo_file = jobs[0].outputFile().substr(0, jobs[0].outputFile().find_last_of('.')) + ".dwo";
if (-1 == ::unlink(dwo_file.c_str())){
log_perror("unlink failed") << "\t" << dwo_file << endl;
for (auto it : job.outputFiles()) {
if (-1 == ::unlink(it.second.c_str())){
log_perror("unlink failed") << "\t" << it.second << endl;
}
}
exit_codes[0] = -1; // overwrite
Expand All @@ -963,45 +973,32 @@ int build_remote(CompileJob &job, MsgChannel *local_daemon, const Environments &
rename(jobs[0].outputFile().c_str(),
(jobs[0].outputFile() + ".caught").c_str());
rename(preproc, (string(preproc) + ".caught").c_str());
if (has_split_dwarf) {
string dwo_file = jobs[0].outputFile().substr(0, jobs[0].outputFile().find_last_of('.')) + ".dwo";
rename(dwo_file.c_str(), (dwo_file + ".caught").c_str());
for (auto it : job.outputFiles()) {
rename(it.second.c_str(), (it.second + ".caught").c_str());
}
exit_codes[0] = -1; // overwrite
break;
}
}

if (-1 == ::unlink(jobs[i].outputFile().c_str())){
log_perror("unlink failed") << "\t" << jobs[i].outputFile() << endl;
}
if (has_split_dwarf) {
string dwo_file = jobs[i].outputFile().substr(0, jobs[i].outputFile().find_last_of('.')) + ".dwo";
if (-1 == ::unlink(dwo_file.c_str())){
log_perror("unlink failed") << "\t" << dwo_file << endl;
for (auto it : job.outputFiles()) {
if (-1 == ::unlink(it.second.c_str())){
log_perror("unlink failed") << "\t" << it.second << endl;
}
}
delete umsgs[i];
}
} else {
if (-1 == ::unlink(jobs[0].outputFile().c_str())){
log_perror("unlink failed") << "\t" << jobs[0].outputFile() << endl;
}
if (has_split_dwarf) {
string dwo_file = jobs[0].outputFile().substr(0, jobs[0].outputFile().find_last_of('.')) + ".dwo";
if (-1 == ::unlink(dwo_file.c_str())){
log_perror("unlink failed") << "\t" << dwo_file << endl;
for (auto it : job.outputFiles()) {
if (-1 == ::unlink(it.second.c_str())){
log_perror("unlink failed") << "\t" << it.second << endl;
}
}

for (int i = 1; i < torepeat; i++) {
if (-1 == ::unlink(jobs[i].outputFile().c_str())){
log_perror("unlink failed") << "\t" << jobs[i].outputFile() << endl;
}
if (has_split_dwarf) {
string dwo_file = jobs[i].outputFile().substr(0, jobs[i].outputFile().find_last_of('.')) + ".dwo";
if (-1 == ::unlink(dwo_file.c_str())){
log_perror("unlink failed") << "\t" << dwo_file << endl;
for (auto it : job.outputFiles()) {
if (-1 == ::unlink(it.second.c_str())){
log_perror("unlink failed") << "\t" << it.second << endl;
}
}
delete umsgs[i];
Expand Down
2 changes: 1 addition & 1 deletion configure.ac
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ if test "$GCC" = yes; then
-Wshadow -Wpointer-arith $cast_align -Wwrite-strings \
-Waggregate-return -Wstrict-prototypes -Wmissing-prototypes \
-Wnested-externs $CFLAGS"
CXXFLAGS=" -g -W -Wall -Wpointer-arith $cast_align $wshadow -Wwrite-strings $CXXFLAGS"
CXXFLAGS=" -std=gnu++11 -g -W -Wall -Wpointer-arith $cast_align $wshadow -Wwrite-strings $CXXFLAGS"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is okay, but I would prefer a minimum level of this, while allowing higher standards if the compiler supports it... (I'm working on cmake which handles this easier than autotools this so don't worry about it)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah just the code get so much nicer ... for the for () loops
I can rewrite it the old way ... and remove if you want

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, I'm saying that if C++14 is available I want to use that instead of C++11.

I'm not sure how far back we want to support C++ standards, maybe the next release should require C++11, and C++17 for the one after that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My vote would be c++ 14 for the next release, and 17 after that. For what it's worth, I've seen several areas of code that could profit from string_view in C++ 17.

AC_MSG_NOTICE([Adding gcc options: $CFLAGS])
fi

Expand Down
10 changes: 5 additions & 5 deletions daemon/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ bool Daemon::setup_listen_fds()
myaddr.sin_port = htons(daemon_port);
myaddr.sin_addr.s_addr = INADDR_ANY;

if (bind(tcp_listen_fd, (struct sockaddr *)&myaddr,
if (::bind(tcp_listen_fd, (struct sockaddr *)&myaddr,
sizeof(myaddr)) < 0) {
log_perror("bind()");
sleep(2);
Expand Down Expand Up @@ -602,7 +602,7 @@ bool Daemon::setup_listen_fds()

myaddr.sun_family = AF_UNIX;

mode_t old_umask = -1U;
mode_t old_umask = (mode_t)-1U;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

odn't use C style cases - I think you want static_cast here?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, personally I jut hate the extra typing :)


if (getenv("ICECC_TEST_SOCKET") == NULL) {
#ifdef HAVE_LIBCAP_NG
Expand Down Expand Up @@ -650,17 +650,17 @@ bool Daemon::setup_listen_fds()
}
}

if (bind(unix_listen_fd, (struct sockaddr*)&myaddr, sizeof(myaddr)) < 0) {
if (::bind(unix_listen_fd, (struct sockaddr*)&myaddr, sizeof(myaddr)) < 0) {
log_perror("bind()");

if (old_umask != -1U) {
if (old_umask != (mode_t)-1U) {
umask(old_umask);
}

return false;
}

if (old_umask != -1U) {
if (old_umask != (mode_t)-1U) {
umask(old_umask);
}

Expand Down
55 changes: 36 additions & 19 deletions daemon/serve.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ static void write_output_file( const string& file, MsgChannel* client )
int obj_fd = -1;
try {
obj_fd = open(file.c_str(), O_RDONLY | O_LARGEFILE);
log_info() << "Write output file " << file << endl;

if (obj_fd == -1) {
log_error() << "open failed" << endl;
Expand Down Expand Up @@ -175,7 +176,7 @@ int handle_connection(const string &basedir, CompileJob *job,

Msg *msg = 0; // The current read message
unsigned int job_id = 0;
string tmp_path, obj_file, dwo_file;
string tmp_path, obj_file;

try {
if (job->environmentVersion().size()) {
Expand Down Expand Up @@ -260,7 +261,6 @@ int handle_connection(const string &basedir, CompileJob *job,
}

obj_file = output_dir + '/' + file_name;
dwo_file = obj_file.substr(0, obj_file.find_last_of('.')) + ".dwo";

ret = work_it(*job, job_stat, client, rmsg, tmp_path, job_working_dir, relative_file_path, mem_limit, client->fd);
}
Expand All @@ -272,6 +272,23 @@ int handle_connection(const string &basedir, CompileJob *job,

ret = work_it(*job, job_stat, client, rmsg, build_path, "", file_name, mem_limit, client->fd);
}
// check the extra output files, even if error, seems gcno et can be before the error
// so the .o is removed ... but the others remain
for (uint32_t i=CompileJob::eExFile_START;i<=CompileJob::eExFile_END; i++)
{
std::string file;
struct stat st;
file = obj_file.substr(0, obj_file.find_last_of('.')) + CompileJob::ef_ext[i];
/* this way we don't bother parsing any args ...
* but any file that exists ... we will send back
*/
//printf("try file ef[%d]= %s\n", i, file.c_str());
if (!stat(file.c_str(), &st)) {
job_stat[JobStatistics::out_uncompressed] += st.st_size;
job->setExtraOutputFileRemote((CompileJob::ExFileEnum)i, file);
log_info() << " exists ef[" << i << "]= " << file << std::endl;
}
}

if (ret) {
if (ret == EXIT_OUT_OF_MEMORY) { // we catch that as special case
Expand All @@ -281,18 +298,20 @@ int handle_connection(const string &basedir, CompileJob *job,
}
}

if (!client->send_msg(rmsg)) {
log_info() << "write of result failed" << endl;
throw myexception(EXIT_DISTCC_FAILED);
}

struct stat st;

if (!stat(obj_file.c_str(), &st)) {
job_stat[JobStatistics::out_uncompressed] += st.st_size;
}
if (!stat(dwo_file.c_str(), &st)) {
job_stat[JobStatistics::out_uncompressed] += st.st_size;
// old model
rmsg.have_dwo_file = job->dwarfFissionEnabled();
// now ensure we set those in the message
rmsg.extra_files = job->ExtraOutputFiles();

if (!client->send_msg(rmsg)) {
log_info() << "write of result failed" << endl;
throw myexception(EXIT_DISTCC_FAILED);
}

/* wake up parent and tell him that compile finished */
Expand All @@ -303,10 +322,12 @@ int handle_connection(const string &basedir, CompileJob *job,
}

if (rmsg.status == 0) {
write_output_file(obj_file, client);
if (rmsg.have_dwo_file) {
write_output_file(dwo_file, client);
// reset the orig output file with the right one, then we can loop through them all
job->setOutputFile(obj_file);
for (auto i : job->outputFiles()) {
write_output_file(i.second, client);
}

}

throw myexception(rmsg.status);
Expand All @@ -315,14 +336,10 @@ int handle_connection(const string &basedir, CompileJob *job,
delete client;
client = 0;

if (!obj_file.empty()) {
if (-1 == unlink(obj_file.c_str())){
log_perror("unlink failure") << "\t" << obj_file << endl;
}
}
if (!dwo_file.empty()) {
if (-1 == unlink(dwo_file.c_str())){
log_perror("unlink failure") << "\t" << dwo_file << endl;
job->setOutputFile(obj_file);
for (auto i : job->outputFiles()) {
if (-1 == unlink(i.second.c_str())){
log_perror("unlink failure") << "\t" << i.second << endl;
}
}
if (!tmp_path.empty()) {
Expand Down
Loading