From: Julian Seward Date: Tue, 19 Aug 2008 07:03:04 +0000 (+0000) Subject: Presently, Valgrind (non-client) code that wants to use the stat X-Git-Tag: svn/VALGRIND_3_4_0~288 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e5150447d708b1d48f66d6ac273f8e62b515f07e;p=thirdparty%2Fvalgrind.git Presently, Valgrind (non-client) code that wants to use the stat family of syscalls is impossible to write in a way that's portable and correct. On some targets (eg x86-linux) you need to do sys_stat64 and receive the results in a 'struct vki_stat64'. But on other targets (eg amd64-linux) neither sys_stat64 nor 'struct vki_stat64' exist. This commit adds a new type, 'struct vg_stat', which contains 64 bit fields in all the right places, and makes VG_(stat) and VG_(fstat) use it. This means callers to the two functions no longer need to worry about the is-it-64-bit-clean-or-not question, since these routines reformat the received data into a'struct vg_stat'. Kind of like what glibc must have been doing for decades. This (indirectly) fixes a bug on x86-linux, in which m_debuginfo would sometimes fail to read debug info, due to VG_(di_notify_mmap) using VG_(stat) (hence sys_stat) on the file, which failed, and when in fact it should have used sys_stat64. Bug reported and tracked down by Marc-Oliver Straub. git-svn-id: svn://svn.valgrind.org/valgrind/trunk@8522 --- diff --git a/coregrind/m_commandline.c b/coregrind/m_commandline.c index 4077b07de4..c2256cf883 100644 --- a/coregrind/m_commandline.c +++ b/coregrind/m_commandline.c @@ -57,7 +57,7 @@ static HChar* read_dot_valgrindrc ( HChar* dir ) { Int n; SysRes fd; - Int size; + Long size; HChar* f_clo = NULL; HChar filename[VKI_PATH_MAX]; diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c index ebb552c96c..d45a647547 100644 --- a/coregrind/m_debuginfo/debuginfo.c +++ b/coregrind/m_debuginfo/debuginfo.c @@ -491,8 +491,8 @@ void VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV ) Int nread; HChar buf1k[1024]; Bool debug = False; - SysRes statres; - struct vki_stat statbuf; + SysRes statres; + struct vg_stat statbuf; /* In short, figure out if this mapping is of interest to us, and if so, try to guess what ld.so is doing and when/if we should @@ -522,16 +522,32 @@ void VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV ) if (debug) VG_(printf)("di_notify_mmap-2: %s\n", filename); - /* Only try to read debug information from regular files. */ + /* Only try to read debug information from regular files. */ statres = VG_(stat)(filename, &statbuf); - /* If the assert below ever fails, replace the VG_(stat)() call above */ - /* by a VG_(lstat)() call. */ + + /* stat dereferences symlinks, so we don't expect it to succeed and + yet produce something that is a symlink. */ vg_assert(statres.isError || ! VKI_S_ISLNK(statbuf.st_mode)); - if (statres.isError || ! VKI_S_ISREG(statbuf.st_mode)) - { + + /* Don't let the stat call fail silently. Filter out some known + sources of noise before complaining, though. */ + if (statres.isError) { + DebugInfo fake_di; + Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL; + if (!quiet) { + VG_(memset)(&fake_di, 0, sizeof(fake_di)); + fake_di.filename = filename; + ML_(symerr)(&fake_di, True, "failed to stat64/stat this file"); + } return; } + /* Finally, the point of all this stattery: if it's not a regular file, + don't try to read debug info from it. */ + if (! VKI_S_ISREG(statbuf.st_mode)) + return; + + /* no uses of statbuf below here. */ /* Peer at the first few bytes of the file, to see if it is an ELF */ /* object file. Ignore the file if we do not have read permission. */ diff --git a/coregrind/m_debuginfo/readelf.c b/coregrind/m_debuginfo/readelf.c index 24934a7948..04d17e5daa 100644 --- a/coregrind/m_debuginfo/readelf.c +++ b/coregrind/m_debuginfo/readelf.c @@ -859,7 +859,7 @@ static Addr open_debug_file( Char* name, UInt crc, /*OUT*/UWord* size ) { SysRes fd, sres; - struct vki_stat stat_buf; + struct vg_stat stat_buf; UInt calccrc; fd = VG_(open)(name, VKI_O_RDONLY, 0); @@ -1083,11 +1083,13 @@ Bool ML_(read_elf_debug_info) ( struct _DebugInfo* di ) return False; } - n_oimage = VG_(fsize)(fd.res); - if (n_oimage <= 0) { - ML_(symerr)(di, True, "Can't stat .so/.exe (to determine its size)?!"); - VG_(close)(fd.res); - return False; + { Long n_oimageLL = VG_(fsize)(fd.res); + if (n_oimageLL <= 0) { + ML_(symerr)(di, True, "Can't stat .so/.exe (to determine its size)?!"); + VG_(close)(fd.res); + return False; + } + n_oimage = (UWord)(ULong)n_oimageLL; } sres = VG_(am_mmap_file_float_valgrind) diff --git a/coregrind/m_libcfile.c b/coregrind/m_libcfile.c index b90cf3c373..4b080c83bd 100644 --- a/coregrind/m_libcfile.c +++ b/coregrind/m_libcfile.c @@ -47,8 +47,7 @@ static inline Bool fd_exists(Int fd) { - struct vki_stat st; - + struct vg_stat st; return VG_(fstat)(fd, &st) == 0; } @@ -139,28 +138,89 @@ OffT VG_(lseek) ( Int fd, OffT offset, Int whence ) change VG_(pread) and all other usage points. */ } -SysRes VG_(stat) ( Char* file_name, struct vki_stat* buf ) + +/* stat/fstat support. It's uggerly. We have impedance-match into a + 'struct vg_stat' in order to have a single structure that callers + can use consistently on all platforms. */ + +#define TRANSLATE_TO_vg_stat(_p_vgstat, _p_vkistat) \ + do { \ + (_p_vgstat)->st_dev = (ULong)( (_p_vkistat)->st_dev ); \ + (_p_vgstat)->st_ino = (ULong)( (_p_vkistat)->st_ino ); \ + (_p_vgstat)->st_nlink = (ULong)( (_p_vkistat)->st_nlink ); \ + (_p_vgstat)->st_mode = (UInt)( (_p_vkistat)->st_mode ); \ + (_p_vgstat)->st_uid = (UInt)( (_p_vkistat)->st_uid ); \ + (_p_vgstat)->st_gid = (UInt)( (_p_vkistat)->st_gid ); \ + (_p_vgstat)->st_rdev = (ULong)( (_p_vkistat)->st_rdev ); \ + (_p_vgstat)->st_size = (Long)( (_p_vkistat)->st_size ); \ + (_p_vgstat)->st_blksize = (ULong)( (_p_vkistat)->st_blksize ); \ + (_p_vgstat)->st_blocks = (ULong)( (_p_vkistat)->st_blocks ); \ + (_p_vgstat)->st_atime = (ULong)( (_p_vkistat)->st_atime ); \ + (_p_vgstat)->st_atime_nsec = (ULong)( (_p_vkistat)->st_atime_nsec ); \ + (_p_vgstat)->st_mtime = (ULong)( (_p_vkistat)->st_mtime ); \ + (_p_vgstat)->st_mtime_nsec = (ULong)( (_p_vkistat)->st_mtime_nsec ); \ + (_p_vgstat)->st_ctime = (ULong)( (_p_vkistat)->st_ctime ); \ + (_p_vgstat)->st_ctime_nsec = (ULong)( (_p_vkistat)->st_ctime_nsec ); \ + } while (0) + +SysRes VG_(stat) ( Char* file_name, struct vg_stat* vgbuf ) { + SysRes res; + VG_(memset)(vgbuf, 0, sizeof(*vgbuf)); # if defined(VGO_linux) - SysRes res = VG_(do_syscall2)(__NR_stat, (UWord)file_name, (UWord)buf); - return res; +# if defined(__NR_stat64) + { struct vki_stat64 buf64; + res = VG_(do_syscall2)(__NR_stat64, (UWord)file_name, (UWord)&buf64); + if (!(res.isError && res.err == VKI_ENOSYS)) { + /* Success, or any failure except ENOSYS */ + if (!res.isError) + TRANSLATE_TO_vg_stat(vgbuf, &buf64); + return res; + } + } +# endif /* if defined(__NR_stat64) */ + { struct vki_stat buf; + res = VG_(do_syscall2)(__NR_stat, (UWord)file_name, (UWord)&buf); + if (!res.isError) + TRANSLATE_TO_vg_stat(vgbuf, &buf); + return res; + } # elif defined(VGO_aix5) - SysRes res = VG_(do_syscall4)(__NR_AIX5_statx, - (UWord)file_name, - (UWord)buf, - sizeof(struct vki_stat), - VKI_STX_NORMAL); + res = VG_(do_syscall4)(__NR_AIX5_statx, + (UWord)file_name, + (UWord)buf, + sizeof(struct vki_stat), + VKI_STX_NORMAL); + if (!res.isError) + TRANSLATE_TO_vg_stat(vgbuf, &buf); return res; # else # error Unknown OS # endif } -Int VG_(fstat) ( Int fd, struct vki_stat* buf ) +Int VG_(fstat) ( Int fd, struct vg_stat* vgbuf ) { + SysRes res; + VG_(memset)(vgbuf, 0, sizeof(*vgbuf)); # if defined(VGO_linux) - SysRes res = VG_(do_syscall2)(__NR_fstat, fd, (UWord)buf); - return res.isError ? (-1) : 0; +# if defined(__NR_fstat64) + { struct vki_stat64 buf64; + res = VG_(do_syscall2)(__NR_fstat64, (UWord)fd, (UWord)&buf64); + if (!(res.isError && res.err == VKI_ENOSYS)) { + /* Success, or any failure except ENOSYS */ + if (!res.isError) + TRANSLATE_TO_vg_stat(vgbuf, &buf64); + return res.isError ? (-1) : 0; + } + } +# endif /* if defined(__NR_fstat64) */ + { struct vki_stat buf; + res = VG_(do_syscall2)(__NR_fstat, (UWord)fd, (UWord)&buf); + if (!res.isError) + TRANSLATE_TO_vg_stat(vgbuf, &buf); + return res.isError ? (-1) : 0; + } # elif defined(VGO_aix5) I_die_here; # else @@ -168,26 +228,19 @@ Int VG_(fstat) ( Int fd, struct vki_stat* buf ) # endif } -Int VG_(fsize) ( Int fd ) +#undef TRANSLATE_TO_vg_stat + + +Long VG_(fsize) ( Int fd ) { -# if defined(VGO_linux) && defined(__NR_fstat64) - struct vki_stat64 buf; - SysRes res = VG_(do_syscall2)(__NR_fstat64, fd, (UWord)&buf); - return res.isError ? (-1) : buf.st_size; -# elif defined(VGO_linux) && !defined(__NR_fstat64) - struct vki_stat buf; - SysRes res = VG_(do_syscall2)(__NR_fstat, fd, (UWord)&buf); - return res.isError ? (-1) : buf.st_size; -# elif defined(VGO_aix5) - I_die_here; -# else -# error Unknown OS -# endif + struct vg_stat buf; + Int res = VG_(fstat)( fd, &buf ); + return (res == -1) ? (-1LL) : buf.st_size; } Bool VG_(is_dir) ( HChar* f ) { - struct vki_stat buf; + struct vg_stat buf; SysRes res = VG_(stat)(f, &buf); return res.isError ? False : VKI_S_ISDIR(buf.st_mode) ? True : False; diff --git a/coregrind/m_ume.c b/coregrind/m_ume.c index 09825e2a39..bdfcfb8fea 100644 --- a/coregrind/m_ume.c +++ b/coregrind/m_ume.c @@ -669,7 +669,7 @@ VG_(pre_exec_check)(const HChar* exe_name, Int* out_fd, Bool allow_setuid) return VG_(mk_SysRes_Error)(ret); } - fsz = VG_(fsize)(fd); + fsz = (SizeT)VG_(fsize)(fd); if (fsz < bufsz) bufsz = fsz; @@ -773,7 +773,7 @@ static Int do_exec_shell_followup(Int ret, HChar* exe_name, { Char* default_interp_name = "/bin/sh"; SysRes res; - struct vki_stat st; + struct vg_stat st; if (VKI_ENOEXEC == ret) { // It was an executable file, but in an unacceptable format. Probably diff --git a/coregrind/pub_core_libcfile.h b/coregrind/pub_core_libcfile.h index aedd7b151a..bbb38bae60 100644 --- a/coregrind/pub_core_libcfile.h +++ b/coregrind/pub_core_libcfile.h @@ -46,8 +46,8 @@ extern Int VG_(fcntl) ( Int fd, Int cmd, Int arg ); /* Convert an fd into a filename */ extern Bool VG_(resolve_filename) ( Int fd, HChar* buf, Int n_buf ); -/* Return the size of a file */ -extern Int VG_(fsize) ( Int fd ); +/* Return the size of a file, or -1 in case of error */ +extern Long VG_(fsize) ( Int fd ); /* Is the file a directory? */ extern Bool VG_(is_dir) ( HChar* f ); diff --git a/include/pub_tool_libcfile.h b/include/pub_tool_libcfile.h index 9e3274b35a..81710d4d94 100644 --- a/include/pub_tool_libcfile.h +++ b/include/pub_tool_libcfile.h @@ -37,6 +37,32 @@ /* To use this file you must first include pub_tool_vki.h. */ +/* Note that VG_(stat) and VG_(fstat) write to a 'struct vg_stat*' and + not a 'struct vki_stat*' or a 'struct vki_stat64*'. 'struct + vg_stat*' is not the same as either of the vki_ versions. No + specific vki_stat{,64} kernel structure will work and is + consistently available on different architectures on Linux, so we + have to use this 'struct vg_stat' impedance-matching type + instead. */ +struct vg_stat { + ULong st_dev; + ULong st_ino; + ULong st_nlink; + UInt st_mode; + UInt st_uid; + UInt st_gid; + ULong st_rdev; + Long st_size; + ULong st_blksize; + ULong st_blocks; + ULong st_atime; + ULong st_atime_nsec; + ULong st_mtime; + ULong st_mtime_nsec; + ULong st_ctime; + ULong st_ctime_nsec; +}; + extern SysRes VG_(open) ( const Char* pathname, Int flags, Int mode ); extern void VG_(close) ( Int fd ); extern Int VG_(read) ( Int fd, void* buf, Int count); @@ -44,8 +70,8 @@ extern Int VG_(write) ( Int fd, const void* buf, Int count); extern Int VG_(pipe) ( Int fd[2] ); extern OffT VG_(lseek) ( Int fd, OffT offset, Int whence ); -extern SysRes VG_(stat) ( Char* file_name, struct vki_stat* buf ); -extern Int VG_(fstat) ( Int fd, struct vki_stat* buf ); +extern SysRes VG_(stat) ( Char* file_name, struct vg_stat* buf ); +extern Int VG_(fstat) ( Int fd, struct vg_stat* buf ); extern SysRes VG_(dup) ( Int oldfd ); extern Int VG_(rename) ( Char* old_name, Char* new_name ); extern Int VG_(unlink) ( Char* file_name );