Menu
▾
▴
valgrind-developers
|
From: Mark W. <ma...@so...> - 2025-03-09 15:05:25
|
https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=86ac4f2b004f57fa11224efafc1cd1c8fa8ded84 commit 86ac4f2b004f57fa11224efafc1cd1c8fa8ded84 Author: Mark Wielaard <ma...@kl...> Date: Sun Mar 9 15:59:29 2025 +0100 coregrind/m_debuginfo: don't try to examine zero sized mmapped files When run on an nfs filesystem memcheck/tests/pointer-trace fails because it generates warnings "connection to image failed". This is caused by trying to mmap a deleted file which the nfs file system represents as a (hidden) regular file. This is normally not a problem except when that file is empty. Fix this by not trying to check whether a file is an ELF or MACHO against an empty (regular) file in di_notify_mmap. An empty file is never a valid ELF or MACHO file (and cannot be represented as DiImage). https://bugs.kde.org/show_bug.cgi?id=501119 Diff: --- NEWS | 1 + coregrind/m_debuginfo/debuginfo.c | 10 ++++++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index 0fcbc5d0e2..16cfeef2a4 100644 --- a/NEWS +++ b/NEWS @@ -56,6 +56,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. 498492 none/tests/amd64/lzcnt64 crashes on FreeBSD compiled with clang 499183 FreeBSD: differences in avx-vmovq output 499212 mmap() with MAP_ALIGNED() returns unaligned pointer +501119 memcheck/tests/pointer-trace fails when run on NFS filesystem 501194 Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any number of segment commands diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c index 612833a997..15686fc5c4 100644 --- a/coregrind/m_debuginfo/debuginfo.c +++ b/coregrind/m_debuginfo/debuginfo.c @@ -1206,8 +1206,14 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd ) } /* 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.mode)) + don't try to read debug info from it. Also if it is a "regular file" + but has a zero size then skip it. Having a zero size will definitely + fail when trying to create an DiImage and wouldn't be a valid elf or + macho file. This can happen when mmapping a deleted file, which + would normally fail in the check above, because the stat call will + fail. But if the deleted file is on an NFS file system then a fake + (regular) empty file might be returned. */ + if (! VKI_S_ISREG(statbuf.mode) || statbuf.size == 0) return 0; /* no uses of statbuf below here. */ |
|
From: ISHIKAWA,chiaki <ish...@yk...> - 2025-03-09 18:49:56
|
Hi, Does this help reduce the occurences of presumably non-critical error messages like the following? I get this when I try to run mozilla's thunderbird mail client under valgrind. They appear many times. 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when reading debug info 23:44.69 GECKO(2640779) --2640783-- When reading debug info from /memfd:mozilla-ipc (deleted): 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when reading debug info 23:44.69 GECKO(2640779) --2640783-- When reading debug info from /memfd:mozilla-ipc (deleted): 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file I have no idea why valgrind wants try to to read debug info from memory region. TIA Chiaki On 2025/03/10 0:05, Mark Wielaard wrote: > https://sourceware.org/git/gitweb.cgi?p=valgrind.git;h=86ac4f2b004f57fa11224efafc1cd1c8fa8ded84 > > commit 86ac4f2b004f57fa11224efafc1cd1c8fa8ded84 > Author: Mark Wielaard <ma...@kl...> > Date: Sun Mar 9 15:59:29 2025 +0100 > > coregrind/m_debuginfo: don't try to examine zero sized mmapped files > > When run on an nfs filesystem memcheck/tests/pointer-trace fails > because it generates warnings "connection to image failed". This is > caused by trying to mmap a deleted file which the nfs file system > represents as a (hidden) regular file. This is normally not a problem > except when that file is empty. > > Fix this by not trying to check whether a file is an ELF or MACHO > against an empty (regular) file in di_notify_mmap. An empty file is > never a valid ELF or MACHO file (and cannot be represented as > DiImage). > > https://bugs.kde.org/show_bug.cgi?id=501119 > > Diff: > --- > NEWS | 1 + > coregrind/m_debuginfo/debuginfo.c | 10 ++++++++-- > 2 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/NEWS b/NEWS > index 0fcbc5d0e2..16cfeef2a4 100644 > --- a/NEWS > +++ b/NEWS > @@ -56,6 +56,7 @@ are not entered into bugzilla tend to get forgotten about or ignored. > 498492 none/tests/amd64/lzcnt64 crashes on FreeBSD compiled with clang > 499183 FreeBSD: differences in avx-vmovq output > 499212 mmap() with MAP_ALIGNED() returns unaligned pointer > +501119 memcheck/tests/pointer-trace fails when run on NFS filesystem > 501194 Fix ML_(check_macho_and_get_rw_loads) so that it is correct for any number of segment commands > > > diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c > index 612833a997..15686fc5c4 100644 > --- a/coregrind/m_debuginfo/debuginfo.c > +++ b/coregrind/m_debuginfo/debuginfo.c > @@ -1206,8 +1206,14 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd ) > } > > /* 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.mode)) > + don't try to read debug info from it. Also if it is a "regular file" > + but has a zero size then skip it. Having a zero size will definitely > + fail when trying to create an DiImage and wouldn't be a valid elf or > + macho file. This can happen when mmapping a deleted file, which > + would normally fail in the check above, because the stat call will > + fail. But if the deleted file is on an NFS file system then a fake > + (regular) empty file might be returned. */ > + if (! VKI_S_ISREG(statbuf.mode) || statbuf.size == 0) > return 0; > > /* no uses of statbuf below here. */ > > > _______________________________________________ > Valgrind-developers mailing list > Val...@li... > https://lists.sourceforge.net/lists/listinfo/valgrind-developers |
|
From: Mark W. <ma...@kl...> - 2025-03-09 20:46:11
|
Hi Chiaki,
On Mon, Mar 10, 2025 at 03:32:16AM +0900, ISHIKAWA,chiaki wrote:
> Does this help reduce the occurences of presumably non-critical
> error messages like the following?
No. These are indeed related messages, but caused by using memfd
allocated memory.
> I get this when I try to run mozilla's thunderbird mail client under
> valgrind.
> They appear many times.
>
> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
> reading debug info
> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
> /memfd:mozilla-ipc (deleted):
> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
> reading debug info
> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
> /memfd:mozilla-ipc (deleted):
> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
>
> I have no idea why valgrind wants try to to read debug info from
> memory region.
I presume because valgrind thinks it might be a real memory mapped
file. The code triggering this warning is:
/* Don't let the stat call fail silently. Filter out some known
sources of noise before complaining, though. */
if (sr_isError(statres)) {
DebugInfo fake_di;
Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
|| VG_(strstr)(filename, "/dev/shm/") != NULL;
if (!quiet && VG_(clo_verbosity) > 1) {
VG_(memset)(&fake_di, 0, sizeof(fake_di));
fake_di.fsm.filename = ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
ML_(symerr)(&fake_di, True, "failed to stat64/stat this file");
}
return 0;
}
So we probably should add "/memfd:" to the patterns for which not to
warn.
https://www.man7.org/linux/man-pages/man2/memfd_create.2.html says:
int memfd_create(const char *name, unsigned int flags);
The name supplied in name is used as a filename and will be
displayed as the target of the corresponding symbolic link in the
directory /proc/self/fd/. The displayed name is always prefixed
with memfd: and serves only for debugging purposes.
So I would propose something like the following (untested) patch:
diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
index 15686fc5c46f..6c4a6926560b 100644
--- a/coregrind/m_debuginfo/debuginfo.c
+++ b/coregrind/m_debuginfo/debuginfo.c
@@ -1196,7 +1196,9 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
if (sr_isError(statres)) {
DebugInfo fake_di;
Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
- || VG_(strstr)(filename, "/dev/shm/") != NULL;
+ || VG_(strstr)(filename, "/dev/shm/") != NULL
+ || VG_(strncmp)("/memfd:", filename,
+ VG_(strlen)("/memfd:")) == 0;
if (!quiet && VG_(clo_verbosity) > 1) {
VG_(memset)(&fake_di, 0, sizeof(fake_di));
fake_di.fsm.filename = ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
Does that work for you?
Thanks,
Mark
|
|
From: ISHIKAWA,chiaki <ish...@yk...> - 2025-03-10 05:44:06
|
Dear Mark,
Thank you for your analysis.
I will try to create a localized version of valgrind after applying the
patch and see how it goes.
It might take a couple of days due to my day job, etc.
Chiaki
On 2025/03/10 5:45, Mark Wielaard wrote:
> Hi Chiaki,
>
> On Mon, Mar 10, 2025 at 03:32:16AM +0900, ISHIKAWA,chiaki wrote:
>> Does this help reduce the occurences of presumably non-critical
>> error messages like the following?
> No. These are indeed related messages, but caused by using memfd
> allocated memory.
>
>> I get this when I try to run mozilla's thunderbird mail client under
>> valgrind.
>> They appear many times.
>>
>> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
>> reading debug info
>> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
>> /memfd:mozilla-ipc (deleted):
>> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
>> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
>> reading debug info
>> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
>> /memfd:mozilla-ipc (deleted):
>> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
>>
>> I have no idea why valgrind wants try to to read debug info from
>> memory region.
> I presume because valgrind thinks it might be a real memory mapped
> file. The code triggering this warning is:
>
> /* Don't let the stat call fail silently. Filter out some known
> sources of noise before complaining, though. */
> if (sr_isError(statres)) {
> DebugInfo fake_di;
> Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
> || VG_(strstr)(filename, "/dev/shm/") != NULL;
> if (!quiet && VG_(clo_verbosity) > 1) {
> VG_(memset)(&fake_di, 0, sizeof(fake_di));
> fake_di.fsm.filename = ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
> ML_(symerr)(&fake_di, True, "failed to stat64/stat this file");
> }
> return 0;
> }
>
> So we probably should add "/memfd:" to the patterns for which not to
> warn.
>
> https://www.man7.org/linux/man-pages/man2/memfd_create.2.html says:
>
> int memfd_create(const char *name, unsigned int flags);
>
> The name supplied in name is used as a filename and will be
> displayed as the target of the corresponding symbolic link in the
> directory /proc/self/fd/. The displayed name is always prefixed
> with memfd: and serves only for debugging purposes.
>
> So I would propose something like the following (untested) patch:
>
> diff --git a/coregrind/m_debuginfo/debuginfo.c b/coregrind/m_debuginfo/debuginfo.c
> index 15686fc5c46f..6c4a6926560b 100644
> --- a/coregrind/m_debuginfo/debuginfo.c
> +++ b/coregrind/m_debuginfo/debuginfo.c
> @@ -1196,7 +1196,9 @@ ULong VG_(di_notify_mmap)( Addr a, Bool allow_SkFileV, Int use_fd )
> if (sr_isError(statres)) {
> DebugInfo fake_di;
> Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
> - || VG_(strstr)(filename, "/dev/shm/") != NULL;
> + || VG_(strstr)(filename, "/dev/shm/") != NULL
> + || VG_(strncmp)("/memfd:", filename,
> + VG_(strlen)("/memfd:")) == 0;
> if (!quiet && VG_(clo_verbosity) > 1) {
> VG_(memset)(&fake_di, 0, sizeof(fake_di));
> fake_di.fsm.filename = ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
>
> Does that work for you?
>
> Thanks,
>
> Mark
|
|
From: ISHIKAWA,chiaki <ish...@yk...> - 2025-03-11 19:39:51
|
Dear Mark,
It works.
I don't get the noisy warnings any more.
Thank you for the quick fix!
Chiaki
On 2025/03/10 14:43, ISHIKAWA,chiaki wrote:
> Dear Mark,
>
> Thank you for your analysis.
>
> I will try to create a localized version of valgrind after applying
> the patch and see how it goes.
>
> It might take a couple of days due to my day job, etc.
>
> Chiaki
>
> On 2025/03/10 5:45, Mark Wielaard wrote:
>> Hi Chiaki,
>>
>> On Mon, Mar 10, 2025 at 03:32:16AM +0900, ISHIKAWA,chiaki wrote:
>>> Does this help reduce the occurences of presumably non-critical
>>> error messages like the following?
>> No. These are indeed related messages, but caused by using memfd
>> allocated memory.
>>
>>> I get this when I try to run mozilla's thunderbird mail client under
>>> valgrind.
>>> They appear many times.
>>>
>>> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
>>> reading debug info
>>> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
>>> /memfd:mozilla-ipc (deleted):
>>> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
>>> 23:44.69 GECKO(2640779) --2640783-- WARNING: Serious error when
>>> reading debug info
>>> 23:44.69 GECKO(2640779) --2640783-- When reading debug info from
>>> /memfd:mozilla-ipc (deleted):
>>> 23:44.69 GECKO(2640779) --2640783-- failed to stat64/stat this file
>>>
>>> I have no idea why valgrind wants try to to read debug info from
>>> memory region.
>> I presume because valgrind thinks it might be a real memory mapped
>> file. The code triggering this warning is:
>>
>> /* Don't let the stat call fail silently. Filter out some known
>> sources of noise before complaining, though. */
>> if (sr_isError(statres)) {
>> DebugInfo fake_di;
>> Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
>> || VG_(strstr)(filename, "/dev/shm/") != NULL;
>> if (!quiet && VG_(clo_verbosity) > 1) {
>> VG_(memset)(&fake_di, 0, sizeof(fake_di));
>> fake_di.fsm.filename =
>> ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
>> ML_(symerr)(&fake_di, True, "failed to stat64/stat this
>> file");
>> }
>> return 0;
>> }
>>
>> So we probably should add "/memfd:" to the patterns for which not to
>> warn.
>>
>> https://www.man7.org/linux/man-pages/man2/memfd_create.2.html says:
>>
>> int memfd_create(const char *name, unsigned int flags);
>>
>> The name supplied in name is used as a filename and will be
>> displayed as the target of the corresponding symbolic link in the
>> directory /proc/self/fd/. The displayed name is always prefixed
>> with memfd: and serves only for debugging purposes.
>>
>> So I would propose something like the following (untested) patch:
>>
>> diff --git a/coregrind/m_debuginfo/debuginfo.c
>> b/coregrind/m_debuginfo/debuginfo.c
>> index 15686fc5c46f..6c4a6926560b 100644
>> --- a/coregrind/m_debuginfo/debuginfo.c
>> +++ b/coregrind/m_debuginfo/debuginfo.c
>> @@ -1196,7 +1196,9 @@ ULong VG_(di_notify_mmap)( Addr a, Bool
>> allow_SkFileV, Int use_fd )
>> if (sr_isError(statres)) {
>> DebugInfo fake_di;
>> Bool quiet = VG_(strstr)(filename, "/var/run/nscd/") != NULL
>> - || VG_(strstr)(filename, "/dev/shm/") != NULL;
>> + || VG_(strstr)(filename, "/dev/shm/") != NULL
>> + || VG_(strncmp)("/memfd:", filename,
>> + VG_(strlen)("/memfd:")) == 0;
>> if (!quiet && VG_(clo_verbosity) > 1) {
>> VG_(memset)(&fake_di, 0, sizeof(fake_di));
>> fake_di.fsm.filename =
>> ML_(dinfo_strdup)("di.debuginfo.nmm", filename);
>>
>> Does that work for you?
>>
>> Thanks,
>>
>> Mark
>
>
>
>
> _______________________________________________
> Valgrind-developers mailing list
> Val...@li...
> https://lists.sourceforge.net/lists/listinfo/valgrind-developers
|
|
From: Mark W. <ma...@kl...> - 2025-03-12 15:19:09
|
Hi Chiaki, On Wed, 2025-03-12 at 04:40 +0900, ISHIKAWA,chiaki wrote: > It works. > I don't get the noisy warnings any more. Thanks for testing. I pushed the attached commit. Cheers, Mark |
×
Want the latest updates on software, tech news, and AI?
Get latest updates about software, tech news, and AI from SourceForge directly in your inbox once a month.
Thanks for helping keep SourceForge clean.
X