Bug 455168 - want/need more 'const' in Valgrind source code
Summary: want/need more 'const' in Valgrind source code
Status: REPORTED
Alias: None
Product: valgrind
Classification: Developer tools
Component: memcheck (show other bugs)
Version: 3.20 GIT
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Julian Seward
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2022-06-11 22:58 UTC by John Reiser
Modified: 2022-06-15 05:10 UTC (History)
1 user (show)

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


Attachments
Patch to add 'const' to many "HChar *" declarations. (60.55 KB, patch)
2022-06-11 22:58 UTC, John Reiser
Details

Note You need to log in before you can comment on or make changes to this bug.
Description John Reiser 2022-06-11 22:58:48 UTC
Created attachment 149626 [details]
Patch to add 'const' to many "HChar *" declarations.

SUMMARY
***
I found 120 regions in 32 Valgrind source code files
where 'const' was omitted:
   git diff master --stat | wc -l;  git diff master | grep -c '@@'
and that was just by hunting near instances of "HChar *"
which often should be "HChar const *".
See the attached patch.

The problems are exacerbated (or perhaps caused) by oversights
(or errors) in the Standard for C language stdlib.  For instance:

       int execve(const char *pathname, char *const argv[],
                  char *const envp[]);

should be

       int execve(const char *pathname, char const *const argv[],
                  char const *const envp[]);

Note the added 'const' in the declaration of the second and third
parameters.  When 'execve' fails, then control returns to the program
(which often is a shell!) and the program is going to assume
that the operating system has not scribbled on the characters
in the argument or environment strings.  But only the revised
declaration states that property explicitly.

A similar bug in the Standard is:

       long strtol(const char *nptr, char **endptr, int base);

which should be

       long strtol(const char *nptr, char const **endptr, int base);

If endptr is not NULL, then upon return from strtol
*endptr designates a substring of nptr.  The characters of nptr
are const (both the application programmer and strtol are not allowed
to write them), so therefore the characters of *endptr also must be 'const'.

Confimation of this problem can be seen in the source code to strtol:
===== glibc-2.29-32-g6d8eaf4a25/stdlib/strtol_l.c
INT
INTERNAL (__strtol_l) (const STRING_TYPE *nptr, STRING_TYPE **endptr,
               int base, int group, locale_t loc)
-----
  /* Store in ENDPTR the address of one character
     past the last character we converted.  */
  if (endptr != NULL)
    *endptr = (STRING_TYPE *) s;
-----
  const STRING_TYPE *save, *end;
-----
    *endptr = (STRING_TYPE *) &save[-1];
-----
    /*  There was no number to convert.  */
    *endptr = (STRING_TYPE *) nptr;
=====
where "gcc -Wcast-qual" will complain about 'const' being elided.
***


STEPS TO REPRODUCE
1.  Compile both  3aee8a29447ea14108eb5d4ab3c1f7677767296a and the patch using "gcc -Wcast-qual" and compare.
2. 
3. 

OBSERVED RESULT
No differences except -Wcast-qual complaints due to external requirements (Standard declarations.)
Therefore the added 'const' should have been used.

EXPECTED RESULT


SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma:  Fedora 34
(available in About System)
KDE Plasma Version: 
KDE Frameworks Version: 
Qt Version: 

ADDITIONAL INFORMATION
gcc (GCC) 11.3.1 20220421 (Red Hat 11.3.1-2)
Comment 1 Paul Floyd 2022-06-13 19:07:50 UTC
I get quite a few new warnings (there were perhaps 5 before, mostly void abuse and for x86)


