Bug 439685 - compiler warning in callgrind/main.c
Summary: compiler warning in callgrind/main.c
Status: RESOLVED FIXED
Alias: None
Product: valgrind
Classification: Developer tools
Component: callgrind (show other bugs)
Version: unspecified
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Josef Weidendorfer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-07-09 15:39 UTC by Florian Krohm
Modified: 2023-04-19 22:53 UTC (History)
1 user (show)

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


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Florian Krohm 2021-07-09 15:39:23 UTC
Building valgrind from git @ 43543527a293e626e601202ca4eeb2216f40815d
on s390x with gcc-5.real (Ubuntu 5.4.0-6ubuntu1~16.04.12) 5.4.0 20160609
produces this warning for callgrind/main.c

main.c: In function ‘vgCallgrind_post_syscalltime’:
main.c:1779:25: warning: ‘*((void *)&ts_now+8)’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     struct vki_timespec ts_now;
main.c:1779:25: warning: ‘ts_now’ may be used uninitialized in this function [-Wmaybe-uninitialized]

In function collect_time the conditional expression in the switch statement
has type int (after integral promotions). GCC assumes that it may have
values other than the ones listed in the enumerated type it was promoted from.
In that case the memory pointed to by its 1st argument remains unintialised.
Later on vki_timespec_diff will read the contents of ts_now undoditionally.
Hence the warning.
The patch below removes the warning and makes the code more robust should
another enumerator ever be added to Collect_Systime.

diff --git a/callgrind/main.c b/callgrind/main.c
index 904eb42..bda10dc 100644
--- a/callgrind/main.c
+++ b/callgrind/main.c
@@ -1711,7 +1711,7 @@ static
 void collect_time (struct vki_timespec *systime, struct vki_timespec *syscputime)
 {
   switch (CLG_(clo).collect_systime) {
-    case systime_no: tl_assert (0);
+    default: tl_assert (0);
     case systime_msec: {
       UInt ms_timer = VG_(read_millisecond_timer)();
       systime->tv_sec = ms_timer / 1000;
Comment 1 Mark Wielaard 2023-04-19 22:53:04 UTC
Thanks for the analysis and the patch. Pushed as:

commit d270b7b15bafd7eb555994483556e3c22400bf47
Author: Mark Wielaard <mark@klomp.org>
Date:   Thu Apr 20 00:42:40 2023 +0200

    Bug 439685 compiler warning in callgrind/main.c
    
    main.c: In function 'vgCallgrind_post_syscalltime':
    main.c:1779:25: warning: '*((void *)&ts_now+8)'
        may be used uninitialized in this function [-Wmaybe-uninitialized]
         struct vki_timespec ts_now;
    main.c:1779:25: warning: 'ts_now'
        may be used uninitialized in this function [-Wmaybe-uninitialized]
    
    In function collect_time the conditional expression in the switch
    statement has type int (after integral promotions). GCC assumes that
    it may have values other than the ones listed in the enumerated type
    it was promoted from.  In that case the memory pointed to by its 1st
    argument remains unintialised.  Later on vki_timespec_diff will read
    the contents of ts_now undoditionally.  Hence the warning.
    
    Using the default case for the tl_assert () removes the warning and
    makes the code more robust should another enumerator ever be added to
    Collect_Systime.
    
    Contributed-by: Florian Krohm <florian@eich-krohm.de>