|
From: <sv...@va...> - 2009-08-23 22:54:55
|
Author: njn
Date: 2009-08-23 23:46:04 +0100 (Sun, 23 Aug 2009)
New Revision: 10864
Log:
Make messaging more consistent for aborts at start-up (mostly due to bad
command line options); they all are prefixed just with "valgrind:" now.
Make '-h -h' equivalent to '--help-debug'.
Modified:
branches/MSG2/cachegrind/cg_main.c
branches/MSG2/callgrind/sim.c
branches/MSG2/coregrind/m_libcprint.c
branches/MSG2/coregrind/m_main.c
branches/MSG2/coregrind/m_options.c
branches/MSG2/coregrind/m_replacemalloc/replacemalloc_core.c
branches/MSG2/coregrind/pub_core_libcprint.h
branches/MSG2/coregrind/pub_core_options.h
branches/MSG2/docs/xml/manual-core.xml
branches/MSG2/include/pub_tool_libcprint.h
branches/MSG2/include/pub_tool_options.h
branches/MSG2/include/pub_tool_tooliface.h
branches/MSG2/massif/ms_main.c
Modified: branches/MSG2/cachegrind/cg_main.c
===================================================================
--- branches/MSG2/cachegrind/cg_main.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/cachegrind/cg_main.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -1175,44 +1175,36 @@
static cache_t clo_D1_cache = UNDEFINED_CACHE;
static cache_t clo_L2_cache = UNDEFINED_CACHE;
-/* Checks cache config is ok; makes it so if not. */
-static
-void check_cache(cache_t* cache, Char *name)
+// Checks cache config is ok. Returns NULL if ok, or a pointer to an error
+// string otherwise.
+static Char* check_cache(cache_t* cache)
{
- /* Simulator requires line size and set count to be powers of two */
- if (( cache->size % (cache->line_size * cache->assoc) != 0) ||
- (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc))) {
- VG_(umsg)("error: %s set count not a power of two; aborting.\n", name);
- VG_(exit)(1);
- }
+ // Simulator requires set count to be a power of two.
+ if ((cache->size % (cache->line_size * cache->assoc) != 0) ||
+ (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc)))
+ {
+ return "Cache set count is not a power of two.\n";
- if (-1 == VG_(log2)(cache->line_size)) {
- VG_(umsg)("error: %s line size of %dB not a power of two; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
- }
+ // Simulator requires line size to be a power of two.
+ } else if (-1 == VG_(log2)(cache->line_size)) {
+ return "Cache line size is not a power of two.\n";
// Then check line size >= 16 -- any smaller and a single instruction could
// straddle three cache lines, which breaks a simulation assertion and is
// stupid anyway.
- if (cache->line_size < MIN_LINE_SIZE) {
- VG_(umsg)("error: %s line size of %dB too small; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
- }
+ } else if (cache->line_size < MIN_LINE_SIZE) {
+ return "Cache line size is too small.\n";
- /* Then check cache size > line size (causes seg faults if not). */
- if (cache->size <= cache->line_size) {
- VG_(umsg)("error: %s cache size of %dB <= line size of %dB; aborting.\n",
- name, cache->size, cache->line_size);
- VG_(exit)(1);
- }
+ // Then check cache size > line size (causes seg faults if not).
+ } else if (cache->size <= cache->line_size) {
+ return "Cache size <= line size.\n";
- /* Then check assoc <= (size / line size) (seg faults otherwise). */
- if (cache->assoc > (cache->size / cache->line_size)) {
- VG_(umsg)("warning: %s associativity > (size / line size); aborting.\n",
- name);
- VG_(exit)(1);
+ // Then check assoc <= (size / line size) (seg faults otherwise).
+ } else if (cache->assoc > (cache->size / cache->line_size)) {
+ return "Cache associativity > (size / line size).\n";
+
+ } else {
+ return NULL;
}
}
@@ -1221,27 +1213,28 @@
{
#define DEFINED(L) (-1 != L.size || -1 != L.assoc || -1 != L.line_size)
- Int n_clos = 0;
+ Char* checkRes;
- // Count how many were defined on the command line.
- if (DEFINED(clo_I1_cache)) { n_clos++; }
- if (DEFINED(clo_D1_cache)) { n_clos++; }
- if (DEFINED(clo_L2_cache)) { n_clos++; }
+ Bool all_caches_clo_defined =
+ (DEFINED(clo_I1_cache) &&
+ DEFINED(clo_D1_cache) &&
+ DEFINED(clo_L2_cache));
// Set the cache config (using auto-detection, if supported by the
- // architecture)
- VG_(configure_caches)( I1c, D1c, L2c, (3 == n_clos) );
+ // architecture).
+ VG_(configure_caches)( I1c, D1c, L2c, all_caches_clo_defined );
- // Then replace with any defined on the command line.
+ // Check the default/auto-detected values.
+ checkRes = check_cache(I1c); tl_assert(!checkRes);
+ checkRes = check_cache(D1c); tl_assert(!checkRes);
+ checkRes = check_cache(L2c); tl_assert(!checkRes);
+
+ // Then replace with any defined on the command line. (Already checked in
+ // parse_cache_opt().)
if (DEFINED(clo_I1_cache)) { *I1c = clo_I1_cache; }
if (DEFINED(clo_D1_cache)) { *D1c = clo_D1_cache; }
if (DEFINED(clo_L2_cache)) { *L2c = clo_L2_cache; }
- // Then check values and fix if not acceptable.
- check_cache(I1c, "I1");
- check_cache(D1c, "D1");
- check_cache(L2c, "L2");
-
if (VG_(clo_verbosity) >= 2) {
VG_(umsg)("Cache configuration used:\n");
VG_(umsg)(" I1: %dB, %d-way, %dB lines\n",
@@ -1659,6 +1652,7 @@
{
Long i1, i2, i3;
Char* endptr;
+ Char* checkRes;
// Option argument looks like "65536,2,64". Extract them.
i1 = VG_(strtoll10)(optval, &endptr); if (*endptr != ',') goto bad;
@@ -1673,12 +1667,20 @@
if (cache->assoc != i2) goto overflow;
if (cache->line_size != i3) goto overflow;
+ checkRes = check_cache(cache);
+ if (checkRes) {
+ VG_(msgf)(checkRes);
+ goto bad;
+ }
+
return;
+ bad:
+ VG_(msgf_bad_option)(opt, "");
+
overflow:
- VG_(umsg)("one of the cache parameters was too large and overflowed\n");
- bad:
- VG_(err_bad_option)(opt);
+ VG_(msgf_bad_option)(opt,
+ "One of the cache parameters was too large and overflowed.\n");
}
static Bool cg_process_cmd_line_option(Char* arg)
Modified: branches/MSG2/callgrind/sim.c
===================================================================
--- branches/MSG2/callgrind/sim.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/callgrind/sim.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -1298,49 +1298,36 @@
static cache_t clo_D1_cache = UNDEFINED_CACHE;
static cache_t clo_L2_cache = UNDEFINED_CACHE;
-
-/* Checks cache config is ok; makes it so if not. */
-static
-void check_cache(cache_t* cache, Char *name)
+// Checks cache config is ok. Returns NULL if ok, or a pointer to an error
+// string otherwise.
+static Char* check_cache(cache_t* cache)
{
- /* Simulator requires line size and set count to be powers of two */
- if (( cache->size % (cache->line_size * cache->assoc) != 0) ||
- (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc))) {
- VG_(message)(Vg_UserMsg,
- "error: %s set count not a power of two; aborting.\n",
- name);
- }
+ // Simulator requires line size and set count to be powers of two.
+ if ((cache->size % (cache->line_size * cache->assoc) != 0) ||
+ (-1 == VG_(log2)(cache->size/cache->line_size/cache->assoc)))
+ {
+ return "Cache set count is not a power of two.\n";
- if (-1 == VG_(log2)(cache->line_size)) {
- VG_(message)(Vg_UserMsg,
- "error: %s line size of %dB not a power of two; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
- }
+ // Simulator requires line size to be a power of two.
+ } else if (-1 == VG_(log2)(cache->line_size)) {
+ return "Cache line size is not a power of two.\n";
// Then check line size >= 16 -- any smaller and a single instruction could
// straddle three cache lines, which breaks a simulation assertion and is
// stupid anyway.
- if (cache->line_size < MIN_LINE_SIZE) {
- VG_(message)(Vg_UserMsg,
- "error: %s line size of %dB too small; aborting.\n",
- name, cache->line_size);
- VG_(exit)(1);
- }
+ } else if (cache->line_size < MIN_LINE_SIZE) {
+ return "Cache line size is too small.\n";
- /* Then check cache size > line size (causes seg faults if not). */
- if (cache->size <= cache->line_size) {
- VG_(message)(Vg_UserMsg,
- "error: %s cache size of %dB <= line size of %dB; aborting.\n",
- name, cache->size, cache->line_size);
- VG_(exit)(1);
- }
+ // Then check cache size > line size (causes seg faults if not).
+ } else if (cache->size <= cache->line_size) {
+ return "Cache size <= line size.\n";
- /* Then check assoc <= (size / line size) (seg faults otherwise). */
- if (cache->assoc > (cache->size / cache->line_size)) {
- VG_(message)(Vg_UserMsg,
- "warning: %s associativity > (size / line size); aborting.\n", name);
- VG_(exit)(1);
+ // Then check assoc <= (size / line size) (seg faults otherwise).
+ } else if (cache->assoc > (cache->size / cache->line_size)) {
+ return "Cache associativity > (size / line size).\n";
+
+ } else {
+ return NULL;
}
}
@@ -1349,27 +1336,27 @@
{
#define DEFINED(L) (-1 != L.size || -1 != L.assoc || -1 != L.line_size)
- Int n_clos = 0;
+ Char* checkRes;
- // Count how many were defined on the command line.
- if (DEFINED(clo_I1_cache)) { n_clos++; }
- if (DEFINED(clo_D1_cache)) { n_clos++; }
- if (DEFINED(clo_L2_cache)) { n_clos++; }
+ Bool all_caches_clo_defined =
+ (DEFINED(clo_I1_cache) &&
+ DEFINED(clo_D1_cache) &&
+ DEFINED(clo_L2_cache));
// Set the cache config (using auto-detection, if supported by the
- // architecture)
- VG_(configure_caches)( I1c, D1c, L2c, (3 == n_clos) );
+ // architecture).
+ VG_(configure_caches)( I1c, D1c, L2c, all_caches_clo_defined );
+ // Check the default/auto-detected values.
+ checkRes = check_cache(I1c); tl_assert(!checkRes);
+ checkRes = check_cache(D1c); tl_assert(!checkRes);
+ checkRes = check_cache(L2c); tl_assert(!checkRes);
+
// Then replace with any defined on the command line.
if (DEFINED(clo_I1_cache)) { *I1c = clo_I1_cache; }
if (DEFINED(clo_D1_cache)) { *D1c = clo_D1_cache; }
if (DEFINED(clo_L2_cache)) { *L2c = clo_L2_cache; }
- // Then check values and fix if not acceptable.
- check_cache(I1c, "I1");
- check_cache(D1c, "D1");
- check_cache(L2c, "L2");
-
if (VG_(clo_verbosity) > 1) {
VG_(message)(Vg_UserMsg, "Cache configuration used:\n");
VG_(message)(Vg_UserMsg, " I1: %dB, %d-way, %dB lines\n",
@@ -1532,13 +1519,14 @@
);
}
-static void parse_opt ( cache_t* cache, char* opt )
+static void parse_opt ( cache_t* cache, Char* opt, Char* optval )
{
Long i1, i2, i3;
Char* endptr;
+ Char* checkRes;
// Option argument looks like "65536,2,64". Extract them.
- i1 = VG_(strtoll10)(opt, &endptr); if (*endptr != ',') goto bad;
+ i1 = VG_(strtoll10)(optval, &endptr); if (*endptr != ',') goto bad;
i2 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != ',') goto bad;
i3 = VG_(strtoll10)(endptr+1, &endptr); if (*endptr != '\0') goto bad;
@@ -1550,15 +1538,20 @@
if (cache->assoc != i2) goto overflow;
if (cache->line_size != i3) goto overflow;
+ checkRes = check_cache(cache);
+ if (checkRes) {
+ VG_(msgf)(checkRes);
+ goto bad;
+ }
+
return;
- overflow:
- VG_(message)(Vg_UserMsg,
- "one of the cache parameters was too large and overflowed\n");
bad:
- // XXX: this omits the "--I1/D1/L2=" part from the message, but that's
- // not a big deal.
- VG_(err_bad_option)(opt);
+ VG_(msgf_bad_option)(opt, "");
+
+ overflow:
+ VG_(msgf_bad_option)(opt,
+ "One of the cache parameters was too large and overflowed.\n");
}
/* Check for command line option for cache configuration.
@@ -1582,11 +1575,11 @@
}
else if VG_STR_CLO(arg, "--I1", tmp_str)
- parse_opt(&clo_I1_cache, tmp_str);
+ parse_opt(&clo_I1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--D1", tmp_str)
- parse_opt(&clo_D1_cache, tmp_str);
+ parse_opt(&clo_D1_cache, arg, tmp_str);
else if VG_STR_CLO(arg, "--L2", tmp_str)
- parse_opt(&clo_L2_cache, tmp_str);
+ parse_opt(&clo_L2_cache, arg, tmp_str);
else
return False;
Modified: branches/MSG2/coregrind/m_libcprint.c
===================================================================
--- branches/MSG2/coregrind/m_libcprint.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/m_libcprint.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -529,6 +529,12 @@
return count;
}
+static void revert_to_stderr ( void )
+{
+ VG_(log_output_sink).fd = 2; /* stderr */
+ VG_(log_output_sink).is_socket = False;
+}
+
/* VG_(message) variants with hardwired first argument. */
UInt VG_(msgf) ( const HChar* format, ... )
@@ -541,6 +547,18 @@
return count;
}
+void VG_(msgf_bad_option) ( HChar* opt, const HChar* format, ... )
+{
+ va_list vargs;
+ va_start(vargs,format);
+ revert_to_stderr();
+ VG_(message) (Vg_StartFailMsg, "Bad option: %s\n", opt);
+ VG_(vmessage)(Vg_StartFailMsg, format, vargs );
+ VG_(message) (Vg_StartFailMsg, "Use --help for more information.\n");
+ VG_(exit)(1);
+ va_end(vargs);
+}
+
// MMM: get rid of eventually, then rename VG_(msgu) as VG_(umsg). Likewise
// for dmsg.
UInt VG_(umsg) ( const HChar* format, ... )
@@ -602,7 +620,25 @@
b->buf_used = 0;
}
+__attribute__((noreturn))
+void VG_(err_missing_prog) ( void )
+{
+ revert_to_stderr();
+ VG_(msgf)("no program specified\n");
+ VG_(msgf)("Use --help for more information.\n");
+ VG_(exit)(1);
+}
+__attribute__((noreturn))
+void VG_(err_config_error) ( Char* msg )
+{
+ revert_to_stderr();
+ VG_(msgf)("Startup or configuration error:\n %s\n", msg);
+ VG_(msgf)("Unable to start up properly. Giving up.\n");
+ VG_(exit)(1);
+}
+
+
/*--------------------------------------------------------------------*/
/*--- end ---*/
/*--------------------------------------------------------------------*/
Modified: branches/MSG2/coregrind/m_main.c
===================================================================
--- branches/MSG2/coregrind/m_main.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/m_main.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -296,10 +296,10 @@
VG_(printf)("valgrind-" VERSION "\n");
VG_(exit)(0);
}
- else if VG_XACT_CLO(str, "--help", *need_help, 1) {}
- else if VG_XACT_CLO(str, "-h", *need_help, 1) {}
+ else if VG_XACT_CLO(str, "--help", *need_help, *need_help+1) {}
+ else if VG_XACT_CLO(str, "-h", *need_help, *need_help+1) {}
- else if VG_XACT_CLO(str, "--help-debug", *need_help, 2) {}
+ else if VG_XACT_CLO(str, "--help-debug", *need_help, *need_help+2) {}
// The tool has already been determined, but we need to know the name
// here.
@@ -531,10 +531,9 @@
else if VG_STR_CLO(arg, "--suppressions", tmp_str) {
if (VG_(clo_n_suppressions) >= VG_CLO_MAX_SFILES) {
- VG_(message)(Vg_UserMsg, "Too many suppression files specified.\n");
- VG_(message)(Vg_UserMsg,
- "Increase VG_CLO_MAX_SFILES and recompile.\n");
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg,
+ "Too many suppression files specified.\n"
+ "Increase VG_CLO_MAX_SFILES and recompile.\n");
}
VG_(clo_suppressions)[VG_(clo_n_suppressions)] = tmp_str;
VG_(clo_n_suppressions)++;
@@ -545,17 +544,15 @@
Int j;
if (8 != VG_(strlen)(tmp_str)) {
- VG_(message)(Vg_UserMsg,
- "--trace-flags argument must have 8 digits\n");
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg,
+ "--trace-flags argument must have 8 digits.\n");
}
for (j = 0; j < 8; j++) {
if ('0' == tmp_str[j]) { /* do nothing */ }
else if ('1' == tmp_str[j]) VG_(clo_trace_flags) |= (1 << (7-j));
else {
- VG_(message)(Vg_UserMsg, "--trace-flags argument can only "
- "contain 0s and 1s\n");
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg,
+ "--trace-flags argument can only contain 0s and 1s.\n");
}
}
}
@@ -565,17 +562,15 @@
Int j;
if (8 != VG_(strlen)(tmp_str)) {
- VG_(message)(Vg_UserMsg,
- "--profile-flags argument must have 8 digits\n");
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg,
+ "--profile-flags argument must have 8 digits.\n");
}
for (j = 0; j < 8; j++) {
if ('0' == tmp_str[j]) { /* do nothing */ }
else if ('1' == tmp_str[j]) VG_(clo_profile_flags) |= (1 << (7-j));
else {
- VG_(message)(Vg_UserMsg, "--profile-flags argument can only "
- "contain 0s and 1s\n");
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg,
+ "--profile-flags argument can only contain 0s and 1s.\n");
}
}
}
@@ -591,7 +586,7 @@
else if ( ! VG_(needs).command_line_options
|| ! VG_TDICT_CALL(tool_process_cmd_line_option, arg) ) {
- VG_(err_bad_option)(arg);
+ VG_(msgf_bad_option)(arg, "");
}
}
@@ -612,22 +607,24 @@
if (VG_(clo_verbosity) < 0)
VG_(clo_verbosity) = 0;
+ // MMM: why single out --gen-suppressions in this way? Heaps of options
+ // only make sense for error-producing tools. Either check all or none.
+ // If checking all, need an equivalent to
+ // VG_(replacement_malloc_process_cmd_line_option) for errors. (See
+ // ms_main.c for more.)
if (VG_(clo_gen_suppressions) > 0 &&
!VG_(needs).core_errors && !VG_(needs).tool_errors) {
- VG_(message)(Vg_UserMsg,
- "Can't use --gen-suppressions= with this tool,\n");
- VG_(message)(Vg_UserMsg,
- "as it doesn't generate errors.\n");
- VG_(err_bad_option)("--gen-suppressions=");
+ VG_(msgf_bad_option)("--gen-suppressions=yes",
+ "Can't use --gen-suppressions=yes with %s\n"
+ "because it doesn't generate errors.\n", VG_(details).name);
}
/* If XML output is requested, check that the tool actually
supports it. */
if (VG_(clo_xml) && !VG_(needs).xml_output) {
VG_(clo_xml) = False;
- VG_(message)(Vg_UserMsg,
+ VG_(msgf_bad_option)("--xml=yes",
"%s does not support XML output.\n", VG_(details).name);
- VG_(err_bad_option)("--xml=yes");
/*NOTREACHED*/
}
@@ -648,33 +645,28 @@
(--gen-suppressions=all is still OK since we don't need any
user interaction in this case.) */
if (VG_(clo_gen_suppressions) == 1) {
- VG_(umsg)(
- "When --xml=yes is specified, only --gen-suppressions=no\n"
- "or --gen-suppressions=all are allowed, but not "
+ VG_(msgf_bad_option)(
+ "--xml=yes together with --gen-suppressions=yes",
+ "When --xml=yes is specified, --gen-suppressions=no\n"
+ "or --gen-suppressions=all is allowed, but not "
"--gen-suppressions=yes.\n");
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --gen-suppressions=yes");
}
/* We can't allow DB attaching (or we maybe could, but results
could be chaotic ..) since it requires user input. Hence
disallow. */
if (VG_(clo_db_attach)) {
- VG_(umsg)("--db-attach=yes is not allowed in XML mode,\n"
- "as it would require user input.\n");
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --db-attach=yes");
+ VG_(msgf_bad_option)(
+ "--xml=yes together with --db-attach=yes",
+ "--db-attach=yes is not allowed with --xml=yes\n"
+ "because it would require user input.\n");
}
/* Disallow dump_error in XML mode; sounds like a recipe for
chaos. No big deal; dump_error is a flag for debugging V
itself. */
if (VG_(clo_dump_error) > 0) {
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes together with --dump-error=");
+ VG_(msgf_bad_option)("--xml=yes together with --dump-error", "");
}
/* Disable error limits (this might be a bad idea!) */
@@ -732,11 +724,9 @@
tmp_log_fd = sr_Res(sres);
VG_(clo_log_fname_expanded) = logfilename;
} else {
- VG_(message)(Vg_UserMsg,
- "Can't create log file '%s' (%s); giving up!\n",
- logfilename, VG_(strerror)(sr_Err(sres)));
- VG_(err_bad_option)(
- "--log-file=<file> (didn't work out for some reason.)");
+ VG_(msgf)("can't create log file '%s': %s\n",
+ logfilename, VG_(strerror)(sr_Err(sres)));
+ VG_(exit)(1);
/*NOTREACHED*/
}
break;
@@ -747,23 +737,15 @@
vg_assert(VG_(strlen)(log_fsname_unexpanded) <= 900); /* paranoia */
tmp_log_fd = VG_(connect_via_socket)( log_fsname_unexpanded );
if (tmp_log_fd == -1) {
- VG_(message)(Vg_UserMsg,
- "Invalid --log-socket=ipaddr or "
- "--log-socket=ipaddr:port spec\n");
- VG_(message)(Vg_UserMsg,
- "of '%s'; giving up!\n", log_fsname_unexpanded );
- VG_(err_bad_option)(
- "--log-socket=");
+ VG_(msgf)("Invalid --log-socket spec of '%s'\n",
+ log_fsname_unexpanded);
+ VG_(exit)(1);
/*NOTREACHED*/
}
if (tmp_log_fd == -2) {
- VG_(message)(Vg_UserMsg,
- "valgrind: failed to connect to logging server '%s'.\n",
- log_fsname_unexpanded );
- VG_(message)(Vg_UserMsg,
- "Log messages will sent to stderr instead.\n" );
- VG_(message)(Vg_UserMsg,
- "\n" );
+ VG_(msgv)("failed to connect to logging server '%s'.\n"
+ "Log messages will sent to stderr instead.\n",
+ log_fsname_unexpanded);
/* We don't change anything here. */
vg_assert(VG_(log_output_sink).fd == 2);
tmp_log_fd = 2;
@@ -803,11 +785,9 @@
*xml_fname_unexpanded = VG_(strdup)( "main.mpclo.2",
xml_fsname_unexpanded );
} else {
- VG_(message)(Vg_UserMsg,
- "Can't create XML file '%s' (%s); giving up!\n",
- xmlfilename, VG_(strerror)(sr_Err(sres)));
- VG_(err_bad_option)(
- "--xml-file=<file> (didn't work out for some reason.)");
+ VG_(msgf)("can't create XML file '%s': %s\n",
+ xmlfilename, VG_(strerror)(sr_Err(sres)));
+ VG_(exit)(1);
/*NOTREACHED*/
}
break;
@@ -818,23 +798,15 @@
vg_assert(VG_(strlen)(xml_fsname_unexpanded) <= 900); /* paranoia */
tmp_xml_fd = VG_(connect_via_socket)( xml_fsname_unexpanded );
if (tmp_xml_fd == -1) {
- VG_(message)(Vg_UserMsg,
- "Invalid --xml-socket=ipaddr or "
- "--xml-socket=ipaddr:port spec\n");
- VG_(message)(Vg_UserMsg,
- "of '%s'; giving up!\n", xml_fsname_unexpanded );
- VG_(err_bad_option)(
- "--xml-socket=");
+ VG_(msgf)("Invalid --xml-socket spec of '%s'\n",
+ xml_fsname_unexpanded );
+ VG_(exit)(1);
/*NOTREACHED*/
}
if (tmp_xml_fd == -2) {
- VG_(message)(Vg_UserMsg,
- "valgrind: failed to connect to XML logging server '%s'.\n",
- xml_fsname_unexpanded );
- VG_(message)(Vg_UserMsg,
- "XML output will sent to stderr instead.\n" );
- VG_(message)(Vg_UserMsg,
- "\n" );
+ VG_(msgv)("failed to connect to XML logging server '%s'.\n"
+ "XML output will sent to stderr instead.\n",
+ xml_fsname_unexpanded);
/* We don't change anything here. */
vg_assert(VG_(xml_output_sink).fd == 2);
tmp_xml_fd = 2;
@@ -852,13 +824,12 @@
but that is likely to confuse the hell out of users, which is
distinctly Ungood. */
if (VG_(clo_xml) && tmp_xml_fd == -1) {
- VG_(umsg)(
+ VG_(msgf_bad_option)(
+ "--xml=yes, but no XML destination specified",
"--xml=yes has been specified, but there is no XML output\n"
"destination. You must specify an XML output destination\n"
- "using --xml-fd=, --xml-file= or --xml=socket=.\n" );
- /* FIXME: this is really a misuse of VG_(err_bad_option). */
- VG_(err_bad_option)(
- "--xml=yes, but no XML destination specified");
+ "using --xml-fd, --xml-file or --xml-socket.\n"
+ );
}
// Finalise the output fds: the log fd ..
@@ -1861,7 +1832,7 @@
//--------------------------------------------------------------
VG_(debugLog)(1, "main", "Print help and quit, if requested\n");
if (need_help) {
- usage_NORETURN(/*--help-debug?*/2 == need_help);
+ usage_NORETURN(/*--help-debug?*/need_help >= 2);
}
//--------------------------------------------------------------
Modified: branches/MSG2/coregrind/m_options.c
===================================================================
--- branches/MSG2/coregrind/m_options.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/m_options.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -95,42 +95,9 @@
/*====================================================================*/
-/*=== Command line errors ===*/
+/*=== File expanstion ===*/
/*====================================================================*/
-static void revert_to_stderr ( void )
-{
- VG_(log_output_sink).fd = 2; /* stderr */
- VG_(log_output_sink).is_socket = False;
-}
-
-__attribute__((noreturn))
-void VG_(err_bad_option) ( Char* opt )
-{
- revert_to_stderr();
- VG_(msgf)("Bad option '%s'; aborting.\n", opt);
- VG_(msgf)("Use --help for more information.\n");
- VG_(exit)(1);
-}
-
-__attribute__((noreturn))
-void VG_(err_missing_prog) ( void )
-{
- revert_to_stderr();
- VG_(msgf)("no program specified\n");
- VG_(msgf)("Use --help for more information.\n");
- VG_(exit)(1);
-}
-
-__attribute__((noreturn))
-void VG_(err_config_error) ( Char* msg )
-{
- revert_to_stderr();
- VG_(msgf)("Startup or configuration error:\n %s\n", msg);
- VG_(msgf)("Unable to start up properly. Giving up.\n");
- VG_(exit)(1);
-}
-
// Copies the string, prepending it with the startup working directory, and
// expanding %p and %q entries. Returns a new, malloc'd string.
Char* VG_(expand_file_name)(Char* option_name, Char* format)
@@ -258,12 +225,10 @@
VG_(strcpy)(opt, option_name);
VG_(strcat)(opt, "=");
VG_(strcat)(opt, format);
- VG_(err_bad_option)(opt);
+ VG_(msgf_bad_option)(opt, ""); // MMM: check this case
}
}
-
-
/*--------------------------------------------------------------------*/
-/*--- end m_options.c ---*/
+/*--- end ---*/
/*--------------------------------------------------------------------*/
Modified: branches/MSG2/coregrind/m_replacemalloc/replacemalloc_core.c
===================================================================
--- branches/MSG2/coregrind/m_replacemalloc/replacemalloc_core.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/m_replacemalloc/replacemalloc_core.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -58,12 +58,9 @@
VG_(clo_alignment) > 4096 ||
VG_(log2)( VG_(clo_alignment) ) == -1 /* not a power of 2 */)
{
- VG_(message)(Vg_UserMsg,
- "Invalid --alignment= setting. "
- "Should be a power of 2, >= %d, <= 4096.\n",
- VG_MIN_MALLOC_SZB
- );
- VG_(err_bad_option)("--alignment");
+ VG_(msgf_bad_option)(arg,
+ "Alignment must be a power of 2 in the range %d..4096.\n",
+ VG_MIN_MALLOC_SZB);
}
}
Modified: branches/MSG2/coregrind/pub_core_libcprint.h
===================================================================
--- branches/MSG2/coregrind/pub_core_libcprint.h 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/pub_core_libcprint.h 2009-08-23 22:46:04 UTC (rev 10864)
@@ -54,6 +54,16 @@
m_main during startup. */
void VG_(elapsed_wallclock_time) ( /*OUT*/HChar* buf );
+/* Call this if the executable is missing. This function prints an
+ error message, then shuts down the entire system. */
+__attribute__((noreturn))
+extern void VG_(err_missing_prog) ( void );
+
+/* Similarly - complain and stop if there is some kind of config error. */
+__attribute__((noreturn))
+extern void VG_(err_config_error) ( Char* msg );
+
+
#endif // __PUB_CORE_LIBCPRINT_H
/*--------------------------------------------------------------------*/
Modified: branches/MSG2/coregrind/pub_core_options.h
===================================================================
--- branches/MSG2/coregrind/pub_core_options.h 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/coregrind/pub_core_options.h 2009-08-23 22:46:04 UTC (rev 10864)
@@ -170,19 +170,6 @@
.dSYM directories as necessary? */
extern Bool VG_(clo_dsymutil);
-/* --------- Functions --------- */
-
-/* Call this if the executable is missing. This function prints an
- error message, then shuts down the entire system. */
-__attribute__((noreturn))
-extern void VG_(err_missing_prog) ( void );
-
-/* Similarly - complain and stop if there is some kind of config
- error. */
-__attribute__((noreturn))
-extern void VG_(err_config_error) ( Char* msg );
-
-
#endif // __PUB_CORE_OPTIONS_H
/*--------------------------------------------------------------------*/
Modified: branches/MSG2/docs/xml/manual-core.xml
===================================================================
--- branches/MSG2/docs/xml/manual-core.xml 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/docs/xml/manual-core.xml 2009-08-23 22:46:04 UTC (rev 10864)
@@ -596,7 +596,8 @@
<term><option>-h --help</option></term>
<listitem>
<para>Show help for all options, both for the core and for the
- selected tool.</para>
+ selected tool. If the option is repeated it is equivalent to giving
+ <option>--help-debug</option>.</para>
</listitem>
</varlistentry>
Modified: branches/MSG2/include/pub_tool_libcprint.h
===================================================================
--- branches/MSG2/include/pub_tool_libcprint.h 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/include/pub_tool_libcprint.h 2009-08-23 22:46:04 UTC (rev 10864)
@@ -32,19 +32,9 @@
#define __PUB_TOOL_LIBCPRINT_H
/* ---------------------------------------------------------------------
- Basic printing
+ Formatting functions
------------------------------------------------------------------ */
-/* Note that they all output to the file descriptor given by the
- --log-fd/--log-file/--log-socket argument, which defaults to 2
- (stderr). Hence no need for VG_(fprintf)().
-*/
-extern UInt VG_(printf) ( const HChar *format, ... )
- PRINTF_CHECK(1, 2);
-
-extern UInt VG_(vprintf) ( const HChar *format, va_list vargs )
- PRINTF_CHECK(1, 0);
-
extern UInt VG_(sprintf) ( Char* buf, const HChar* format, ... )
PRINTF_CHECK(2, 3);
@@ -65,29 +55,22 @@
void* opaque,
const HChar* format, va_list vargs );
-/* These are the same as the non "_xml" versions above, except the
- output goes on the selected XML output channel instead of the
- normal one.
-*/
-extern UInt VG_(printf_xml) ( const HChar *format, ... )
- PRINTF_CHECK(1, 2);
-
-extern UInt VG_(vprintf_xml) ( const HChar *format, va_list vargs )
- PRINTF_CHECK(1, 0);
-
-// Just like VG_(printf_xml) but without the PRINTF_CHECK, so it can be used
-// with our non-standard %t format specifier.
-extern UInt VG_(printf_xml_no_f_c) ( const HChar *format, ... );
-
// Percentify n/m with d decimal places. Includes the '%' symbol at the end.
// Right justifies in 'buf'.
extern void VG_(percentify)(ULong n, ULong m, UInt d, Int n_buf, char buf[]);
/* ---------------------------------------------------------------------
- Messages for the user
+ Output-printing functions
------------------------------------------------------------------ */
+/* Note that almost all output goes to the file descriptor given by the
+ --log-fd/--log-file/--log-socket argument, which defaults to 2 (stderr).
+ (Except that some text always goes to stdout/stderr at startup, and
+ debugging messages always go to stderr.) Hence no need for
+ VG_(fprintf)().
+*/
+
/* No, really. I _am_ that strange. */
#define OINK(nnn) VG_(message)(Vg_DebugMsg, "OINK %d\n",nnn)
@@ -106,29 +89,64 @@
}
VgMsgKind;
-/* Send a single-part message. The format specification may contain
- any ISO C format specifier or %t. No attempt is made to let the
- compiler verify consistency of the format string and the argument
- list. */
+// These print output that isn't prefixed with anything, and should be
+// used in very few cases, such as printing usage messages.
+extern UInt VG_(printf) ( const HChar *format, ... )
+ PRINTF_CHECK(1, 2);
+extern UInt VG_(vprintf) ( const HChar *format, va_list vargs )
+ PRINTF_CHECK(1, 0);
+
+// The "_no_f_c" functions here are just like their non-"_no_f_c" counterparts
+// but without the PRINTF_CHECK, so they can be used with our non-standard %t
+// format specifier.
+
+// These are the same as the non "_xml" versions above, except the
+// output goes on the selected XML output channel instead of the
+// normal one.
+extern UInt VG_(printf_xml) ( const HChar *format, ... )
+ PRINTF_CHECK(1, 2);
+extern UInt VG_(printf_xml_no_f_c) ( const HChar *format, ... );
+extern UInt VG_(vprintf_xml) ( const HChar *format, va_list vargs )
+ PRINTF_CHECK(1, 0);
+
+// Send a single-part message.
+// MMM: make private?
+extern UInt VG_(message) ( VgMsgKind kind, const HChar* format, ... )
+ PRINTF_CHECK(2, 3);
extern UInt VG_(message_no_f_c)( VgMsgKind kind, const HChar* format, ... );
-/* Send a single-part message. The format specification may contain
- any ISO C format specifier. The gcc compiler will verify
- consistency of the format string and the argument list. */
-// MMM: make private?
-extern UInt VG_(message)( VgMsgKind kind, const HChar* format, ... )
- PRINTF_CHECK(2, 3);
-
// MMM: remove?
-extern UInt VG_(vmessage)( VgMsgKind kind, const HChar* format, va_list vargs )
- PRINTF_CHECK(2, 0);
+extern UInt VG_(vmessage) ( VgMsgKind kind, const HChar* format,
+ va_list vargs )
+ PRINTF_CHECK(2, 0);
// Short-cuts for VG_(message)().
// MMM: rename these as vmsg/fmsg/etc eventually
+// This is used for start-up failures that occur before the preamble is
+// printed, eg. due to a bad executable.
extern UInt VG_(msgf) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used if an option was bad for some reason. Note: don't use it just
+// because an option was unrecognised -- return 'False' from
+// VG_(tdict).tool_process_cmd_line_option) to indicate that -- use it if eg.
+// an option was given an inappropriate argument. This function prints an
+// error message, then shuts down the entire system.
+__attribute__((noreturn))
+extern void VG_(msgf_bad_option) ( HChar* opt,
+ const HChar* format, ... ) PRINTF_CHECK(2, 3);
+
+// This is used for messages that are interesting to the user: info about
+// their program (eg. preamble, tool error messages, postamble) or stuff they
+// requested.
extern UInt VG_(msgu) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used for messages that are about Valgrind's execution, and of less
+// direct interest to the user.
extern UInt VG_(msgv) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
+
+// This is used for debugging messages that are only of use to developers.
extern UInt VG_(msgd) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
-// MMM: get rid of these to begin with
+
+// MMM: synonyms; get rid of these to begin with
extern UInt VG_(umsg) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
extern UInt VG_(dmsg) ( const HChar* format, ... ) PRINTF_CHECK(1, 2);
Modified: branches/MSG2/include/pub_tool_options.h
===================================================================
--- branches/MSG2/include/pub_tool_options.h 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/include/pub_tool_options.h 2009-08-23 22:46:04 UTC (rev 10864)
@@ -77,7 +77,7 @@
Long n = VG_(strtoll10)( val, &s ); \
(qq_var) = n; \
/* Check for non-numeralness, or overflow. */ \
- if ('\0' != s[0] || (qq_var) != n) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0] || (qq_var) != n) VG_(msgf_bad_option)(qq_arg, ""); \
True; \
}) \
)
@@ -91,15 +91,16 @@
Char* s; \
Long n = VG_(strtoll##qq_base)( val, &s ); \
(qq_var) = n; \
+ /* MMM: separate the two cases, and explain the problem; likewise */ \
+ /* for all the other macros in this file. */ \
/* Check for non-numeralness, or overflow. */ \
/* Nb: it will overflow if qq_var is unsigned and qq_val is negative! */ \
- if ('\0' != s[0] || (qq_var) != n) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0] || (qq_var) != n) VG_(msgf_bad_option)(qq_arg, ""); \
/* Check bounds. */ \
if ((qq_var) < (qq_lo) || (qq_var) > (qq_hi)) { \
- VG_(message)(Vg_UserMsg, \
- "'%s' argument must be between %lld and %lld\n", \
- (qq_option), (Long)(qq_lo), (Long)(qq_hi)); \
- VG_(err_bad_option)(qq_arg); \
+ VG_(msgf_bad_option)(qq_arg, \
+ "'%s' argument must be between %lld and %lld\n", \
+ (qq_option), (Long)(qq_lo), (Long)(qq_hi)); \
} \
True; \
}) \
@@ -124,7 +125,7 @@
double n = VG_(strtod)( val, &s ); \
(qq_var) = n; \
/* Check for non-numeralness */ \
- if ('\0' != s[0]) VG_(err_bad_option)(qq_arg); \
+ if ('\0' != s[0]) VG_(msgf_bad_option)(qq_arg, ""); \
True; \
}) \
)
@@ -165,15 +166,6 @@
extern Bool VG_(clo_show_below_main);
-/* Call this if a recognised option was bad for some reason. Note:
- don't use it just because an option was unrecognised -- return
- 'False' from VG_(tdict).tool_process_cmd_line_option) to indicate that --
- use it if eg. an option was given an inappropriate argument.
- This function prints an error message, then shuts down the entire system.
- It returns a Bool so it can be used in the _CLO_ macros. */
-__attribute__((noreturn))
-extern void VG_(err_bad_option) ( Char* opt );
-
/* Used to expand file names. "option_name" is the option name, eg.
"--log-file". 'format' is what follows, eg. "cachegrind.out.%p". In
'format':
Modified: branches/MSG2/include/pub_tool_tooliface.h
===================================================================
--- branches/MSG2/include/pub_tool_tooliface.h 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/include/pub_tool_tooliface.h 2009-08-23 22:46:04 UTC (rev 10864)
@@ -378,6 +378,11 @@
// record the option as well. Nb: tools can assume that the argv will
// never disappear. So they can, for example, store a pointer to a string
// within an option, rather than having to make a copy.
+ //
+ // Options (and combinations of options) should be checked in this function
+ // if possible rather than in post_clo_init(), and if they are bad then
+ // VG_(err_bad_option)() should be called. This ensures that the messaging
+ // is consistent with command line option errors from the core.
Bool (*process_cmd_line_option)(Char* argv),
// Print out command line usage for options for normal tool operation.
Modified: branches/MSG2/massif/ms_main.c
===================================================================
--- branches/MSG2/massif/ms_main.c 2009-08-23 11:17:25 UTC (rev 10863)
+++ branches/MSG2/massif/ms_main.c 2009-08-23 22:46:04 UTC (rev 10864)
@@ -423,7 +423,12 @@
else if VG_BINT_CLO(arg, "--heap-admin", clo_heap_admin, 0, 1024) {}
else if VG_BINT_CLO(arg, "--depth", clo_depth, 1, MAX_DEPTH) {}
- else if VG_DBL_CLO(arg, "--threshold", clo_threshold) {}
+ else if VG_DBL_CLO(arg, "--threshold", clo_threshold) {
+ if (clo_threshold < 0 || clo_threshold > 100) {
+ VG_(msgf_bad_option)(arg,
+ "--threshold must be between 0.0 and 100.0\n");
+ }
+ }
else if VG_DBL_CLO(arg, "--peak-inaccuracy", clo_peak_inaccuracy) {}
@@ -443,6 +448,8 @@
else if VG_STR_CLO(arg, "--massif-out-file", clo_massif_out_file) {}
else
+ // MMM: should handle this in the core if needs_malloc_replacement is
+ // set.
return VG_(replacement_malloc_process_cmd_line_option)(arg);
return True;
@@ -2264,12 +2271,6 @@
{
Int i;
- // Check options.
- if (clo_threshold < 0 || clo_threshold > 100) {
- VG_(umsg)("--threshold must be between 0.0 and 100.0\n");
- VG_(err_bad_option)("--threshold");
- }
-
// If we have --heap=no, set --heap-admin to zero, just to make sure we
// don't accidentally use a non-zero heap-admin size somewhere.
if (!clo_heap) {
|