|
From: <sv...@va...> - 2013-06-27 17:39:30
|
sewardj 2013-06-27 18:39:15 +0100 (Thu, 27 Jun 2013)
New Revision: 13431
Log:
Minimal changes needed to make this suitable for trunk:
* add a new flag --allow-mismatched-debuginfo to override the
CRC32/build-id checks, if needed
* tidy up logic for finding files on the --extra-debuginfo-path
and at the --debuginfo-server
* don't assert if connection to the debuginfo server is lost;
instead print a reasonable message and quit.
Modified files:
branches/DISRV/auxprogs/valgrind-di-server.c
branches/DISRV/coregrind/m_debuginfo/image.c
branches/DISRV/coregrind/m_debuginfo/readelf.c
branches/DISRV/coregrind/m_main.c
branches/DISRV/coregrind/m_options.c
branches/DISRV/coregrind/pub_core_options.h
Modified: branches/DISRV/coregrind/m_main.c (+6 -0)
===================================================================
--- branches/DISRV/coregrind/m_main.c 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/coregrind/m_main.c 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -178,6 +178,9 @@
" well known search paths.\n"
" --debuginfo-server=ipaddr:port also query this server\n"
" (valgrind-di-server) for debug symbols\n"
+" --allow-mismatched-debuginfo=no|yes [no]\n"
+" for the above two flags only, accept debuginfo\n"
+" objects that don't \"match\" the main object\n"
" --smc-check=none|stack|all|all-non-file [stack]\n"
" checks for self-modifying code: none, only for\n"
" code found in stacks, for all code, or for all\n"
@@ -681,6 +684,9 @@
else if VG_STR_CLO(arg, "--debuginfo-server",
VG_(clo_debuginfo_server)) {}
+ else if VG_BOOL_CLO(arg, "--allow-mismatched-debuginfo",
+ VG_(clo_allow_mismatched_debuginfo)) {}
+
else if VG_STR_CLO(arg, "--xml-user-comment",
VG_(clo_xml_user_comment)) {}
Modified: branches/DISRV/auxprogs/valgrind-di-server.c (+15 -0)
===================================================================
--- branches/DISRV/auxprogs/valgrind-di-server.c 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/auxprogs/valgrind-di-server.c 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -4,6 +4,15 @@
/*--- valgrind-di-server.c ---*/
/*--------------------------------------------------------------------*/
+/* To build for an x86_64-linux host:
+ gcc -g -Wall -O -o valgrind-di-server \
+ auxprogs/valgrind-di-server.c -Icoregrind -Iinclude \
+ -IVEX/pub -DVGO_linux -DVGA_amd64
+
+ To build for an x86 (32-bit) host
+ The same, except change -DVGA_amd64 to -DVGA_x86
+*/
+
/*
This file is part of Valgrind, a dynamic binary instrumentation
framework.
@@ -33,6 +42,9 @@
/* This code works (just), but it's a mess. Cleanups (also for
coregrind/m_debuginfo/image.c):
+ * Build this file for the host arch, not the target. But how?
+ Even Tromey had difficulty figuring out how to do that.
+
* Change the use of pread w/ fd to FILE*, for the file we're
serving. Or, at least, put a loop around the pread uses
so that it works correctly in the case where pread reads more
@@ -627,6 +639,9 @@
*ok = False;
return 0;
}
+ /* this is a kludge .. we should loop around pread and deal
+ with short reads, for whatever reason */
+ assert(nRead == avail);
UInt i;
for (i = 0; i < (UInt)nRead; i++)
crc = crc32_table[(crc ^ buf[i]) & 0xff] ^ (crc >> 8);
Modified: branches/DISRV/coregrind/pub_core_options.h (+7 -0)
===================================================================
--- branches/DISRV/coregrind/pub_core_options.h 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/coregrind/pub_core_options.h 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -134,6 +134,13 @@
"d.d.d.d:d", where d is one or more digits. */
extern const HChar* VG_(clo_debuginfo_server);
+/* Do we allow reading debuginfo from debuginfo objects that don't
+ match (in some sense) the main object? This is dangerous, so the
+ default is NO (False). In any case it applies only to objects
+ found either in _extra_debuginfo_path or via the
+ _debuginfo_server. */
+extern Bool VG_(clo_allow_mismatched_debuginfo);
+
/* DEBUG: print generated code? default: 00000000 ( == NO ) */
extern UChar VG_(clo_trace_flags);
Modified: branches/DISRV/coregrind/m_debuginfo/readelf.c (+110 -16)
===================================================================
--- branches/DISRV/coregrind/m_debuginfo/readelf.c 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/coregrind/m_debuginfo/readelf.c 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -1131,7 +1131,12 @@
/* Try to find a separate debug file for a given object file. If
- found, return its DiImage, which should be freed by the caller. */
+ found, return its DiImage, which should be freed by the caller. If
+ |buildid| is non-NULL, then a debug object matching it is
+ acceptable. If |buildid| is NULL or doesn't specify a findable
+ debug object, then we look in various places to find a file with
+ the specified CRC. And if that doesn't work out then we give
+ up. */
static
DiImage* find_debug_file( struct _DebugInfo* di,
const HChar* objpath, const HChar* buildid,
@@ -1166,11 +1171,11 @@
debugpath = ML_(dinfo_zalloc)(
"di.fdf.3",
- VG_(strlen)(objdir) + VG_(strlen)(debugname) + 32 +
- (extrapath ? VG_(strlen)(extrapath) : 0));
+ VG_(strlen)(objdir) + VG_(strlen)(debugname) + 64
+ + (extrapath ? VG_(strlen)(extrapath) : 0)
+ + (serverpath ? VG_(strlen)(serverpath) : 0));
VG_(sprintf)(debugpath, "%s/%s", objdir, debugname);
-
dimg = open_debug_file(debugpath, NULL, crc, rel_ok, NULL);
if (dimg != NULL) goto dimg_ok;
@@ -1184,13 +1189,20 @@
if (extrapath) {
VG_(sprintf)(debugpath, "%s%s/%s", extrapath,
- objdir, debugname);
+ objdir, debugname);
dimg = open_debug_file(debugpath, NULL, crc, rel_ok, NULL);
if (dimg != NULL) goto dimg_ok;
}
if (serverpath) {
- dimg = open_debug_file(debugname, NULL, crc, rel_ok, serverpath);
+ /* When looking on the debuginfo server, always just pass the
+ basename. */
+ const HChar* basename = debugname;
+ if (VG_(strstr)(basename, "/") != NULL) {
+ basename = VG_(strrchr)(basename, '/') + 1;
+ }
+ VG_(sprintf)(debugpath, "%s on %s", basename, serverpath);
+ dimg = open_debug_file(basename, NULL, crc, rel_ok, serverpath);
if (dimg) goto dimg_ok;
}
@@ -1212,6 +1224,77 @@
}
+/* Try to find a separate debug file for a given object file, in a
+ hacky and dangerous way: check only the --extra-debuginfo-path and
+ the --debuginfo-server. And don't do a consistency check. */
+static
+DiImage* find_debug_file_ad_hoc( struct _DebugInfo* di,
+ const HChar* objpath )
+{
+ const HChar* extrapath = VG_(clo_extra_debuginfo_path);
+ const HChar* serverpath = VG_(clo_debuginfo_server);
+
+ DiImage* dimg = NULL; /* the img that we found */
+ HChar* debugpath = NULL; /* where we found it */
+
+ HChar *objdir = ML_(dinfo_strdup)("di.fdfah.1", objpath);
+ HChar *objdirptr;
+
+ if ((objdirptr = VG_(strrchr)(objdir, '/')) != NULL)
+ *objdirptr = '\0';
+
+ debugpath = ML_(dinfo_zalloc)(
+ "di.fdfah.3",
+ VG_(strlen)(objdir) + 64
+ + (extrapath ? VG_(strlen)(extrapath) : 0)
+ + (serverpath ? VG_(strlen)(serverpath) : 0));
+
+ if (extrapath) {
+ VG_(sprintf)(debugpath, "%s/%s", extrapath, objpath);
+ dimg = ML_(img_from_local_file)(debugpath);
+ if (dimg != NULL) {
+ if (VG_(clo_verbosity) > 1) {
+ VG_(message)(Vg_DebugMsg, " Using (POSSIBLY MISMATCHED) %s\n",
+ debugpath);
+ }
+ goto dimg_ok;
+ }
+ }
+ if (serverpath) {
+ /* When looking on the debuginfo server, always just pass the
+ basename. */
+ const HChar* basename = objpath;
+ if (VG_(strstr)(basename, "/") != NULL) {
+ basename = VG_(strrchr)(basename, '/') + 1;
+ }
+ VG_(sprintf)(debugpath, "%s on %s", basename, serverpath);
+ dimg = ML_(img_from_di_server)(basename, serverpath);
+ if (dimg != NULL) {
+ if (VG_(clo_verbosity) > 1) {
+ VG_(message)(Vg_DebugMsg, " Using (POSSIBLY MISMATCHED) %s\n",
+ debugpath);
+ }
+ goto dimg_ok;
+ }
+ }
+
+ dimg_ok:
+
+ ML_(dinfo_free)(objdir);
+
+ if (dimg != NULL) {
+ vg_assert(debugpath);
+ TRACE_SYMTAB("\n");
+ TRACE_SYMTAB("------ Found an ad_hoc debuginfo file: %s\n", debugpath);
+ }
+
+ if (debugpath)
+ ML_(dinfo_free)(debugpath);
+
+ return dimg;
+}
+
+
static DiOffT INDEX_BIS ( DiOffT base, UWord idx, UWord scale ) {
// This is a bit stupid. Really, idx and scale ought to be
// 64-bit quantities, always.
@@ -2218,7 +2301,12 @@
/* Look for a build-id */
HChar* buildid = find_buildid(mimg, False, False);
- /* Look for a debug image */
+ /* Look for a debug image that matches either the build-id or
+ the debuglink-CRC32 in the main image. If the main image
+ doesn't contain either of those then this won't even bother
+ to try looking. This looks in all known places, including
+ the --extra-debuginfo-path if specified and on the
+ --debuginfo-server if specified. */
if (buildid != NULL || debuglink_escn.img != NULL) {
/* Do have a debuglink section? */
if (debuglink_escn.img != NULL) {
@@ -2251,16 +2339,22 @@
buildid = NULL; /* paranoia */
}
-# if defined(VGPV_arm_linux_android)
- if (dimg == NULL && VG_(clo_debuginfo_server) != NULL) {
- HChar* basename = di->fsm.filename;
- if (VG_(strstr)(basename, "/") != NULL)
- basename = VG_(strrchr)(basename, '/') + 1;
- VG_(printf)("XXXXXXXXXXX trying ad-hoc %s\n", basename);
- dimg = ML_(img_from_di_server)(basename,
- VG_(clo_debuginfo_server));
+ /* As a last-ditch measure, try looking for in the
+ --extra-debuginfo-path and/or on the --debuginfo-server, but
+ only in the case where --allow-mismatched-debuginfo=yes.
+ This is dangerous in that (1) it gives no assurance that the
+ debuginfo object matches the main one, and hence (2) we will
+ very likely get an assertion in the code below, if indeed
+ there is a mismatch. Hence it is disabled by default
+ (--allow-mismatched-debuginfo=no). Nevertheless it's
+ sometimes a useful way of getting out of a tight spot.
+
+ Note that we're ignoring the name in the .gnu_debuglink
+ section here, and just looking for a file of the same name
+ either the extra-path or on the server. */
+ if (dimg == NULL && VG_(clo_allow_mismatched_debuginfo)) {
+ dimg = find_debug_file_ad_hoc( di, di->fsm.filename );
}
-# endif
/* TOPLEVEL */
/* If we were successful in finding a debug image, pull various
Modified: branches/DISRV/coregrind/m_options.c (+1 -0)
===================================================================
--- branches/DISRV/coregrind/m_options.c 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/coregrind/m_options.c 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -81,6 +81,7 @@
const HChar* VG_(clo_fullpath_after)[VG_CLO_MAX_FULLPATH_AFTER];
const HChar* VG_(clo_extra_debuginfo_path) = NULL;
const HChar* VG_(clo_debuginfo_server) = NULL;
+Bool VG_(clo_allow_mismatched_debuginfo) = False;
UChar VG_(clo_trace_flags) = 0; // 00000000b
Bool VG_(clo_profyle_sbs) = False;
UChar VG_(clo_profyle_flags) = 0; // 00000000b
Modified: branches/DISRV/coregrind/m_debuginfo/image.c (+31 -12)
===================================================================
--- branches/DISRV/coregrind/m_debuginfo/image.c 2013-06-25 13:42:52 +01:00 (rev 13430)
+++ branches/DISRV/coregrind/m_debuginfo/image.c 2013-06-27 18:39:15 +01:00 (rev 13431)
@@ -179,6 +179,18 @@
}
}
+/* If we lost communication with the remote server, just give up.
+ Recovering is too difficult. */
+static void give_up(void)
+{
+ VG_(umsg)("\n");
+ VG_(umsg)("Valgrind: Lost communication with the remote "
+ "debuginfo server.\n");
+ VG_(umsg)("Valgrind: I can't recover from that. Giving up. Sorry.\n");
+ VG_(umsg)("\n");
+ VG_(exit)(1);
+ /*NOTREACHED*/
+}
/* "Do" a transaction: that is, send the given frame to the server and
return the frame it sends back. Caller owns the resulting frame
@@ -186,7 +198,9 @@
some reason. */
static Frame* do_transaction ( Int sd, Frame* req )
{
-if (0) VG_(printf)("CLIENT: send %c%c%c%c\n", req->data[0], req->data[1], req->data[2], req->data[3]);
+ if (0) VG_(printf)("CLIENT: send %c%c%c%c\n",
+ req->data[0], req->data[1], req->data[2], req->data[3]);
+
/* What goes on the wire is:
adler(le32) n_data(le32) data[0 .. n_data-1]
where the checksum covers n_data as well as data[].
@@ -224,7 +238,8 @@
r = my_read(sd, res->data, res->n_data);
if (r != rd_len) return NULL;
-if (0) VG_(printf)("CLIENT: recv %c%c%c%c\n", res->data[0], res->data[1], res->data[2], res->data[3]);
+ if (0) VG_(printf)("CLIENT: recv %c%c%c%c\n",
+ res->data[0], res->data[1], res->data[2], res->data[3]);
/* Compute the checksum for the received data, and check it. */
adler = VG_(adler32)(0, NULL, 0); // initial value
@@ -417,16 +432,16 @@
/* So, read off .. off+len-1 into the entry. */
CEnt* ce = img->ces[entNo];
-if (0) {
-static UInt t_last = 0;
-static ULong nread = 0;
-UInt now = VG_(read_millisecond_timer)();
-UInt delay = now - t_last;
-t_last = now;
-nread += len;
-VG_(printf)("XXXXXXXX (tot %lld) read %ld offset %lld %u\n",
- nread, len, off, delay);
-}
+ if (0) {
+ static UInt t_last = 0;
+ static ULong nread = 0;
+ UInt now = VG_(read_millisecond_timer)();
+ UInt delay = now - t_last;
+ t_last = now;
+ nread += len;
+ VG_(printf)("XXXXXXXX (tot %lld) read %ld offset %lld %u\n",
+ nread, len, off, delay);
+ }
if (img->source.is_local) {
// Simple: just read it
@@ -487,6 +502,8 @@
VG_(umsg)("set_CEnt (reading data from DI server): fail: "
"server unexpectedly closed the connection\n");
}
+ give_up();
+ /* NOTREACHED */
vg_assert(0);
end_of_else_clause:
{}
@@ -968,6 +985,8 @@
if (req) free_Frame(req);
if (res) free_Frame(res);
// FIXME: now what?
+ give_up();
+ /* NOTREACHED */
vg_assert(0);
}
/*NOTREACHED*/
|