m_main.c:3439:34: warning: passing argument 2 of 'valgrind_main' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_main.c:3439:40: warning: passing argument 3 of 'valgrind_main' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_main.c:3439:34: warning: passing argument 2 of 'valgrind_main' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_main.c:3439:40: warning: passing argument 3 of 'valgrind_main' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_libcbase.c:94:46: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:123:47: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:151:46: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:188:48: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:225:43: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_pathscan.c:131:26: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
m_initimg/initimg-freebsd.c:286:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_initimg/initimg-freebsd.c:557:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:559:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:562:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:566:19: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:573:21: warning: assignment to 'HChar **' {aka 'char **'} from incompatible pointer type 'const HChar **' {aka 'const char **'} [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:575:29: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_sigframe/sigframe-amd64-freebsd.c:256:27: warning: taking address of expression of type 'void'
m_syswrap/syswrap-generic.c:3084:38: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
m_libcbase.c:94:46: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:123:47: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:151:46: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:188:48: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_libcbase.c:225:43: warning: to be safe all intermediate pointers in cast from 'HChar **' {aka 'char **'} to 'const HChar **' {aka 'const char **'} must be 'const' qualified [-Wcast-qual]
m_pathscan.c:131:26: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
m_initimg/initimg-freebsd.c:286:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
m_initimg/initimg-freebsd.c:557:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:559:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:562:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:566:19: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:573:21: warning: assignment to 'HChar **' {aka 'char **'} from incompatible pointer type 'const HChar **' {aka 'const char **'} [-Wincompatible-pointer-types]
m_initimg/initimg-freebsd.c:575:29: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
m_sigframe/sigframe-x86-freebsd.c:292:27: warning: taking address of expression of type 'void'
m_syswrap/syswrap-generic.c:3084:38: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
m_syswrap/syswrap-x86-freebsd.c:401:24: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
m_syswrap/syswrap-x86-freebsd.c:406:24: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
m_syswrap/syswrap-x86-freebsd.c:419:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
m_syswrap/syswrap-x86-freebsd.c:460:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
Comment 2 John Reiser 2022-06-13 19:45:28 UTC
(In reply to Paul Floyd from comment #1)
> I get quite a few new warnings (there were perhaps 5 before, mostly void
> abuse and for x86)

Please specify the compiler name and version, and the command-line parameters.
Comment 3 Paul Floyd 2022-06-13 19:53:50 UTC
For this, just 'git clone' and 'configure' which would have picked up

paulf> gcc --version
gcc (FreeBSD Ports Collection) 10.3.0

More often I use
paulf> clang --version 
FreeBSD clang version 11.0.1 (git@github.com:llvm/llvm-project.git llvmorg-11.0.1-0-g43ff75f2c3fe)

With clang I get

m_main.c:3439:34: warning: passing 'const HChar *const *' (aka 'const char *const *') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
   r = valgrind_main( (Int)argc, argv, envp );m_main.c:3439:34: warning: passing 'const HChar *const *' (aka 'const char *const *') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
m_main.c:3439:40: warning: passing 'const HChar *const *' (aka 'const char *const *') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
m_main.c:3439:40: warning: passing 'const HChar *const *' (aka 'const char *const *') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
m_libcbase.c:94:61: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:123:62: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:151:61: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:188:63: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:225:58: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_pathscan.c:131:33: warning: cast from 'const char *' to 'void *' drops const qualifier [-Wcast-qual]
m_libcbase.c:94:61: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:123:62: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:151:61: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:188:63: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_libcbase.c:225:58: warning: cast from 'HChar **' (aka 'char **') to 'const HChar **' (aka 'const char **') must have all intermediate pointers const qualified to be safe [-Wcast-qual]
m_pathscan.c:131:33: warning: cast from 'const char *' to 'void *' drops const qualifier [-Wcast-qual]
m_initimg/initimg-freebsd.c:286:11: warning: initializing 'HChar *' (aka 'char *') with an expression of type 'const HChar *' (aka 'const char *') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:557:31: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:559:31: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:562:31: warning: clang -DHAVE_CONFIG_H -I. -I..  -I.. -I../include -I../include -I../VEX/pub -I../VEX/pub -DVGA_x86=1 -DVGO_freebsd=1 -DVGP_x86_freebsd=1 -DVGPV_x86_freebsd_vanilla=1  -I../coregrind -DVG_LIBDIR="\"/usr/local/libexec/valgrind"\" -DVG_PLATFORM="\"x86-freebsd\""   -B/usr/lib32 -m32 -O2 -g -Wall -Wmissing-prototypes -Wshadow -Wpointer-arith -Wstrict-prototypes -Wmissing-declarations -Wcast-align -Wcast-qual -Wwrite-strings -Wempty-body -Wformat -Wformat-security -Wignored-qualifiers -Wenum-conversion -finline-functions -fno-stack-protector -fno-strict-aliasing -fno-builtin -Wno-cast-align -Wno-self-assign -Wno-tautological-compare -Wno-expansion-to-defined -fomit-frame-pointer   -MT m_replacemalloc/libcoregrind_x86_freebsd_a-replacemalloc_core.o -MD -MP -MF m_replacemalloc/.deps/libcoregrind_x86_freebsd_a-replacemalloc_core.Tpo -c -o m_replacemalloc/libcoregrind_x86_freebsd_a-replacemalloc_core.o `test -f 'm_replacemalloc/replacemalloc_core.c' || echo './'`m_replacemalloc/replacemalloc_core.c
m_initimg/initimg-freebsd.c:566:19: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:573:21: warning: assigning to 'HChar **' (aka 'char **') from 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:575:29: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_syswrap/syswrap-generic.c:3084:46: warning: cast from 'const char *' to 'void *' drops const qualifier [-Wcast-qual]
m_initimg/initimg-freebsd.c:286:11: warning: initializing 'HChar *' (aka 'char *') with an expression of type 'const HChar *' (aka 'const char *') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:557:31: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:559:31: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:562:31: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:566:19: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:573:21: warning: assigning to 'HChar **' (aka 'char **') from 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_initimg/initimg-freebsd.c:575:29: warning: passing 'HChar **' (aka 'char **') to parameter of type 'const HChar **' (aka 'const char **') discards qualifiers in nested pointer types [-Wincompatible-pointer-types-discards-qualifiers]
m_syswrap/syswrap-generic.c:3084:46: warning: cast from 'const char *' to 'void *' drops const qualifier [-Wcast-qual]
Comment 4 John Reiser 2022-06-14 22:08:50 UTC
(In reply to Paul Floyd from comment #1)
> I get quite a few new warnings (there were perhaps 5 before, mostly void
> abuse and for x86)

When I process those warning lines using the shell pipeline
   sort  |  uniq -c  | sort -rn
then I see that most of the lines appear twice as duplicates:
-----
      2 m_syswrap/syswrap-generic.c:3084:38: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
      2 m_pathscan.c:131:26: warning: cast discards 'const' qualifier from pointer target type [-Wcast-qual]
      2 m_main.c:3439:40: warning: passing argument 3 of 'valgrind_main' discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
   [[snip]]
      2 m_initimg/initimg-freebsd.c:559:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
      2 m_initimg/initimg-freebsd.c:557:31: warning: passing argument 1 of 'copy_str' from incompatible pointer type [-Wincompatible-pointer-types]
      2 m_initimg/initimg-freebsd.c:286:16: warning: initialization discards 'const' qualifier from pointer target type [-Wdiscarded-qualifiers]
      1 m_syswrap/syswrap-x86-freebsd.c:460:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      1 m_syswrap/syswrap-x86-freebsd.c:419:10: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      1 m_syswrap/syswrap-x86-freebsd.c:406:24: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      1 m_syswrap/syswrap-x86-freebsd.c:401:24: warning: cast to pointer from integer of different size [-Wint-to-pointer-cast]
      1 m_sigframe/sigframe-x86-freebsd.c:292:27: warning: taking address of expression of type 'void'
      1 m_sigframe/sigframe-amd64-freebsd.c:256:27: warning: taking address of expression of type 'void'
-----
So "quite a few" is actually not so may.  (And the Makefile could be made less wasteful.)
[Further analysis later ...]
Comment 5 Paul Floyd 2022-06-15 05:10:50 UTC
On amd64 most files get compiled twice, once for x86 and once for amd64.