Bug 485778 - Crash with --track-fds=all and --gen-suppressions=all
Summary: Crash with --track-fds=all and --gen-suppressions=all
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: general (show other bugs)
Version: 3.23 GIT
Platform: Compiled Sources Linux
: NOR crash
Target Milestone: ---
Assignee: Mark Wielaard
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2024-04-19 07:23 UTC by Paul Floyd
Modified: 2024-04-20 01:22 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments
core errors gensuppressions support (4.08 KB, patch)
2024-04-19 15:31 UTC, Mark Wielaard
Details
core errors suppression support (9.51 KB, text/plain)
2024-04-19 18:54 UTC, Mark Wielaard
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Paul Floyd 2024-04-19 07:23:38 UTC
I just noticed this warning

In file included from pub_core_basics.h:38,
                 from m_errormgr.c:29:
m_errormgr.c: In function 'gen_suppression':
../include/pub_tool_basics.h:68:30: warning: 'name' may be used uninitialized in this function [-Wmaybe-uninitialized]
   68 | #define VG_(str)    VGAPPEND(vgPlain_,          str)
      |                              ^~~~~~~~
m_errormgr.c:367:17: note: 'name' was declared here
  367 |    const HChar* name;
      |  

And I ran a few tests.

In the "none/tests" directory

../../vg-in-place --track-fds=all --tool=none --gen-suppressions=all ./fdleak_creat

--32134-- VALGRIND INTERNAL ERROR: Valgrind received a signal 11 (SIGSEGV) - exiting
--32134-- si_code=1;  Faulting address: 0x6;  sp: 0x100288dc00

valgrind: the 'impossible' happened:
   Killed by fatal signal

host stacktrace:
==32134==    at 0x5801EB51: myvprintf_str (m_debuglog.c:742)
==32134==    by 0x5801F4D6: vgPlain_debugLog_vprintf (m_debuglog.c:1146)
==32134==    by 0x58008207: vgPlain_xaprintf (m_xarray.c:383)
==32134==    by 0x58020DE9: gen_suppression (m_errormgr.c:397)
==32134==    by 0x580226EA: pp_Error (m_errormgr.c:546)
==32134==    by 0x58022F71: vgPlain_unique_error (m_errormgr.c:919)
==32134==    by 0x5806FAA3: vgPlain_show_open_fds (syswrap-generic.c:980)
==32134==    by 0x5801633B: shutdown_actions_NORETURN (m_main.c:2317)
==32134==    by 0x5807C06A: run_a_thread_NORETURN (syswrap-linux.c:202)

Are we planning on adding suppressions to core errors? If so this message will also need to change:
==32134== Bad option: --gen-suppressions=yes
==32134== Can't use --gen-suppressions= with Nulgrind
==32134== because it doesn't generate errors.
==32134== Use --help for more information or consult the user manual.

(it says it can't use it and then it does use it!)
Comment 1 Mark Wielaard 2024-04-19 11:36:22 UTC
(In reply to Paul Floyd from comment #0)
> Are we planning on adding suppressions to core errors? If so this message
> will also need to change:
> ==32134== Bad option: --gen-suppressions=yes
> ==32134== Can't use --gen-suppressions= with Nulgrind
> ==32134== because it doesn't generate errors.
> ==32134== Use --help for more information or consult the user manual.
> 
> (it says it can't use it and then it does use it!)

There are a couple of extra callbacks needed to support suppressions for core errors.
But they shouldn't be hard to add. There also needs to be a special check for core errors
that have a NULL where (e.g. when reporting with --track-fds=all on inherited file descriptors).

Working on it now.
Comment 2 Mark Wielaard 2024-04-19 15:31:57 UTC
Created attachment 168679 [details]
core errors gensuppressions support

Initial implementation for --gen-suppressions support for core errors

With this the testcase ../../vg-in-place --track-fds=all --tool=none --gen-suppressions=all ./fdleak_creat produces:

==381147== Bad option: --gen-suppressions=yes
==381147== Can't use --gen-suppressions= with Nulgrind
==381147== because it doesn't generate errors.
==381147== Use --help for more information or consult the user manual.
==381147== Nulgrind, the minimal Valgrind tool
==381147== Copyright (C) 2002-2017, and GNU GPL'd, by Nicholas Nethercote.
==381147== Using Valgrind-3.23.0.GIT and LibVEX; rerun with -h for copyright info
==381147== Command: ./fdleak_creat
==381147== 
==381147== 
==381147== FILE DESCRIPTORS: 4 open (3 std) at exit.
==381147== Open file descriptor 3: /tmp/file.381147
==381147==    at 0x495B9C4: creat (creat64.c:28)
==381147==    by 0x401249: main (fdleak_creat.c:13)
==381147== 
{
   <insert_a_suppression_name_here>
   CoreError:FdNotClosed
   fun:creat
   fun:main
}
==381147== Open file descriptor 2: /dev/pts/4
==381147==    <inherited from parent>
==381147== 
==381147== (No origin, error cannot be suppressed)
==381147== Open file descriptor 1: /dev/pts/4
==381147==    <inherited from parent>
==381147== 
==381147== (No origin, error cannot be suppressed)
==381147== Open file descriptor 0: /dev/pts/4
==381147==    <inherited from parent>
==381147== 
==381147== (No origin, error cannot be suppressed)
==381147==
Comment 3 Mark Wielaard 2024-04-19 18:54:19 UTC
Created attachment 168690 [details]
core errors suppression support

This version handles both generating and using suppressions. So now you can also add something like:

{
   core-bad-close
   CoreError:FdBadClose
   fun:close
   fun:closefile
   fun:main
}

And it will suppress the error as expected.

It also adds VG_(needs_core_errors) to all tools that support tool_errors and to the none tool.

No testcases yet though
Comment 4 Mark Wielaard 2024-04-20 01:22:23 UTC
commit d68d584cead390c339b9575c5c9679e8ce50c37f
Author: Mark Wielaard <mark@klomp.org>
Date:   Fri Apr 19 22:46:11 2024 +0200

    core errors suppression support
    
    Add two new functions core_get_extra_suppression_info and
    core_get_error_name as alternatives for tool suppression callbacks.
    These functions are used in gen_suppression.
    
    Instead of a tool name, a core error component name is "CoreError".
    
    Two new suppression kinds FdBadCloseSupp and FdNotClosedSupp
    were added. Corresponding to the FdBadClose and FdNotClosed
    error kinds.
    
    core_error_matches_suppression matches these suppression kinds
    with error kinds.
    
    core_get_extra_suppression_info and core_print_extra_suppression_use
    are noops for core errors.
    
    is_suppressible_error, supp_matches_error, load_one_suppressions_file
    and show_used_suppressions have been adjusted to work with core
    error kinds.
    
    A new function VG_(found_or_suppressed_errs) helps to not output
    an empty error summary if only core errors are requested, but no
    errors were detected.
    
    VG_(clo_track_fds) has been moved from pub_core_options.h to
    pub_tool_options.h. And VG_(needs_core_errors) now takes a Bool
    that can be set to false in the tool post_clo_init handler. This
    is used in the none tool to request core errors, but disable all
    reporting if no option has been given that enables such errors.
    
    This make sure only the none/tests/fdleak_ipv4.stderr.exp needs
    adjustment. For all other tests the output is exactly as before.
    
    https://bugs.kde.org/show_bug.cgi?id=485778