From: Maynard J. <may...@us...> - 2013-05-20 22:42:06
|
Fix Coverity errors found on May 20, 2013 git snapshot Coverity identified the following errors on scans run from May 7 through May 20, 2013: Type,Category,File,Function Wrapper object use after free,Memory - illegal accesses,/agents/jvmpi/jvmpi_oprofile.cpp,compiled_method_load(JVMPI_Event *) Unchecked return value,Error handling issues,/daemon/opd_mangling.c,opd_open_sample_file Dereference after null check,Null pointer dereferences,/daemon/opd_sfile.c,sfile_hash Uninitialized scalar field,Uninitialized members,/gui/oprof_start_config.cpp,config_setting::config_setting() Division or modulo by zero,Integer handling issues,/libdb/db_stat.c,odb_hash_stat Resource leak,Resource leaks,/libop/op_cpu_type.c,_auxv_fetch Resource leak,Resource leaks,/libop/op_cpu_type.c,fetch_at_hw_platform Negative array index read,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask Write to pointer after free,Memory - corruptions,/libop/op_events.c,read_events Read from pointer after free,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename Time of check time of use,Security best practices violations,/libopagent/opagent.c,op_open_agent Improper use of negative value,Integer handling issues,/libperf_events/operf_counter.cpp,operf_record::setup() Double free,Memory - corruptions,/libperf_events/operf_counter.cpp,operf_record::setup() Uninitialized pointer read,Memory - illegal accesses,/libperf_events/operf_counter.cpp,<unnamed>::_get_perf_event_from_file(mmap_info &) Unchecked return value,Error handling issues,/libperf_events/operf_mangling.cpp,"operf_open_sample_file(odb_t *, operf_sfile *, operf_sfile *, int, int)" Using invalid iterator,API usage errors,/libperf_events/operf_process_info.cpp,operf_process_info::try_disassociate_from_parent(char *) Non-array delete for scalars,Memory - illegal accesses,/libregex/op_regex.cpp,"<unnamed>::op_regerror(int, const re_pattern_buffer &)" Resource leak,Resource leaks,/libutil++/op_bfd.cpp,"op_bfd::op_bfd(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const string_filter &, const extra_images &, bool &)" Explicit null dereferenced,Null pointer dereferences,/opjitconv/create_bfd.c,fill_symtab Resource leak,Resource leaks,/opjitconv/opjitconv.c,_cleanup_jitdumps Use of untrusted string value,Insecure data handling,/opjitconv/opjitconv.c,main Resource leak,Resource leaks,/pe_profiling/operf.cpp,_get_cpu_for_perf_events_cap() Dereference null return value,Null pointer dereferences,/pe_profiling/operf.cpp,_process_session_dir() Incorrect deallocator used,API usage errors,/pe_profiling/operf.cpp,_process_events_list() =============================== This patch fixes those errors. Signed-off-by: Maynard Johnson <may...@us...> --- agents/jvmpi/jvmpi_oprofile.cpp | 4 +- daemon/opd_mangling.c | 7 +++++- gui/oprof_start_config.cpp | 2 + libdb/db_stat.c | 3 +- libop/op_cpu_type.c | 12 ++++++++++ libop/op_events.c | 10 ++++---- libop/op_mangle.c | 2 +- libopagent/opagent.c | 40 +++++++++++++++++++++++++++----- libperf_events/operf_counter.cpp | 13 ++++++---- libperf_events/operf_mangling.cpp | 6 ++++- libperf_events/operf_process_info.cpp | 4 +- opjitconv/opjitconv.c | 13 ++++++++-- pe_profiling/operf.cpp | 7 +++++- pp/opreport.cpp | 7 +++-- 14 files changed, 98 insertions(+), 32 deletions(-) diff --git a/agents/jvmpi/jvmpi_oprofile.cpp b/agents/jvmpi/jvmpi_oprofile.cpp index 800667f..9ae5cda 100644 --- a/agents/jvmpi/jvmpi_oprofile.cpp +++ b/agents/jvmpi/jvmpi_oprofile.cpp @@ -106,13 +106,13 @@ void compiled_method_load(JVMPI_Event * event) throw runtime_error("Error: Cannot find method name for " "compiled method\n"); } - char const * method_name = ((string)method_it->second).c_str(); + char const * method_name = method_it->second.c_str(); method_it = cls_info.method_signatures.find(method); if (method_it == cls_info.method_signatures.end()) { throw runtime_error("Error: Cannot find method signature " "for compiled method\n"); } - char const * method_signature = ((string)method_it->second).c_str(); + char const * method_signature = method_it->second.c_str(); string const class_signature = "L" + cls_info.name + ";"; pthread_mutex_unlock(&class_map_mutex); diff --git a/daemon/opd_mangling.c b/daemon/opd_mangling.c index 201eabf..9d14dc4 100644 --- a/daemon/opd_mangling.c +++ b/daemon/opd_mangling.c @@ -157,7 +157,12 @@ int opd_open_sample_file(odb_t *file, struct sfile *last, verbprintf(vsfile, "Opening \"%s\"\n", mangled); - create_path(mangled); + err = create_path(mangled); + if (err) { + fprintf(stderr, "oprofiled: create path for %s failed: %s\n", + mangled, strerror(err)); + goto out; + } /* locking sf will lock associated cg files too */ sfile_get(sf); diff --git a/gui/oprof_start_config.cpp b/gui/oprof_start_config.cpp index 02bfe64..3165b85 100644 --- a/gui/oprof_start_config.cpp +++ b/gui/oprof_start_config.cpp @@ -34,6 +34,8 @@ event_setting::event_setting() config_setting::config_setting() : + buffer_size(0), + note_table_size(0), no_kernel(false), verbose(false), separate_lib(false), diff --git a/libdb/db_stat.c b/libdb/db_stat.c index 6d29e9a..c85d93a 100644 --- a/libdb/db_stat.c +++ b/libdb/db_stat.c @@ -65,7 +65,8 @@ odb_hash_stat_t * odb_hash_stat(odb_t const * odb) } result->max_list_length = max_length; - result->average_list_length = total_length / nr_non_empty_list; + result->average_list_length = (!nr_non_empty_list) ? 0 + : total_length / nr_non_empty_list; return result; } diff --git a/libop/op_cpu_type.c b/libop/op_cpu_type.c index 8962e97..ba81fca 100644 --- a/libop/op_cpu_type.c +++ b/libop/op_cpu_type.c @@ -198,6 +198,7 @@ static ElfW(auxv_t) * _auxv_fetch() auxv_temp = (ElfW(auxv_t) *)malloc(page_size); if (!auxv_temp) { perror("Allocation of space for auxv failed."); + close(auxv_f); return NULL; } bytes = read(auxv_f, (void *)auxv_temp, page_size); @@ -214,6 +215,7 @@ static ElfW(auxv_t) * _auxv_fetch() fprintf(stderr, "Recoverable error. Continuing.\n"); } } + auxv_buf = auxv_temp; } return (ElfW(auxv_t) *)auxv_temp; } @@ -239,6 +241,14 @@ static const char * fetch_at_hw_platform(ElfW(Addr) type) return platform; } +static void release_at_hw_platform(void) +{ + ElfW(auxv_t) * my_auxv = NULL; + + if ((my_auxv = (ElfW(auxv_t)*) _auxv_fetch())) + free(my_auxv); +} + static op_cpu _try_ppc64_arch_generic_cpu(void) { const char * platform, * base_platform; @@ -249,6 +259,7 @@ static op_cpu _try_ppc64_arch_generic_cpu(void) if (!platform || !base_platform) { fprintf(stderr, "NULL returned for one or both of AT_PLATFORM/AT_BASE_PLATFORM\n"); fprintf(stderr, "AT_PLATFORM: %s; \tAT_BASE_PLATFORM: %s\n", platform, base_platform); + release_at_hw_platform(); return cpu_type; } // FIXME whenever a new IBM Power processor is added -- need to ensure @@ -271,6 +282,7 @@ static op_cpu _try_ppc64_arch_generic_cpu(void) cpu_type = CPU_PPC64_ARCH_V1; } } + release_at_hw_platform(); return cpu_type; } diff --git a/libop/op_events.c b/libop/op_events.c index e5ecbcc..5b86ca3 100644 --- a/libop/op_events.c +++ b/libop/op_events.c @@ -256,6 +256,7 @@ static void free_unit_mask(struct op_unit_mask * um) { list_del(&um->um_next); free(um); + um = NULL; } /* @@ -289,8 +290,6 @@ static void read_unit_masks(char const * file) } else { if (!um) parse_error("no unit mask name line"); - if (um->num >= MAX_UNIT_MASK) - parse_error("oprofile: maximum unit mask entries exceeded"); parse_um_entry(&um->um[um->num], line); ++(um->num); @@ -561,6 +560,7 @@ static void read_events(char const * file) c = skip_ws(c); if (*c != '\0' && *c != '#') parse_error("non whitespace after include:"); + break; } else { parse_error("unknown tag"); } @@ -1037,7 +1037,7 @@ static int _is_um_valid_bitmask(struct op_event * event, u32 passed_um) for (i = 1; i < evt.unit->num; i++) { int j = i - 1; u32 tmp = evt.unit->um[i].value; - while (tmp < evt.unit->um[j].value && j >= 0) { + while (j >= 0 && tmp < evt.unit->um[j].value) { evt.unit->um[j + 1].value = evt.unit->um[j].value; j -= 1; } @@ -1081,8 +1081,6 @@ static int _is_um_valid_bitmask(struct op_event * event, u32 passed_um) } } - free(tmp_um); - free(tmp_um_no_dups); if (dup_value_used) { fprintf(stderr, "Ambiguous bitmask: Unit mask values" " cannot include non-unique numerical values (i.e., 0x%x).\n", @@ -1092,6 +1090,8 @@ static int _is_um_valid_bitmask(struct op_event * event, u32 passed_um) } else if (masked_val == passed_um && passed_um != 0) { retval = 1; } + free(tmp_um); + free(tmp_um_no_dups); return retval; } diff --git a/libop/op_mangle.c b/libop/op_mangle.c index 791e387..17a0ab7 100644 --- a/libop/op_mangle.c +++ b/libop/op_mangle.c @@ -73,7 +73,7 @@ char * op_mangle_filename(struct mangle_values const * values) strcat(mangled, "{dep}" "/"); append_image(mangled, values->flags, anon, dep_name, anon_name); - if (values->flags & MANGLE_CALLGRAPH) { + if (cg_image_name && (values->flags & MANGLE_CALLGRAPH)) { strcat(mangled, "{cg}" "/"); append_image(mangled, values->flags, cg_anon, cg_image_name, anon_name); diff --git a/libopagent/opagent.c b/libopagent/opagent.c index 9e31e4e..1e236a7 100644 --- a/libopagent/opagent.c +++ b/libopagent/opagent.c @@ -57,6 +57,7 @@ #include <stdint.h> #include <limits.h> #include <sys/types.h> +#include <dirent.h> #include <sys/stat.h> #include <fcntl.h> #include <unistd.h> @@ -136,21 +137,46 @@ op_agent_t op_open_agent(void) struct timeval tv; FILE * dumpfile = NULL; - if (0 == stat(TMP_OPROFILE_DIR, &dirstat) && !S_ISDIR(dirstat.st_mode)) { - fprintf(stderr, "Error: Creation of directory %s failed.\n", TMP_OPROFILE_DIR); - errno = EEXIST; - return NULL; + /* Coverity complains about 'time-of-check-time-of-use' race if we do stat() on + * a file (or directory) and then open or create it afterwards. So instead, + * we'll try to open it and see what happens. + */ + int create_dir = 0; + DIR * dir1 = opendir(TMP_OPROFILE_DIR); + if (!dir1) { + if (errno == ENOENT) { + create_dir = 1; + } else if (errno == ENOTDIR) { + fprintf(stderr, "Error: Creation of directory %s failed. File exists where directory is expected.\n", + TMP_OPROFILE_DIR); + return NULL; + } } else { + closedir(dir1); + } + if (create_dir) { + create_dir = 0; rc = mkdir(TMP_OPROFILE_DIR, S_IRWXU | S_IRWXG | S_IRWXO); if (rc && (errno != EEXIST)) { fprintf(stderr, "Error trying to create %s dir.\n", TMP_OPROFILE_DIR); return NULL; } } - if (0 == stat(JITDUMP_DIR, &dirstat) && !S_ISDIR(dirstat.st_mode)) { - fprintf(stderr, "Error: Creation of directory %s failed.\n", JITDUMP_DIR); - return NULL; + + dir1 = opendir(JITDUMP_DIR); + if (!dir1) { + if (errno == ENOENT) { + create_dir = 1; + } else if (errno == ENOTDIR) { + fprintf(stderr, "Error: Creation of directory %s failed. File exists where directory is expected.\n", + JITDUMP_DIR); + return NULL; + } } else { + closedir(dir1); + } + + if (create_dir) { rc = mkdir(JITDUMP_DIR, S_IRWXU | S_IRWXG | S_IRWXO); if (rc && (errno != EEXIST)) { fprintf(stderr, "Error trying to create %s dir.\n", JITDUMP_DIR); diff --git a/libperf_events/operf_counter.cpp b/libperf_events/operf_counter.cpp index a7279f1..4f20e4a 100644 --- a/libperf_events/operf_counter.cpp +++ b/libperf_events/operf_counter.cpp @@ -147,7 +147,7 @@ out: static event_t * _get_perf_event_from_file(struct mmap_info & info) { - uint32_t size; + uint32_t size = 0; static int num_remaps = 0; event_t * event; size_t pe_header_size = sizeof(struct perf_event_header); @@ -600,8 +600,11 @@ void operf_record::setup() */ use_cpu_minus_one = use_cpu_minus_one ? true : profile_process_group; num_cpus = use_cpu_minus_one ? 1 : sysconf(_SC_NPROCESSORS_ONLN); - if (!num_cpus) - throw runtime_error("Number of online CPUs is zero; cannot continue");; + if (num_cpus < 1) { + char int_str[256]; + sprintf(int_str, "Number of online CPUs is %d; cannot continue", num_cpus); + throw runtime_error(int_str); + } cverb << vrecord << "calling perf_event_open for pid " << pid_to_profile << " on " << num_cpus << " cpus" << endl; @@ -675,8 +678,6 @@ void operf_record::setup() } } } - if (dir) - closedir(dir); int num_mmaps; if (pid_started && (procs.size() > 1)) num_mmaps = procs.size(); @@ -691,6 +692,8 @@ void operf_record::setup() // Set bit to indicate we're set to go. valid = true; + if (dir) + closedir(dir); return; error: diff --git a/libperf_events/operf_mangling.cpp b/libperf_events/operf_mangling.cpp index 7ded563..384b05d 100644 --- a/libperf_events/operf_mangling.cpp +++ b/libperf_events/operf_mangling.cpp @@ -145,7 +145,11 @@ int operf_open_sample_file(odb_t *file, struct operf_sfile *last, cverb << vsfile << "Opening \"" << mangled << "\"" << endl; - create_path(mangled); + err = create_path(mangled); + if (err) { + cerr << "operf: create path for " << mangled << " failed: " << strerror(err) << endl; + goto out; + } /* locking sf will lock associated cg files too */ operf_sfile_get(sf); diff --git a/libperf_events/operf_process_info.cpp b/libperf_events/operf_process_info.cpp index 66804fc..5f9351a 100644 --- a/libperf_events/operf_process_info.cpp +++ b/libperf_events/operf_process_info.cpp @@ -362,11 +362,11 @@ void operf_process_info::try_disassociate_from_parent(char * app_shortname) */ if (mmappings_from_parent[cur->start_addr]) { mmappings_from_parent[cur->start_addr] = false; - mmappings.erase(it); + mmappings.erase(it++); } else { process_mapping(cur, false); + it++; } - it++; } if (parent_of_fork) { parent_of_fork->remove_forked_process(this->pid); diff --git a/opjitconv/opjitconv.c b/opjitconv/opjitconv.c index 7d81629..a9dfa91 100644 --- a/opjitconv/opjitconv.c +++ b/opjitconv/opjitconv.c @@ -24,6 +24,7 @@ #include <errno.h> #include <fcntl.h> #include <limits.h> +#include <assert.h> #include <pwd.h> #include <stdint.h> #include <stdio.h> @@ -739,6 +740,7 @@ static void _cleanup_jitdumps(void) char fname[1024]; memset(buf, '\0', 1024); memset(fname, '\0', 1024); + memset(buf, '\0', 1024); strcpy(fname, proc_fd_dir); strncat(fname, dirent->d_name, 1023 - proc_fd_dir_len); if (readlink(fname, buf, 1023) > 0) { @@ -751,6 +753,7 @@ static void _cleanup_jitdumps(void) } } } + closedir(dir); } if (!do_not_delete) { if (remove(dmpfile_pathname)) @@ -772,8 +775,9 @@ static void _cleanup_jitdumps(void) int main(int argc, char ** argv) { unsigned long long start_time, end_time; - char const * session_dir; + char session_dir[PATH_MAX]; int rc = 0; + size_t sessdir_len = 0; char * path_end; debug = 0; @@ -811,12 +815,15 @@ int main(int argc, char ** argv) * are not possible anymore. */ path_end = memchr(argv[1], '\0', PATH_MAX); - if (!path_end || ((path_end - argv[1]) > PATH_MAX - 16)) { + if (!path_end || ((sessdir_len = (path_end - argv[1])) >= PATH_MAX - 16)) { printf("opjitconv: Path name length limit exceeded for session directory\n"); rc = EXIT_FAILURE; goto out; } - session_dir = argv[1]; + memset(session_dir, '\0', PATH_MAX); + assert(sessdir_len < (PATH_MAX - 16 - 1)); + strncpy(session_dir, argv[1], sessdir_len); + session_dir[PATH_MAX -1] = '\0'; start_time = atol(argv[2]); end_time = atol(argv[3]); diff --git a/pe_profiling/operf.cpp b/pe_profiling/operf.cpp index 11b9ed0..4543c78 100644 --- a/pe_profiling/operf.cpp +++ b/pe_profiling/operf.cpp @@ -1324,7 +1324,7 @@ static void _process_events_list(void) << endl << "15 times the minimum count value for the event." << endl; exit(EXIT_FAILURE); } - fclose(fp); + pclose(fp); char * event_str = op_xstrndup(event_spec.c_str(), event_spec.length()); operf_event_t event; strncpy(event.name, strtok(event_str, ":"), OP_MAX_EVT_NAME_LEN - 1); @@ -1445,6 +1445,10 @@ static void _process_session_dir(void) cwd = (char *) xmalloc(PATH_MAX); // set default session dir cwd = getcwd(cwd, PATH_MAX); + if (cwd == NULL) { + perror("Error calling getcwd"); + exit(EXIT_FAILURE); + } operf_options::session_dir = cwd; operf_options::session_dir +="/oprofile_data"; samples_dir = operf_options::session_dir + "/samples"; @@ -1798,6 +1802,7 @@ static int _get_cpu_for_perf_events_cap(void) memset(cpus_online, 0, sizeof(cpus_online)); if ( fgets(cpus_online, sizeof(cpus_online), online_cpus) == NULL) { + fclose(online_cpus); err_msg = "Internal Error (3): Number of online cpus cannot be determined."; retval = -1; goto error; diff --git a/pp/opreport.cpp b/pp/opreport.cpp index 327043c..7ad6190 100644 --- a/pp/opreport.cpp +++ b/pp/opreport.cpp @@ -323,9 +323,10 @@ void output_summaries(summary_container const & summaries) for (size_t i = 0; i < summaries.apps.size(); ++i) { app_summary const & app = summaries.apps[i]; - - if ((app.counts[0] * 100.0) / summaries.total_counts[0] - < options::threshold) { + double ratio = (!summaries.total_counts[0]) ? 0 + : (app.counts[0] * 100.0)/ + summaries.total_counts[0]; + if (ratio < options::threshold) { continue; } -- 1.7.1 |
From: Maynard J. <may...@us...> - 2013-05-28 13:37:29
|
On 05/20/2013 05:41 PM, Maynard Johnson wrote: > Fix Coverity errors found on May 20, 2013 git snapshot Patch committed. -Maynard > > Coverity identified the following errors on scans run from May 7 through > May 20, 2013: > > Type,Category,File,Function > Wrapper object use after free,Memory - illegal accesses,/agents/jvmpi/jvmpi_oprofile.cpp,compiled_method_load(JVMPI_Event *) > Unchecked return value,Error handling issues,/daemon/opd_mangling.c,opd_open_sample_file > Dereference after null check,Null pointer dereferences,/daemon/opd_sfile.c,sfile_hash > Uninitialized scalar field,Uninitialized members,/gui/oprof_start_config.cpp,config_setting::config_setting() > Division or modulo by zero,Integer handling issues,/libdb/db_stat.c,odb_hash_stat > Resource leak,Resource leaks,/libop/op_cpu_type.c,_auxv_fetch > Resource leak,Resource leaks,/libop/op_cpu_type.c,fetch_at_hw_platform > Negative array index read,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask > Write to pointer after free,Memory - corruptions,/libop/op_events.c,read_events > Read from pointer after free,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask > Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename > Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename > Time of check time of use,Security best practices violations,/libopagent/opagent.c,op_open_agent > Improper use of negative value,Integer handling issues,/libperf_events/operf_counter.cpp,operf_record::setup() > Double free,Memory - corruptions,/libperf_events/operf_counter.cpp,operf_record::setup() > Uninitialized pointer read,Memory - illegal accesses,/libperf_events/operf_counter.cpp,<unnamed>::_get_perf_event_from_file(mmap_info &) > Unchecked return value,Error handling issues,/libperf_events/operf_mangling.cpp,"operf_open_sample_file(odb_t *, operf_sfile *, operf_sfile *, int, int)" > Using invalid iterator,API usage errors,/libperf_events/operf_process_info.cpp,operf_process_info::try_disassociate_from_parent(char *) > Non-array delete for scalars,Memory - illegal accesses,/libregex/op_regex.cpp,"<unnamed>::op_regerror(int, const re_pattern_buffer &)" > Resource leak,Resource leaks,/libutil++/op_bfd.cpp,"op_bfd::op_bfd(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const string_filter &, const extra_images &, bool &)" > Explicit null dereferenced,Null pointer dereferences,/opjitconv/create_bfd.c,fill_symtab > Resource leak,Resource leaks,/opjitconv/opjitconv.c,_cleanup_jitdumps > Use of untrusted string value,Insecure data handling,/opjitconv/opjitconv.c,main > Resource leak,Resource leaks,/pe_profiling/operf.cpp,_get_cpu_for_perf_events_cap() > Dereference null return value,Null pointer dereferences,/pe_profiling/operf.cpp,_process_session_dir() > Incorrect deallocator used,API usage errors,/pe_profiling/operf.cpp,_process_events_list() > > =============================== > > This patch fixes those errors. > > Signed-off-by: Maynard Johnson <may...@us...> > --- > agents/jvmpi/jvmpi_oprofile.cpp | 4 +- > daemon/opd_mangling.c | 7 +++++- > gui/oprof_start_config.cpp | 2 + > libdb/db_stat.c | 3 +- > libop/op_cpu_type.c | 12 ++++++++++ > libop/op_events.c | 10 ++++---- > libop/op_mangle.c | 2 +- > libopagent/opagent.c | 40 +++++++++++++++++++++++++++----- > libperf_events/operf_counter.cpp | 13 ++++++---- > libperf_events/operf_mangling.cpp | 6 ++++- > libperf_events/operf_process_info.cpp | 4 +- > opjitconv/opjitconv.c | 13 ++++++++-- > pe_profiling/operf.cpp | 7 +++++- > pp/opreport.cpp | 7 +++-- > 14 files changed, 98 insertions(+), 32 deletions(-) > [snip] |
From: Carl E. L. <ce...@li...> - 2013-05-28 22:21:26
|
On Tue, 2013-05-28 at 08:36 -0500, Maynard Johnson wrote: > On 05/20/2013 05:41 PM, Maynard Johnson wrote: > > Fix Coverity errors found on May 20, 2013 git snapshot > > Patch committed. Maynard: I am testing the patch from email "Re: [PATCH] Add support for IBM POWER8 processor". I applied the patch to the lasted GSA as of May 28th, 2013. I had some issues with a few of the events, specifically: operf --events PM_RUN_INST_CMPL:500000:0:1:1 dd bs=16 if=/dev/urandom of=/dev/null count=5000 operf --events PM_RUN_CYC:500000:0:1:1 dd bs=16 if=/dev/urandom of=/dev/null count=5000 I get the message "NULL returned for one or both of AT_PLATFORM/AT_BASE_PLATFORM AT_PLATFORM: (null); AT_BASE_PLATFORM: (null) *** Error in `/home/carll/bin//ophelp': double free or corruption (out): 0x000001001ab603d0 *** When I applied the patch from the other email, I noticed that there was a little fuzz with one of the files. So, I suspect that the patch you sent out was done without this coverity patch applied. It may be that these issues are associated with the coverity patch. I haven't sorted it out for sure. Can you verify that the coverity patch and the P8 patch work together nicely?? Carl Love > > -Maynard > > > > Coverity identified the following errors on scans run from May 7 through > > May 20, 2013: > > > > Type,Category,File,Function > > Wrapper object use after free,Memory - illegal accesses,/agents/jvmpi/jvmpi_oprofile.cpp,compiled_method_load(JVMPI_Event *) > > Unchecked return value,Error handling issues,/daemon/opd_mangling.c,opd_open_sample_file > > Dereference after null check,Null pointer dereferences,/daemon/opd_sfile.c,sfile_hash > > Uninitialized scalar field,Uninitialized members,/gui/oprof_start_config.cpp,config_setting::config_setting() > > Division or modulo by zero,Integer handling issues,/libdb/db_stat.c,odb_hash_stat > > Resource leak,Resource leaks,/libop/op_cpu_type.c,_auxv_fetch > > Resource leak,Resource leaks,/libop/op_cpu_type.c,fetch_at_hw_platform > > Negative array index read,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask > > Write to pointer after free,Memory - corruptions,/libop/op_events.c,read_events > > Read from pointer after free,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask > > Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename > > Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename > > Time of check time of use,Security best practices violations,/libopagent/opagent.c,op_open_agent > > Improper use of negative value,Integer handling issues,/libperf_events/operf_counter.cpp,operf_record::setup() > > Double free,Memory - corruptions,/libperf_events/operf_counter.cpp,operf_record::setup() > > Uninitialized pointer read,Memory - illegal accesses,/libperf_events/operf_counter.cpp,<unnamed>::_get_perf_event_from_file(mmap_info &) > > Unchecked return value,Error handling issues,/libperf_events/operf_mangling.cpp,"operf_open_sample_file(odb_t *, operf_sfile *, operf_sfile *, int, int)" > > Using invalid iterator,API usage errors,/libperf_events/operf_process_info.cpp,operf_process_info::try_disassociate_from_parent(char *) > > Non-array delete for scalars,Memory - illegal accesses,/libregex/op_regex.cpp,"<unnamed>::op_regerror(int, const re_pattern_buffer &)" > > Resource leak,Resource leaks,/libutil++/op_bfd.cpp,"op_bfd::op_bfd(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const string_filter &, const extra_images &, bool &)" > > Explicit null dereferenced,Null pointer dereferences,/opjitconv/create_bfd.c,fill_symtab > > Resource leak,Resource leaks,/opjitconv/opjitconv.c,_cleanup_jitdumps > > Use of untrusted string value,Insecure data handling,/opjitconv/opjitconv.c,main > > Resource leak,Resource leaks,/pe_profiling/operf.cpp,_get_cpu_for_perf_events_cap() > > Dereference null return value,Null pointer dereferences,/pe_profiling/operf.cpp,_process_session_dir() > > Incorrect deallocator used,API usage errors,/pe_profiling/operf.cpp,_process_events_list() > > > > =============================== > > > > This patch fixes those errors. > > > > Signed-off-by: Maynard Johnson <may...@us...> > > --- > > agents/jvmpi/jvmpi_oprofile.cpp | 4 +- > > daemon/opd_mangling.c | 7 +++++- > > gui/oprof_start_config.cpp | 2 + > > libdb/db_stat.c | 3 +- > > libop/op_cpu_type.c | 12 ++++++++++ > > libop/op_events.c | 10 ++++---- > > libop/op_mangle.c | 2 +- > > libopagent/opagent.c | 40 +++++++++++++++++++++++++++----- > > libperf_events/operf_counter.cpp | 13 ++++++---- > > libperf_events/operf_mangling.cpp | 6 ++++- > > libperf_events/operf_process_info.cpp | 4 +- > > opjitconv/opjitconv.c | 13 ++++++++-- > > pe_profiling/operf.cpp | 7 +++++- > > pp/opreport.cpp | 7 +++-- > > 14 files changed, 98 insertions(+), 32 deletions(-) > > > [snip] > > > ------------------------------------------------------------------------------ > Try New Relic Now & We'll Send You this Cool Shirt > New Relic is the only SaaS-based application performance monitoring service > that delivers powerful full stack analytics. Optimize and monitor your > browser, app, & servers with just a few lines of code. Try New Relic > and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may > _______________________________________________ > oprofile-list mailing list > opr...@li... > https://lists.sourceforge.net/lists/listinfo/oprofile-list > |
From: Maynard J. <may...@us...> - 2013-05-28 22:55:38
|
On 05/28/2013 05:21 PM, Carl E. Love wrote: > On Tue, 2013-05-28 at 08:36 -0500, Maynard Johnson wrote: >> On 05/20/2013 05:41 PM, Maynard Johnson wrote: >>> Fix Coverity errors found on May 20, 2013 git snapshot >> >> Patch committed. > > Maynard: > > I am testing the patch from email "Re: [PATCH] Add support for IBM > POWER8 processor". > > I applied the patch to the lasted GSA as of May 28th, 2013. I had some > issues with a few of the events, specifically: > operf --events PM_RUN_INST_CMPL:500000:0:1:1 dd bs=16 if=/dev/urandom of=/dev/null count=5000 > operf --events PM_RUN_CYC:500000:0:1:1 dd bs=16 if=/dev/urandom of=/dev/null count=5000 > > I get the message "NULL returned for one or both of AT_PLATFORM/AT_BASE_PLATFORM > AT_PLATFORM: (null); AT_BASE_PLATFORM: (null) > *** Error in `/home/carll/bin//ophelp': double free or corruption (out): > 0x000001001ab603d0 *** Ouch! Yes, you're right. The "coverity patch" and the power8 patch don't work together. The fault was in an incorrect change I made for the coverity patch. I'll post a patch for that soon. Thanks for finding this! -Maynard > > When I applied the patch from the other email, I noticed that there was > a little fuzz with one of the files. So, I suspect that the patch you > sent out was done without this coverity patch applied. It may be that > these issues are associated with the coverity patch. I haven't sorted > it out for sure. Can you verify that the coverity patch and the P8 > patch work together nicely?? > > Carl Love >> >> -Maynard >>> >>> Coverity identified the following errors on scans run from May 7 through >>> May 20, 2013: >>> >>> Type,Category,File,Function >>> Wrapper object use after free,Memory - illegal accesses,/agents/jvmpi/jvmpi_oprofile.cpp,compiled_method_load(JVMPI_Event *) >>> Unchecked return value,Error handling issues,/daemon/opd_mangling.c,opd_open_sample_file >>> Dereference after null check,Null pointer dereferences,/daemon/opd_sfile.c,sfile_hash >>> Uninitialized scalar field,Uninitialized members,/gui/oprof_start_config.cpp,config_setting::config_setting() >>> Division or modulo by zero,Integer handling issues,/libdb/db_stat.c,odb_hash_stat >>> Resource leak,Resource leaks,/libop/op_cpu_type.c,_auxv_fetch >>> Resource leak,Resource leaks,/libop/op_cpu_type.c,fetch_at_hw_platform >>> Negative array index read,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask >>> Write to pointer after free,Memory - corruptions,/libop/op_events.c,read_events >>> Read from pointer after free,Memory - illegal accesses,/libop/op_events.c,_is_um_valid_bitmask >>> Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename >>> Dereference after null check,Null pointer dereferences,/libop/op_mangle.c,op_mangle_filename >>> Time of check time of use,Security best practices violations,/libopagent/opagent.c,op_open_agent >>> Improper use of negative value,Integer handling issues,/libperf_events/operf_counter.cpp,operf_record::setup() >>> Double free,Memory - corruptions,/libperf_events/operf_counter.cpp,operf_record::setup() >>> Uninitialized pointer read,Memory - illegal accesses,/libperf_events/operf_counter.cpp,<unnamed>::_get_perf_event_from_file(mmap_info &) >>> Unchecked return value,Error handling issues,/libperf_events/operf_mangling.cpp,"operf_open_sample_file(odb_t *, operf_sfile *, operf_sfile *, int, int)" >>> Using invalid iterator,API usage errors,/libperf_events/operf_process_info.cpp,operf_process_info::try_disassociate_from_parent(char *) >>> Non-array delete for scalars,Memory - illegal accesses,/libregex/op_regex.cpp,"<unnamed>::op_regerror(int, const re_pattern_buffer &)" >>> Resource leak,Resource leaks,/libutil++/op_bfd.cpp,"op_bfd::op_bfd(const std::basic_string<char, std::char_traits<char>, std::allocator<char>>&, const string_filter &, const extra_images &, bool &)" >>> Explicit null dereferenced,Null pointer dereferences,/opjitconv/create_bfd.c,fill_symtab >>> Resource leak,Resource leaks,/opjitconv/opjitconv.c,_cleanup_jitdumps >>> Use of untrusted string value,Insecure data handling,/opjitconv/opjitconv.c,main >>> Resource leak,Resource leaks,/pe_profiling/operf.cpp,_get_cpu_for_perf_events_cap() >>> Dereference null return value,Null pointer dereferences,/pe_profiling/operf.cpp,_process_session_dir() >>> Incorrect deallocator used,API usage errors,/pe_profiling/operf.cpp,_process_events_list() >>> >>> =============================== >>> >>> This patch fixes those errors. >>> >>> Signed-off-by: Maynard Johnson <may...@us...> >>> --- >>> agents/jvmpi/jvmpi_oprofile.cpp | 4 +- >>> daemon/opd_mangling.c | 7 +++++- >>> gui/oprof_start_config.cpp | 2 + >>> libdb/db_stat.c | 3 +- >>> libop/op_cpu_type.c | 12 ++++++++++ >>> libop/op_events.c | 10 ++++---- >>> libop/op_mangle.c | 2 +- >>> libopagent/opagent.c | 40 +++++++++++++++++++++++++++----- >>> libperf_events/operf_counter.cpp | 13 ++++++---- >>> libperf_events/operf_mangling.cpp | 6 ++++- >>> libperf_events/operf_process_info.cpp | 4 +- >>> opjitconv/opjitconv.c | 13 ++++++++-- >>> pe_profiling/operf.cpp | 7 +++++- >>> pp/opreport.cpp | 7 +++-- >>> 14 files changed, 98 insertions(+), 32 deletions(-) >>> >> [snip] >> >> >> ------------------------------------------------------------------------------ >> Try New Relic Now & We'll Send You this Cool Shirt >> New Relic is the only SaaS-based application performance monitoring service >> that delivers powerful full stack analytics. Optimize and monitor your >> browser, app, & servers with just a few lines of code. Try New Relic >> and get this awesome Nerd Life shirt! http://p.sf.net/sfu/newrelic_d2d_may >> _______________________________________________ >> oprofile-list mailing list >> opr...@li... >> https://lists.sourceforge.net/lists/listinfo/oprofile-list >> > > |