Re: [PATCH] Fix issues identified by Coverity run from Aug 11 2014
William Cohen <[email protected]>
| Newsgroups | gmane.linux.oprofile |
|---|---|
| Message-ID | <[email protected]> |
On 08/12/2014 04:09 PM, Maynard Johnson wrote:
> Fix issues identified by Coverity run from Aug 11 2014
>
> Running Coverity against the oprofile source tree resulted
> in 8 problems being identified. One issue was dead code
> that was originally created for special handling of the
> IBM Cell BE processor. Other issues were fairly mundane,
> from uninitialized variables to ignoring return values of
> functions, etc.
Hi Maynard,
I attempted to build an fedora 20 rpm with the patch and it looks like need to remove an unused variable in profile_container::add(...) other wise get:
g++ -DHAVE_CONFIG_H -I. -I.. -I ../libop -I ../libutil -I ../libdb -I ../libopt++ -I ../libutil++ -I ../libop++ -I ../libregex -W -Wall -fno-common -ftemplate-depth-50 -Werror -g -c -o profile_container.o profile_container.cpp
profile_container.cpp: In member function 'void profile_container::add(const profile_t&, const op_bfd&, const string&, size_t)':
profile_container.cpp:78:13: error: variable 'header' set but not used [-Werror=unused-but-set-variable]
opd_header header = profile.get_header();
^
cc1plus: all warnings being treated as errors
make[2]: *** [profile_container.o] Error 1
After fixing that I ran it through the version of coverity available at Red Hat. There are still a few minor coverity errors being reported, but they look harmless. I don't see how the CHECKED_RETURN is possible because the read return value is being checked.
Error: CHECKED_RETURN (CWE-252): [#def1]
oprofile-1.0.0git/libperf_events/operf_counter.cpp:66: check_return: "read(int, void *, size_t)" returns the number of bytes read, but it is ignored.
Error: DEADCODE (CWE-561): [#def2]
oprofile-1.0.0git/libpp/image_errors.cpp:68: dead_error_condition: The switch value "error" cannot be "0".
oprofile-1.0.0git/libpp/image_errors.cpp:68: dead_error_begin: Execution cannot reach this statement "case 0:".
Error: STREAM_FORMAT_STATE: [#def3]
oprofile-1.0.0git/pp/opreport.cpp:251: format_changed: "setf" changes the format state of "std::cout" for category adjustfield.
I looked through the patch and it looked okay.
-Will Cohen
>
> This patch fixes all issues found by Coverity. One particular
> change of note, however, is that the sample data format has
> changed with the removal of Cell SPU-related fields. This
> change required the bumping of the OPD_VERSION number in the
> sample file header, so that sample files created with earlier
> oprofile builds will no longer be readable by oprofile. The
> error reported by oprofile post-profiling tools would be:
> oprofpp: samples files version mismatch
>
> Signed-off-by: Maynard Johnson <[email protected]>
> ---
> libop/op_config.h | 2 +-
> libop/op_cpu_type.c | 12 +-
> libop/op_sample_file.h | 4 -
> libopagent/opagent.c | 11 ++-
> libperf_events/operf_counter.h | 1 +
> libperf_events/operf_mangling.cpp | 8 +-
> libpp/Makefile.am | 4 +-
> libpp/callgraph_container.cpp | 2 +-
> libpp/format_output.cpp | 26 +----
> libpp/populate.cpp | 7 --
> libpp/populate_for_spu.cpp | 166 --------------------------------
> libpp/populate_for_spu.h | 42 --------
> libpp/profile.cpp | 14 ---
> libpp/profile.h | 11 --
> libpp/profile_container.cpp | 9 --
> libpp/symbol.h | 4 +-
> libutil++/Makefile.am | 4 +-
> libutil++/bfd_spu_support.cpp | 117 -----------------------
> libutil++/bfd_support.h | 3 -
> libutil++/op_bfd.cpp | 7 +-
> libutil++/op_bfd.h | 18 ----
> libutil++/op_spu_bfd.cpp | 188 -------------------------------------
> pe_profiling/operf.cpp | 11 ++-
> 23 files changed, 43 insertions(+), 628 deletions(-)
> delete mode 100644 libpp/populate_for_spu.cpp
> delete mode 100644 libpp/populate_for_spu.h
> delete mode 100644 libutil++/bfd_spu_support.cpp
> delete mode 100644 libutil++/op_spu_bfd.cpp
>
> diff --git a/libop/op_config.h b/libop/op_config.h
> index 83820ea..8fe70c8 100644
> --- a/libop/op_config.h
> +++ b/libop/op_config.h
> @@ -66,7 +66,7 @@ extern char op_samples_current_dir[];
> #endif
>
> #define OPD_MAGIC "DAE\n"
> -#define OPD_VERSION 0x12
> +#define OPD_VERSION 0x13
>
> #if defined(__cplusplus)
> }
> diff --git a/libop/op_cpu_type.c b/libop/op_cpu_type.c
> index e5594e3..90fa861 100644
> --- a/libop/op_cpu_type.c
> +++ b/libop/op_cpu_type.c
> @@ -733,6 +733,7 @@ op_cpu op_get_cpu_type(void)
> op_cpu op_get_cpu_number(char const * cpu_string)
> {
> int cpu_type = CPU_NO_GOOD;
> + int scan_matches = 0;
> size_t i;
>
> for (i = 0; i < nr_cpu_descrs; ++i) {
> @@ -743,12 +744,11 @@ op_cpu op_get_cpu_number(char const * cpu_string)
> }
>
> /* Attempt to convert into a number */
> - if (cpu_type == CPU_NO_GOOD)
> - sscanf(cpu_string, "%d\n", &cpu_type);
> -
> - if (cpu_type <= CPU_NO_GOOD || cpu_type >= MAX_CPU_TYPE)
> - cpu_type = CPU_NO_GOOD;
> -
> + if (cpu_type == CPU_NO_GOOD) {
> + scan_matches = sscanf(cpu_string, "%d\n", &cpu_type);
> + if (scan_matches && (cpu_type <= CPU_NO_GOOD || cpu_type >= MAX_CPU_TYPE))
> + cpu_type = CPU_NO_GOOD;
> + }
> return cpu_type;
> }
>
> diff --git a/libop/op_sample_file.h b/libop/op_sample_file.h
> index c29191c..3637d76 100644
> --- a/libop/op_sample_file.h
> +++ b/libop/op_sample_file.h
> @@ -30,13 +30,9 @@ struct opd_header {
> double cpu_speed;
> u64 mtime;
> u32 cg_to_is_kernel;
> - /* spu_profile=1 says sample file contains Cell BE SPU profile data */
> - u32 spu_profile;
> - uint64_t embedded_offset;
> u64 anon_start;
> u64 cg_to_anon_start;
> /* binary compatibility reserve */
> - u32 reserved1[1];
> };
>
> #endif /* OP_SAMPLE_FILE_H */
> diff --git a/libopagent/opagent.c b/libopagent/opagent.c
> index c38d7df..f567f9e 100644
> --- a/libopagent/opagent.c
> +++ b/libopagent/opagent.c
> @@ -370,19 +370,24 @@ again:
> * we are called within a multi-threaded context */
> flockfile(dumpfile);
> /* Write record, symbol name, code (optionally), and (if necessary)
> - * additonal padding \0 bytes.
> + * additional padding \0 bytes.
> */
> if (fwrite_unlocked(&rec, sizeof(rec), 1, dumpfile) &&
> fwrite_unlocked(symbol_name, sz_symb_name, 1, dumpfile)) {
> + size_t sz = 0;
> if (code)
> - fwrite_unlocked(code, size, 1, dumpfile);
> + sz = fwrite_unlocked(code, size, 1, dumpfile);
> if (padding_count)
> - fwrite_unlocked(pad_bytes, padding_count, 1, dumpfile);
> + sz += fwrite_unlocked(pad_bytes, padding_count, 1, dumpfile);
> /* Always flush to ensure conversion code to elf will see
> * data as soon as possible */
> fflush_unlocked(dumpfile);
> funlockfile(dumpfile);
> flock(dumpfd, LOCK_UN);
> + if (sz != 2) {
> + printf("opagent: fwrite_unlocked failed");
> + return -1;
> + }
> return 0;
> }
> fflush_unlocked(dumpfile);
> diff --git a/libperf_events/operf_counter.h b/libperf_events/operf_counter.h
> index b37689b..4eb7775 100644
> --- a/libperf_events/operf_counter.h
> +++ b/libperf_events/operf_counter.h
> @@ -138,6 +138,7 @@ public:
> operf_read(std::vector<operf_event_t> & _evts)
> : sample_data_fd(-1), inputFname(""), evts(_evts), cpu_type(CPU_NO_GOOD)
> { valid = syswide = false;
> + write_comm_pipe = read_comm_pipe = 1;
> post_profiling_pipe = -1; }
> void init(int sample_data_pipe_fd, std::string input_filename, std::string samples_dir, op_cpu cputype,
> bool systemwide, int _record_write_pipe, int _record_read_pipe,
> diff --git a/libperf_events/operf_mangling.cpp b/libperf_events/operf_mangling.cpp
> index 6497802..0c160d7 100644
> --- a/libperf_events/operf_mangling.cpp
> +++ b/libperf_events/operf_mangling.cpp
> @@ -109,8 +109,7 @@ mangle_filename(struct operf_sfile * last, struct operf_sfile const * sf, int co
>
> static void fill_header(struct opd_header * header, unsigned long counter,
> vma_t anon_start, vma_t cg_to_anon_start,
> - int is_kernel, int cg_to_is_kernel,
> - int spu_samples, uint64_t embed_offset, time_t mtime)
> + int is_kernel, int cg_to_is_kernel, time_t mtime)
> {
> const operf_event_t * event = operfRead.get_event_by_counter(counter);
>
> @@ -126,8 +125,6 @@ static void fill_header(struct opd_header * header, unsigned long counter,
> header->cpu_speed = cpu_speed;
> header->mtime = mtime;
> header->anon_start = anon_start;
> - header->spu_profile = spu_samples;
> - header->embedded_offset = embed_offset;
> header->cg_to_anon_start = cg_to_anon_start;
> }
>
> @@ -204,8 +201,7 @@ retry:
>
> fill_header((struct opd_header *)odb_get_data(file), counter,
> sf->is_anon ? sf->start_addr : 0, last_start,
> - !!sf->kernel, last ? !!last->kernel : 0,
> - 0, 0, mtime);
> + !!sf->kernel, last ? !!last->kernel : 0, mtime);
>
> out:
> operf_sfile_put(sf);
> diff --git a/libpp/Makefile.am b/libpp/Makefile.am
> index d3fcbdd..79c415e 100644
> --- a/libpp/Makefile.am
> +++ b/libpp/Makefile.am
> @@ -52,7 +52,5 @@ libpp_a_SOURCES = \
> symbol_sort.cpp \
> symbol_sort.h \
> xml_utils.h \
> - xml_utils.cpp \
> - populate_for_spu.cpp \
> - populate_for_spu.h
> + xml_utils.cpp
>
> diff --git a/libpp/callgraph_container.cpp b/libpp/callgraph_container.cpp
> index c5bc272..2affa8f 100644
> --- a/libpp/callgraph_container.cpp
> +++ b/libpp/callgraph_container.cpp
> @@ -485,7 +485,7 @@ void callgraph_container::populate(list<string> const & cg_files,
>
> bool callee_bfd_ok = true;
> if (strncmp(callee_file.cg_image.c_str(), KALL_SYM_FILE,
> - strlen(callee_file.lib_image.c_str())) == 0)
> + strlen(callee_file.cg_image.c_str())) == 0)
> callee_bfd = new op_bfd(callee_file.cg_image, extra_found_images);
>
> else
> diff --git a/libpp/format_output.cpp b/libpp/format_output.cpp
> index 315fad2..11cd396 100644
> --- a/libpp/format_output.cpp
> +++ b/libpp/format_output.cpp
> @@ -665,26 +665,12 @@ xml_formatter::get_bfd_object(symbol_entry const * symb, op_bfd * & abfd) const
> bool ok = true;
>
> string const & image_name = get_image_name(symb->image_name,
> - image_name_storage::int_filename, extra_found_images);
> - if (symb->spu_offset) {
> - // FIXME: what about archive:tmp, actually it's not supported
> - // for spu since oparchive doesn't archive the real file but
> - // in future it would work ?
> - string tmp = get_image_name(symb->embedding_filename,
> - image_name_storage::int_filename, extra_found_images);
> - if (abfd && abfd->get_filename() == tmp)
> - return true;
> - delete abfd;
> - abfd = new op_bfd(symb->spu_offset, tmp,
> - symbol_filter, extra_found_images, ok);
> - } else {
> - if (abfd && abfd->get_filename() == image_name)
> - return true;
> - delete abfd;
> - abfd = new op_bfd(image_name, symbol_filter,
> - extra_found_images, ok);
> -
> - }
> + image_name_storage::int_filename, extra_found_images);
> + if (abfd && abfd->get_filename() == image_name)
> + return true;
> + delete abfd;
> + abfd = new op_bfd(image_name, symbol_filter,
> + extra_found_images, ok);
>
> if (!ok) {
> report_image_error(image_name, image_format_failure,
> diff --git a/libpp/populate.cpp b/libpp/populate.cpp
> index 22a4e20..bd49276 100644
> --- a/libpp/populate.cpp
> +++ b/libpp/populate.cpp
> @@ -18,7 +18,6 @@
> #include "op_bfd.h"
> #include "op_header.h"
> #include "populate.h"
> -#include "populate_for_spu.h"
>
> #include "image_errors.h"
> #include "utility.h"
> @@ -63,12 +62,6 @@ populate_for_image(profile_container & samples, inverted_profile const & ip,
> {
> op_bfd *abfd;
>
> - if (is_spu_profile(ip)) {
> - populate_for_spu_image(samples, ip, symbol_filter,
> - has_debug_info);
> - return;
> - }
> -
> bool ok = ip.error == image_ok;
>
> if (strncmp(ip.image.c_str(), KALL_SYM_FILE, strlen(ip.image.c_str())) == 0)
> diff --git a/libpp/populate_for_spu.cpp b/libpp/populate_for_spu.cpp
> deleted file mode 100644
> index 0f4606b..0000000
> --- a/libpp/populate_for_spu.cpp
> +++ /dev/null
> @@ -1,166 +0,0 @@
> -/**
> - * @file libpp/populate_for_spu.cpp
> - * Fill up a profile_container from inverted profiles for
> - * a Cell BE SPU profile
> - *
> - * @remark Copyright 2007 OProfile authors
> - * @remark Read the file COPYING
> - *
> - * @author Maynard Johnson
> - * (C) Copyright IBM Corporation 2007
> - */
> -
> -#include "profile.h"
> -#include "profile_container.h"
> -#include "arrange_profiles.h"
> -#include "op_bfd.h"
> -#include "op_header.h"
> -#include "populate.h"
> -#include "populate_for_spu.h"
> -
> -#include "image_errors.h"
> -
> -#include <iostream>
> -
> -using namespace std;
> -
> -namespace {
> -
> -static int spu_profile = unknown_profile;
> -
> -/*
> - * On Cell Broadband Engine, an application executing on an SPE may
> - * have been loaded from a separate SPU executable binary file or may
> - * have been loaded from an embedded section of a PPE application or
> - * shared library. In the embedded case, the embedding file may actually
> - * contain multiple SPU images, resulting in different SPU images being loaded
> - * onto different SPUs. Thus, the SPUs may be executing different code, even
> - * though the application of the parent PPE process is the same. Therefore,
> - * we must be sure to create a separate op_bfd object for each SPU. When doing
> - * so below, we examine header.embedded_offset. If embedded_offset is > 0, it's
> - * interpreted as the offset of an SPU image embedded in the containing file,
> - * so the filename to do the check_mtime on is the containing file, ip.image;
> - * otherwise, the filename to do the check_mtime on is the separate backing
> - * file of the SPU image, abfd->filename.
> - */
> -void
> -populate_spu_profile_from_files(list<profile_sample_files> const & files,
> - string const app_image,
> - profile_container & samples,
> - inverted_profile const & ip,
> - string_filter const & symbol_filter,
> - size_t ip_grp_num, bool * has_debug_info)
> -{
> - string archive_path = samples.extra_found_images.get_archive_path();
> - bool ok = ip.error == image_ok;
> - op_bfd * abfd = NULL;
> - string fname_to_check;
> - list<profile_sample_files>::const_iterator it = files.begin();
> - list<profile_sample_files>::const_iterator const end = files.end();
> - for (; it != end; ++it) {
> - profile_t profile;
> - if (it->sample_filename.empty())
> - continue;
> -
> - profile.add_sample_file(it->sample_filename);
> - opd_header header = profile.get_header();
> - if (header.embedded_offset) {
> - abfd = new op_bfd(header.embedded_offset,
> - ip.image,
> - symbol_filter,
> - samples.extra_found_images,
> - ok);
> - fname_to_check = ip.image;
> - } else {
> - abfd = new op_bfd(ip.image,
> - symbol_filter,
> - samples.extra_found_images,
> - ok);
> - fname_to_check = abfd->get_filename();
> - }
> - profile.set_offset(*abfd);
> - if (!ok && ip.error == image_ok)
> - ip.error = image_format_failure;
> -
> - if (ip.error == image_format_failure)
> - report_image_error(ip, false,
> - samples.extra_found_images);
> -
> - samples.add(profile, *abfd, app_image, ip_grp_num);
> - if (ip.error == image_ok) {
> - image_error error;
> - string filename =
> - samples.extra_found_images.find_image_path(
> - fname_to_check, error, true);
> - check_mtime(filename, profile.get_header());
> - }
> -
> - if (has_debug_info && !*has_debug_info)
> - *has_debug_info = abfd->has_debug_info();
> - delete abfd;
> - }
> -}
> -} // anon namespace
> -
> -void
> -populate_for_spu_image(profile_container & samples,
> - inverted_profile const & ip,
> - string_filter const & symbol_filter,
> - bool * has_debug_info)
> -{
> -
> - for (size_t i = 0; i < ip.groups.size(); ++i) {
> - list < image_set >::const_iterator it=
> - ip.groups[i].begin();
> - list < image_set >::const_iterator const end
> - = ip.groups[i].end();
> -
> - for (; it != end; ++it)
> - populate_spu_profile_from_files(it->files,
> - it->app_image, samples, ip,
> - symbol_filter, i, has_debug_info);
> - }
> -}
> -
> -bool is_spu_profile(inverted_profile const & ip)
> -{
> - bool retval = false;
> - string sfname = "";
> - if (spu_profile != unknown_profile)
> - return spu_profile;
> -
> - if (!ip.groups.size())
> - return false;
> -
> - for (size_t i = 0; i < ip.groups.size(); ++i) {
> - list<image_set>::const_iterator grp_it
> - = ip.groups[i].begin();
> - list<image_set>::const_iterator const grp_end
> - = ip.groups[i].end();
> -
> - for (; grp_it != grp_end; ++grp_it) {
> - list<profile_sample_files>::const_iterator sfiles_it =
> - grp_it->files.begin();
> - list<profile_sample_files>::const_iterator sfiles_end =
> - grp_it->files.end();
> - for (; sfiles_it != sfiles_end; ++sfiles_it) {
> - if (!sfiles_it->sample_filename.empty()) {
> - sfname = sfiles_it->sample_filename;
> - goto do_check;
> - }
> - }
> - }
> - }
> - goto out;
> -
> -do_check:
> - spu_profile = profile_t::is_spu_sample_file(sfname);
> -
> - if (spu_profile == cell_spu_profile)
> - retval = true;
> -
> -out:
> - return retval;
> -}
> -
> -
> diff --git a/libpp/populate_for_spu.h b/libpp/populate_for_spu.h
> deleted file mode 100644
> index ec48099..0000000
> --- a/libpp/populate_for_spu.h
> +++ /dev/null
> @@ -1,42 +0,0 @@
> -/**
> - * @file libpp/populate_for_spu.h
> - * Fill up a profile_container from inverted profiles for
> - * a Cell BE SPU profile
> - *
> - * @remark Copyright 2007 OProfile authors
> - * @remark Read the file COPYING
> - *
> - * @author Maynard Johnson
> - * (C) Copyright IBM Corporation 2007
> - */
> -
> -#ifndef POPULATE_FOR_SPU_H
> -#define POPULATE_FOR_SPU_H
> -
> -class profile_container;
> -class inverted_profile;
> -class string_filter;
> -
> -/*
> - * When profiling SPUs on Cell Broadband Engine, all sample file
> - * headers get a flag set indicating "spu_profile". This function
> - * checks the first sample file for this indicator.
> - */
> -bool is_spu_profile(inverted_profile const & ip);
> -
> -/*
> - * This is a special-purpose function for CELL BE SPU profiling.
> - * See populate_spu_profile_from_files prologue for more details.
> - */
> -void populate_for_spu_image(profile_container & samples,
> - inverted_profile const & ip,
> - string_filter const & symbol_filter,
> - bool * has_debug_info);
> -
> -enum profile_type {
> - unknown_profile = -1,
> - normal_profile,
> - cell_spu_profile
> -};
> -
> -#endif /* POPULATE_FOR_SPU_H */
> diff --git a/libpp/profile.cpp b/libpp/profile.cpp
> index f117508..8e80e98 100644
> --- a/libpp/profile.cpp
> +++ b/libpp/profile.cpp
> @@ -27,7 +27,6 @@
> #include "profile.h"
> #include "op_bfd.h"
> #include "cverb.h"
> -#include "populate_for_spu.h"
>
> using namespace std;
>
> @@ -57,19 +56,6 @@ count_type profile_t::sample_count(string const & filename)
> }
>
> //static member
> -enum profile_type profile_t::is_spu_sample_file(string const & filename)
> -{
> - profile_type retval;
> - odb_t samples_db;
> - open_sample_file(filename, samples_db);
> - opd_header const & hdr =
> - *static_cast<opd_header *>(odb_get_data(&samples_db));
> - retval = hdr.spu_profile ? cell_spu_profile: normal_profile;
> - odb_close(&samples_db);
> - return retval;
> -}
> -
> -//static member
> void profile_t::open_sample_file(string const & filename, odb_t & db)
> {
> // Check first if the sample file version is ok else odb_open() can
> diff --git a/libpp/profile.h b/libpp/profile.h
> index 3f0c21f..78cda72 100644
> --- a/libpp/profile.h
> +++ b/libpp/profile.h
> @@ -20,7 +20,6 @@
> #include "odb.h"
> #include "op_types.h"
> #include "utility.h"
> -#include "populate_for_spu.h"
>
> class opd_header;
> class op_bfd;
> @@ -56,16 +55,6 @@ public:
> static count_type sample_count(std::string const & filename);
>
> /**
> - * Indicate if given sample file is from a Cell Broadband Engine
> - * SPU profile
> - * @param filename sample filename
> - *
> - * Convenience interface put here so all access to samples files
> - * go through profile_t static or non static member.
> - */
> - static enum profile_type is_spu_sample_file(std::string const & filename);
> -
> - /**
> * cumulate sample file to our container of samples
> * @param filename sample file name
> *
> diff --git a/libpp/profile_container.cpp b/libpp/profile_container.cpp
> index e543f51..35c63a1 100644
> --- a/libpp/profile_container.cpp
> +++ b/libpp/profile_container.cpp
> @@ -24,7 +24,6 @@
> #include "profile_container.h"
> #include "sample_container.h"
> #include "symbol_container.h"
> -#include "populate_for_spu.h"
> #include "cverb.h"
>
> using namespace std;
> @@ -118,14 +117,6 @@ void profile_container::add(profile_t const & profile,
> symb_entry.app_name = image_names.create(app_name);
>
> symb_entry.sample.vma = abfd.syms[i].vma();
> - if ((header.spu_profile == cell_spu_profile) &&
> - header.embedded_offset) {
> - symb_entry.spu_offset = header.embedded_offset;
> - symb_entry.embedding_filename =
> - image_names.create(abfd.get_embedding_filename());
> - } else {
> - symb_entry.spu_offset = 0;
> - }
> symbol_entry const * symbol = symbols->insert(symb_entry);
>
> if (need_details)
> diff --git a/libpp/symbol.h b/libpp/symbol.h
> index 07f9fd4..017e04c 100644
> --- a/libpp/symbol.h
> +++ b/libpp/symbol.h
> @@ -62,7 +62,7 @@ struct sample_entry {
> /// associate a symbol with a file location, samples count and vma address
> class symbol_entry {
> public:
> - symbol_entry() : sym_index(0), size(0), spu_offset(0), vma_adj(0) {}
> + symbol_entry() : sym_index(0), size(0), vma_adj(0) {}
> virtual ~symbol_entry() {}
>
> /// which image this symbol belongs to
> @@ -88,8 +88,6 @@ public:
> * the start is below it, but the the hint is only used for formatting
> */
> column_flags output_hint(column_flags fl) const;
> - uint64_t spu_offset;
> - image_name_id embedding_filename;
>
> /**
> * The vma_adj is set according to the corresponding op_bfd::vma_adj.
> diff --git a/libutil++/Makefile.am b/libutil++/Makefile.am
> index cfd8551..2f2f87b 100644
> --- a/libutil++/Makefile.am
> +++ b/libutil++/Makefile.am
> @@ -40,6 +40,4 @@ libutil___a_SOURCES = \
> cached_value.h \
> comma_list.h \
> xml_output.h \
> - xml_output.cpp \
> - bfd_spu_support.cpp \
> - op_spu_bfd.cpp
> + xml_output.cpp
> diff --git a/libutil++/bfd_spu_support.cpp b/libutil++/bfd_spu_support.cpp
> deleted file mode 100644
> index fc31d6a..0000000
> --- a/libutil++/bfd_spu_support.cpp
> +++ /dev/null
> @@ -1,117 +0,0 @@
> -/**
> - * @file libutil++/bfd_spu_support.cpp
> - * Special BFD functions for processing a Cell BE SPU profile
> - *
> - * @remark Copyright 2007 OProfile authors
> - * @remark Read the file COPYING
> - *
> - * @author Maynard Johnson
> - * (C) Copyright IBM Corporation 2007
> - */
> -
> -#include "bfd_support.h"
> -#include "op_bfd.h"
> -#include "config.h"
> -#include "cverb.h"
> -
> -#include <stdlib.h>
> -#include <stdio.h>
> -#include <iostream>
> -#include <fstream>
> -#include <sstream>
> -#include <string>
> -#include <cstring>
> -#include <sys/types.h>
> -
> -struct spu_elf {
> - FILE * stream;
> - off_t spu_offset;
> -};
> -
> -using namespace std;
> -
> -extern verbose vbfd;
> -
> -#ifdef HAVE_BFD_OPENR_IOVEC_WITH_7PARMS
> -
> -namespace {
> -
> -static void *
> -spu_bfd_iovec_open(bfd * nbfd, void * open_closure)
> -{
> - /* Checking nbfd isn't really necessary, except to silence
> - * compile warning. In fact, nbfd will always be non-NULL.
> - */
> - if (nbfd)
> - return open_closure;
> - else
> - return NULL;
> -}
> -
> -static int
> -spu_bfd_iovec_close(bfd * nbfd, void * stream)
> -{
> - spu_elf * my_stream = (spu_elf *) stream;
> -
> - fclose(my_stream->stream);
> - free(my_stream);
> - /* Checking nbfd isn't really necessary, except to silence
> - * compile warning. In fact, nbfd will always be non-NULL.
> - */
> - if (nbfd)
> - return 1;
> - else
> - return 0;
> -}
> -
> -static file_ptr
> -spu_bfd_iovec_pread(bfd * abfd, void * stream, void * buf,
> - file_ptr nbytes, file_ptr offset)
> -{
> - spu_elf * my_stream = (spu_elf *) stream;
> - if (fseek(my_stream->stream, my_stream->spu_offset + offset,
> - SEEK_SET) < 0)
> - return 0;
> - nbytes = fread(buf, sizeof(char), nbytes, my_stream->stream);
> - /* Checking abfd isn't really necessary, except to silence
> - * compile warning. In fact, abfd will always be non-NULL.
> - */
> - if (abfd)
> - return nbytes;
> - else
> - return 0;
> -}
> -} // namespace anon
> -#endif
> -
> -bfd *
> -spu_open_bfd(string const name, int fd, uint64_t offset_to_spu_elf)
> -{
> -
> - bfd * nbfd = NULL;
> - spu_elf * spu_elf_stream = (spu_elf *)malloc(sizeof(spu_elf));
> -
> - FILE * fp = fdopen(fd, "r");
> - spu_elf_stream->stream = fp;
> - spu_elf_stream->spu_offset = offset_to_spu_elf;
> -#ifdef HAVE_BFD_OPENR_IOVEC_WITH_7PARMS
> - nbfd = bfd_openr_iovec(strdup(name.c_str()), "elf32-spu",
> - spu_bfd_iovec_open, spu_elf_stream,
> - spu_bfd_iovec_pread, spu_bfd_iovec_close, NULL);
> -#else
> - ostringstream os;
> - os << "Attempt to process a Cell Broadband Engine SPU profile without"
> - << "proper BFD support.\n"
> - << "Rebuild the opreport utility with the correct BFD library.\n"
> - << "See the OProfile user manual for more information.\n";
> - throw op_runtime_error(os.str());
> -#endif
> - if (!nbfd) {
> - cverb << vbfd << "spu_open_bfd failed for " << name << endl;
> - return NULL;
> - }
> -
> - bfd_check_format(nbfd, bfd_object);
> -
> - return nbfd;
> -}
> diff --git a/libutil++/bfd_support.h b/libutil++/bfd_support.h
> index e71d4c4..6ff4b25 100644
> --- a/libutil++/bfd_support.h
> +++ b/libutil++/bfd_support.h
> @@ -132,9 +132,6 @@ bfd * open_bfd(std::string const & file);
> /// open the given BFD from the fd
> bfd * fdopen_bfd(std::string const & file, int fd);
>
> -/// Return a BFD for an SPU ELF embedded in PPE binary file
> -bfd * spu_open_bfd(std::string const name, int fd, uint64_t offset_to_spu_elf);
> -
> /// Return true if the symbol is worth looking at
> bool interesting_symbol(asymbol * sym);
>
> diff --git a/libutil++/op_bfd.cpp b/libutil++/op_bfd.cpp
> index c92d532..389c920 100644
> --- a/libutil++/op_bfd.cpp
> +++ b/libutil++/op_bfd.cpp
> @@ -356,8 +356,10 @@ void op_bfd::get_kallsym_symbols(symbols_found_t & symbols, ifstream& infile)
> /* Add symbols */
> copy(symbols.begin(), symbols.end(), back_inserter(syms));
>
> - cverb << vbfd << "Kallsyms, number of symbols now "
> - << dec << syms.size() << hex << endl;
> + ostringstream msg;
> + msg << "Kallsyms, number of symbols now "
> + << dec << syms.size() << hex << endl;
> + cverb << vbfd << msg.str();
> return;
> }
>
> @@ -378,6 +380,7 @@ op_bfd::op_bfd(string const & fname, extra_images const & extra_images)
> {
> symbols_found_t symbols;
> ifstream infile;
> + fd = -1;
>
> ibfd.abfd = (bfd * ) NULL;
>
> diff --git a/libutil++/op_bfd.h b/libutil++/op_bfd.h
> index 1aa7e10..c7092b0 100644
> --- a/libutil++/op_bfd.h
> +++ b/libutil++/op_bfd.h
> @@ -111,24 +111,12 @@ public:
> bool & ok);
>
> /**
> - * This constructor is used when processing an SPU profile
> - * where the SPU ELF is embedded within the PPE binary.
> - */
> - op_bfd(uint64_t spu_offset,
> - std::string const & filename,
> - string_filter const & symbol_filter,
> - extra_images const & extra_images,
> - bool & ok);
> -
> - /**
> * This constructor is used when the /proc/kallsyms file is used
> * to get the kernel symbols.
> */
> op_bfd(std::string const & filename,
> extra_images const & extra_images);
>
> - std::string get_embedding_filename() const { return embedding_filename; }
> -
> /// close an opened bfd image and free all related resources
> ~op_bfd();
>
> @@ -304,12 +292,6 @@ private:
> // mapping of section names to filepos in the original binary
> filepos_map_t filepos_map;
>
> - /**
> - * If spu_offset is non-zero, embedding_filename is the file containing
> - * the embedded SPU image.
> - */
> - std::string embedding_filename;
> -
> bool anon_obj;
>
> /**
> diff --git a/libutil++/op_spu_bfd.cpp b/libutil++/op_spu_bfd.cpp
> deleted file mode 100644
> index 29d6e06..0000000
> --- a/libutil++/op_spu_bfd.cpp
> +++ /dev/null
> @@ -1,188 +0,0 @@
> -/**
> - * @file libutil++/op_spu_bfd.cpp
> - * Encapsulation of bfd objects for Cell BE SPU
> - *
> - * @remark Copyright 2007 OProfile authors
> - * @remark Read the file COPYING
> - *
> - * @author Maynard Johnson
> - * (C) Copyright IBM Corporation 2007
> - */
> -
> -
> -#include <fcntl.h>
> -#include <sys/stat.h>
> -#include <unistd.h>
> -#include <cstdlib>
> -#include <cstring>
> -
> -#include <iostream>
> -#include <cstring>
> -#include <cstdlib>
> -
> -#include "op_bfd.h"
> -#include "locate_images.h"
> -#include "op_libiberty.h"
> -#include "string_filter.h"
> -#include "cverb.h"
> -
> -#define OP_SPU_DYN_FLAG 0x10000000 /* kernel module adds this offset */
> - /* to SPU code it can't find in the map */
> -#define OP_SPU_MEMSIZE 0x3ffff /* Physical memory size on an SPU */
> -
> -using namespace std;
> -
> -extern verbose vbfd;
> -
> -/*
> - * This overload of the op_bfd constructor is patterned after the
> - * constructor in libutil++/op_bfd.cpp, with the additional processing
> - * needed to handle an embedded spu offset.
> - */
> -op_bfd::op_bfd(uint64_t spu_offset, string const & fname,
> - string_filter const & symbol_filter,
> - extra_images const & extra_images, bool & ok)
> - :
> - archive_path(extra_images.get_archive_path()),
> - extra_found_images(extra_images),
> - file_size(-1),
> - embedding_filename(fname),
> - anon_obj(false),
> - vma_adj(0)
> -{
> - fd = -1;
> - struct stat st;
> - int notes_remaining;
> - bool spu_note_found = false;
> - size_t sec_size = 0;
> - unsigned int oct_per_byte;
> - asection * note = NULL;
> -
> - symbols_found_t symbols;
> - asection const * sect;
> -
> - image_error image_ok;
> - string const image_path =
> - extra_images.find_image_path(fname, image_ok, true);
> -
> - cverb << vbfd << "op_bfd ctor for " << image_path << endl;
> - if (!ok)
> - goto out_fail;
> -
> - fd = open(image_path.c_str(), O_RDONLY);
> - if (fd == -1) {
> - cverb << vbfd << "open failed for " << image_path << endl;
> - ok = false;
> - goto out_fail;
> - }
> -
> - if (fstat(fd, &st)) {
> - cverb << vbfd << "stat failed for " << image_path << endl;
> - ok = false;
> - goto out_fail;
> - }
> -
> - file_size = st.st_size;
> - ibfd.abfd = spu_open_bfd(image_path, fd, spu_offset);
> - if (!ibfd.valid()) {
> - cverb << vbfd << "fdopen_bfd failed for " << image_path << endl;
> - ok = false;
> - goto out_fail;
> - }
> -
> - /* For embedded SPU ELF, a note section named '.note.spu_name'
> - * contains the name of the SPU binary image in the description
> - * field.
> - */
> - note = bfd_get_section_by_name(ibfd.abfd, ".note.spu_name");
> - if (!note) {
> - cverb << vbfd << "No .note.spu-name section found" << endl;
> - goto find_sec_code;
> - }
> - cverb << vbfd << "found .note.spu_name section" << endl;
> -
> - bfd_byte * sec_contents;
> - oct_per_byte = bfd_octets_per_byte(ibfd.abfd);
> - sec_size = bfd_section_size(ibfd.abfd, note)/oct_per_byte;
> -
> - sec_contents = (bfd_byte *) xmalloc(sec_size);
> - if (!bfd_get_section_contents(ibfd.abfd, note, sec_contents,
> - 0, sec_size)) {
> - cverb << vbfd << "bfd_get_section_contents with size "
> - << sec_size << " returned an error" << endl;
> - ok = false;
> - goto out_fail;
> - }
> - notes_remaining = sec_size;
> - while (notes_remaining && !spu_note_found) {
> - unsigned int nsize, dsize, type;
> - nsize = *((unsigned int *) sec_contents);
> - dsize = *((unsigned int *) sec_contents +1);
> - type = *((unsigned int *) sec_contents +2);
> - int remainder, desc_start, name_pad_length, desc_pad_length;
> - name_pad_length = desc_pad_length = 0;
> - /* Calculate padding for 4-byte alignment */
> - remainder = nsize % 4;
> - if (remainder != 0)
> - name_pad_length = 4 - remainder;
> - desc_start = 12 + nsize + name_pad_length;
> - if (type != 1) {
> - int note_record_length;
> - if ((remainder = (dsize % 4)) != 0)
> - desc_pad_length = 4 - remainder;
> - note_record_length = 12 + nsize +
> - name_pad_length + dsize + desc_pad_length;
> - notes_remaining -= note_record_length;
> - sec_contents += note_record_length;
> - continue;
> - } else {
> - spu_note_found = true;
> - /* Must memcpy the data from sec_contents to a
> - * 'char *' first, then stringify it, since
> - * the type of sec_contents (bfd_byte *) cannot be
> - * used as input for creating a string.
> - */
> - char * description = (char *) xmalloc(dsize);
> - memcpy(description, sec_contents + desc_start, dsize);
> - filename = description;
> - free(description);
> - }
> - }
> - free(sec_contents);
> - /* Default to app name for the image name */
> - if (spu_note_found == false)
> - filename = fname;
> -
> -find_sec_code:
> - for (sect = ibfd.abfd->sections; sect; sect = sect->next) {
> - if (sect->flags & SEC_CODE) {
> - if (filepos_map[sect->name] != 0) {
> - cerr << "Found section \"" << sect->name
> - << "\" twice for " << get_filename()
> - << endl;
> - abort();
> - }
> -
> - filepos_map[sect->name] = sect->filepos;
> - }
> - }
> -
> - get_symbols(symbols);
> -
> - /* In some cases the SPU library code generates code stubs on the stack. */
> - /* The kernel module remaps those addresses so add an entry to catch/report them. */
> - symbols.push_back(op_bfd_symbol(OP_SPU_DYN_FLAG, OP_SPU_MEMSIZE,
> - "__send_to_ppe(stack)"));
> -
> -out:
> - add_symbols(symbols, symbol_filter);
> - return;
> -out_fail:
> - ibfd.close();
> - dbfd.close();
> - if (fd != -1)
> - close(fd);
> - file_size = -1;
> - goto out;
> -}
> -
> diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp
> index 0663022..87245ba 100644
> --- a/pe_profiling/operf.cpp
> +++ b/pe_profiling/operf.cpp
> @@ -418,7 +418,16 @@ int start_profiling(void)
>
> fail_out:
> if (operfRecord)
> - delete operfRecord;
> + try {
> + delete operfRecord;
> + } catch (const runtime_error & re) {
> + // We're already in failure mode here; if we get a runtime_error while
> + // deleting operfRecord, we'll only print it if user requests "-V misc"
> + if (cverb << vmisc) {
> + cerr << "Caught runtime_error: " << re.what() << endl;
> + exit_code = EXIT_FAILURE;
> + }
> + }
>
> if (!ready){
> /* ready==0 means we've not yet told parent we're ready,
>
------------------------------------------------------------------------------