Re: [Valgrind-developers] [valgrind] coregrind/m_debuginfo: don't try to examine zero sized mmapped files

[ISHIKAWA,chiaki <ish...@yk...>]

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