From: Dodji Seketeli Date: Wed, 1 Mar 2017 13:13:39 +0000 (+0100) Subject: Move test-read-dwarf.cc to abigail::workers X-Git-Tag: upstream/1.0~124 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0ebb20c6c226a18bfa35015e6754df46c6f5574f;p=platform%2Fupstream%2Flibabigail.git Move test-read-dwarf.cc to abigail::workers Moving this test away from using pthreads directly to use abigail::workers. This patch also updates the valgrind suppression file to suppress some Helgrind false positives. Those are due to: - libstdc++ apparently having some benign data races when emitting data to ostream. This seems related to some facet manipulation that happen at that point. - some benign data race in some elfutils functions. * tests/test-read-dwarf.cc (iospec, spec_lock, write_lock) (out_abi_base, in_elf_base, in_abi_base): Remove these global variables. (handle_in_out_spec): Remove this. (struct test_task): Write this task that does what handle_in_out_spec was doing. (test_task_sptr): Define new typedef. (main): Remove the pthreads artifacts. Use the new test_task type along with the abigail::workers interface. * tests/test-valgrind-suppressions.supp: Add more helgrind suppressions for ostream writting false positives. Signed-off-by: Dodji Seketeli --- diff --git a/tests/test-read-dwarf.cc b/tests/test-read-dwarf.cc index 245a27ab..349b1a5c 100644 --- a/tests/test-read-dwarf.cc +++ b/tests/test-read-dwarf.cc @@ -27,18 +27,20 @@ #include #include #include +#include #include -#include -#include #include "abg-ir.h" #include "abg-dwarf-reader.h" +#include "abg-workers.h" #include "abg-writer.h" #include "abg-tools-utils.h" #include "test-utils.h" +using std::vector; using std::string; using std::ofstream; using std::cerr; +using std::tr1::dynamic_pointer_cast; using abigail::tests::get_build_dir; using abigail::dwarf_reader::read_corpus_from_elf; using abigail::dwarf_reader::read_context; @@ -218,20 +220,6 @@ InOutSpec in_out_specs[] = {NULL, NULL, NULL, NULL} }; -// The global pointer to the testsuite paths. -InOutSpec *iospec = in_out_specs; -// Lock to help atomically increment iospec. -pthread_mutex_t spec_lock = PTHREAD_MUTEX_INITIALIZER; -pthread_mutex_t write_lock = PTHREAD_MUTEX_INITIALIZER; -// No lock needed here, since is_ok is only ever re-set to false. -bool is_ok = true; - -// These prefixes don't change during the program's lifetime, so -// we only get them once. -const string out_abi_base = string(get_build_dir()) + "/tests/"; -const string in_elf_base = string(abigail::tests::get_src_dir()) + "/tests/"; -const string in_abi_base = in_elf_base; - using abigail::suppr::suppression_sptr; using abigail::suppr::suppressions_type; using abigail::suppr::read_suppressions; @@ -249,45 +237,47 @@ set_suppressions(read_context& read_ctxt, const string& path) add_read_context_suppressions(read_ctxt, supprs); } -/// Read the current entry of the global in_out_specs variable and -/// according to the data it contains, read an ELF binary (possibly -/// taking its accompanying suppression specification into account), -/// serialize its ABI into the .abi format, compare that .abi against -/// a reference one using GNU diff. -/// -/// Also run abidw --abidiff against the ELF binary specified by the -/// current entry of in_out_specs. -/// -/// Note that the current entry of in_out_specs is the one pointed to -/// by the iospec pointer. This function increments that pointer -/// after each invocation. -void -handle_in_out_spec(void) +/// The task that peforms the tests. +struct test_task : public abigail::workers::task { - string in_elf_path, in_abi_path, in_suppr_spec_path, out_abi_path; - abigail::ir::environment_sptr env; - InOutSpec *s; + bool is_ok; + InOutSpec spec; + string error_message; + string out_abi_base; + string in_elf_base; + string in_abi_base; + + test_task(const InOutSpec &s, + string& a_out_abi_base, + string& a_in_elf_base, + string& a_in_abi_base) + : is_ok(true), + spec(s), + out_abi_base(a_out_abi_base), + in_elf_base(a_in_elf_base), + in_abi_base(a_in_abi_base) + {} - while (true) + /// The actual test. + /// + /// This reads the corpus into memory, saves it to disk, loads it + /// again and compares the new in-memory representation against the + /// saved one. + virtual void + perform() { - pthread_mutex_lock(&spec_lock); - if (iospec->in_elf_path) - s = iospec++; - else - s = NULL; - pthread_mutex_unlock(&spec_lock); - - if (!s) - pthread_exit(NULL); - in_elf_path = in_elf_base + s->in_elf_path; - if (s->in_suppr_spec_path) - in_suppr_spec_path = in_elf_base + s->in_suppr_spec_path; + string in_elf_path, in_abi_path, in_suppr_spec_path, out_abi_path; + abigail::ir::environment_sptr env; + + in_elf_path = in_elf_base + spec.in_elf_path; + if (spec.in_suppr_spec_path) + in_suppr_spec_path = in_elf_base + spec.in_suppr_spec_path; else in_suppr_spec_path.clear(); env.reset(new abigail::ir::environment); abigail::dwarf_reader::status status = - abigail::dwarf_reader::STATUS_UNKNOWN; + abigail::dwarf_reader::STATUS_UNKNOWN; read_context_sptr ctxt = create_read_context(in_elf_path, /*debug_info_root_path=*/0, env.get()); @@ -298,71 +288,66 @@ handle_in_out_spec(void) abigail::corpus_sptr corp = read_corpus_from_elf(*ctxt, status); if (!corp) { - cerr << "failed to read " << in_elf_path << "\n"; + error_message = string("failed to read ") + in_elf_path + "\n"; is_ok = false; - continue; + return; } - corp->set_path(s->in_elf_path); + corp->set_path(spec.in_elf_path); // Do not take architecture names in comparison so that these // test input binaries can come from whatever arch the // programmer likes. corp->set_architecture_name(""); - out_abi_path = out_abi_base + s->out_abi_path; + out_abi_path = out_abi_base + spec.out_abi_path; if (!abigail::tools_utils::ensure_parent_dir_created(out_abi_path)) { - cerr << "Could not create parent directory for " << out_abi_path; + error_message = + string("Could not create parent directory for ") + out_abi_path; is_ok = false; - exit(1); + return; } ofstream of(out_abi_path.c_str(), std::ios_base::trunc); if (!of.is_open()) { - cerr << "failed to read " << out_abi_path << "\n"; + error_message = string("failed to read ") + out_abi_path + "\n"; is_ok = false; - continue; + return; } - - pthread_mutex_lock(&write_lock); is_ok = - abigail::xml_writer::write_corpus_to_native_xml(corp, - /*indent=*/0, - of); - pthread_mutex_unlock(&write_lock); + abigail::xml_writer::write_corpus_to_native_xml(corp, /*indent=*/0, of); of.close(); string abidw = string(get_build_dir()) + "/tools/abidw"; string cmd = abidw + " --abidiff " + in_elf_path; if (system(cmd.c_str())) { - cerr << "ABIs differ:\n" - << in_elf_path - << "\nand:\n" - << out_abi_path - << "\n"; + error_message = string("ABIs differ:\n") + + in_elf_path + + "\nand:\n" + + out_abi_path + + "\n"; is_ok = false; } - in_abi_path = in_abi_base + s->in_abi_path; + in_abi_path = in_abi_base + spec.in_abi_path; cmd = "diff -u " + in_abi_path + " " + out_abi_path; if (system(cmd.c_str())) is_ok = false; } -} +}; // end struct test_task + +typedef shared_ptr test_task_sptr; int main(int argc, char *argv[]) { - // Number of currently online processors in the system. - size_t nprocs = sysconf(_SC_NPROCESSORS_ONLN); - // All the pthread_ts we've created. - pthread_t *pthr = new pthread_t[nprocs]; + bool no_parallel = false; if (argc == 2) { if (argv[1] == string("--no-parallel")) - nprocs = 1; + no_parallel = true; else { cerr << "unrecognized option\n"; @@ -371,17 +356,55 @@ main(int argc, char *argv[]) } } - assert(nprocs >= 1); + /// Create a task queue. The max number of worker threads of the + /// queue is the number of the concurrent threads supported by the + /// processor of the machine this code runs on. But if + /// --no-parallel was provided then the number of worker threads + /// equals 1. + const size_t num_tests = sizeof(in_out_specs) / sizeof (InOutSpec) - 1; + size_t num_workers = (no_parallel + ? 1 + : std::min(abigail::workers::get_number_of_threads(), + num_tests)); + abigail::workers::queue task_queue(num_workers); + bool is_ok = true; - for (size_t i = 0; i < nprocs; ++i) - pthread_create(&pthr[i], NULL, - (void*(*)(void*))handle_in_out_spec, - NULL); + string out_abi_base = string(get_build_dir()) + "/tests/"; + string in_elf_base = string(abigail::tests::get_src_dir()) + "/tests/"; + string in_abi_base = in_elf_base; - for (size_t i = 0; i < nprocs; ++i) - pthread_join(pthr[i], NULL); + for (InOutSpec *s = in_out_specs; s->in_elf_path; ++s) + { + test_task_sptr t(new test_task(*s, out_abi_base, + in_elf_base, + in_abi_base)); + assert(task_queue.schedule_task(t)); + } + + /// Wait for all worker threads to finish their job, and wind down. + task_queue.wait_for_workers_to_complete(); + + // Now walk the results and print whatever error messages need to be + // printed. + + const vector& completed_tasks = + task_queue.get_completed_tasks(); - delete [] pthr; + assert(completed_tasks.size() == num_tests); + + for (vector::const_iterator ti = + completed_tasks.begin(); + ti != completed_tasks.end(); + ++ti) + { + test_task_sptr t = dynamic_pointer_cast(*ti); + if (!t->is_ok) + { + is_ok = false; + if (!t->error_message.empty()) + cerr << t->error_message << '\n'; + } + } return !is_ok; } diff --git a/tests/test-valgrind-suppressions.supp b/tests/test-valgrind-suppressions.supp index 75de6d2b..95999883 100644 --- a/tests/test-valgrind-suppressions.supp +++ b/tests/test-valgrind-suppressions.supp @@ -53,6 +53,12 @@ fun:elf_begin } +{ + suppress helgrind another race report from elfutils. + Helgrind:Race + fun:_dlerror_run +} + { suppress helgrind race report from inserting into an ostringstream from abigail::ir::array_type_def::subrange_type::as_string() const Helgrind:Race @@ -66,3 +72,31 @@ fun:_ZNSo9_M_insertImEERSoT_ fun:_ZN7abigail10comparison11corpus_diff4priv15emit_diff_statsERKNS1_10diff_statsERSoRKSs } + +{ + suppress helgrind race report from inserting into an ostream from abigail::xml_writer::write_translation_unit + Helgrind:Race + fun:_ZNSo9_M_insertIlEERSoT_ + fun:_ZN7abigail10xml_writerL22write_translation_unitERKNS_2ir16translation_unitERNS0_13write_contextEj +} + +{ + Writting to ostream has data races (IAUI) due to facets-related manipulations. + Helgrind:Race + fun:_ZNSo9_M_insertImEERSoT_ + fun:_ZN7abigail10xml_writerL16write_elf_symbolERKNSt3tr110shared_ptrINS_2ir10elf_symbolEEERNS0_13write_contextEj +} + +{ + Writting to ostream has data races (IAUI) I believe due to facets-related manipulations. + Helgrind:Race + fun:_ZNSo9_M_insertImEERSoT_ + fun:_ZN7abigail10xml_writerL16write_elf_symbolERKNSt3tr110shared_ptrINS_2ir10elf_symbolEEERNS0_13write_contextEj +} + +{ + Writting to ostream has data races (IAUI) I believe due to facets-related manipulations. + Helgrind:Race + fun:_ZNSo9_M_insertIyEERSoT_ + fun:_ZNK7abigail10xml_writer10id_manager18get_id_with_prefixERKSs +} \ No newline at end of file