From: Julian Seward Date: Thu, 27 Jun 2013 17:39:15 +0000 (+0000) Subject: Minimal changes needed to make this suitable for trunk: X-Git-Tag: svn/VALGRIND_3_9_0~251^2~2 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=e3116970fd237affe1fc429f9fe29ee1a23e556f;p=thirdparty%2Fvalgrind.git 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. git-svn-id: svn://svn.valgrind.org/valgrind/branches/DISRV@13431 --- diff --git a/auxprogs/valgrind-di-server.c b/auxprogs/valgrind-di-server.c index 4fa0309744..06dfdf84b3 100644 --- a/auxprogs/valgrind-di-server.c +++ b/auxprogs/valgrind-di-server.c @@ -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 @@ static UInt calc_gnu_debuglink_crc32(/*OUT*/Bool* ok, int fd, ULong size) *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); diff --git a/coregrind/m_debuginfo/image.c b/coregrind/m_debuginfo/image.c index b176678a76..74beb797dc 100644 --- a/coregrind/m_debuginfo/image.c +++ b/coregrind/m_debuginfo/image.c @@ -179,6 +179,18 @@ static Int my_write ( Int fd, UChar* buf, Int len ) } } +/* 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 @@ static Int my_write ( Int fd, UChar* buf, Int len ) 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 @@ if (0) VG_(printf)("CLIENT: send %c%c%c%c\n", req->data[0], req->data[1], req->d 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 @@ static void set_CEnt ( DiImage* img, UInt entNo, DiOffT off ) /* 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_(printf)("XXXXXXXX (tot %lld) read %ld offset %lld %u\n", 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 @@ UInt ML_(img_calc_gnu_debuglink_crc32)(DiImage* img) if (req) free_Frame(req); if (res) free_Frame(res); // FIXME: now what? + give_up(); + /* NOTREACHED */ vg_assert(0); } /*NOTREACHED*/ diff --git a/coregrind/m_debuginfo/readelf.c b/coregrind/m_debuginfo/readelf.c index 319c453775..fc76178877 100644 --- a/coregrind/m_debuginfo/readelf.c +++ b/coregrind/m_debuginfo/readelf.c @@ -1131,7 +1131,12 @@ DiImage* open_debug_file( const HChar* name, const HChar* buildid, UInt crc, /* 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 @@ DiImage* find_debug_file( struct _DebugInfo* di, 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 @@ DiImage* find_debug_file( struct _DebugInfo* di, 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 @@ DiImage* find_debug_file( struct _DebugInfo* di, } +/* 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 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) /* 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 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) 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 diff --git a/coregrind/m_main.c b/coregrind/m_main.c index fcb8fb2ddb..531a37fdbe 100644 --- a/coregrind/m_main.c +++ b/coregrind/m_main.c @@ -178,6 +178,9 @@ static void usage_NORETURN ( Bool debug_help ) " 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 @@ void main_process_cmd_line_options ( /*OUT*/Bool* logging_to_fd, 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)) {} diff --git a/coregrind/m_options.c b/coregrind/m_options.c index f249317e4c..b8e3cac583 100644 --- a/coregrind/m_options.c +++ b/coregrind/m_options.c @@ -81,6 +81,7 @@ Int VG_(clo_n_fullpath_after) = 0; 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 diff --git a/coregrind/pub_core_options.h b/coregrind/pub_core_options.h index ed46455094..58e19dd109 100644 --- a/coregrind/pub_core_options.h +++ b/coregrind/pub_core_options.h @@ -134,6 +134,13 @@ extern const HChar* VG_(clo_extra_debuginfo_path); "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);