Bug 16068 - Valgrind in R - detect system headers and use them when available
Summary: Valgrind in R - detect system headers and use them when available
Alias: None
Product: R
Classification: Unclassified
Component: Low-level (show other bugs)
Version: R 3.1.2
Hardware: Other Linux
: P5 enhancement
Assignee: R-core
Depends on:
Reported: 2014-11-10 21:14 UTC by Tom Callaway
Modified: 2014-12-07 06:30 UTC (History)
1 user (show)

See Also:

Check for valgrind/memcheck.h on the system before using bundled copy. (1.14 KB, patch)
2014-11-10 21:14 UTC, Tom Callaway
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tom Callaway 2014-11-10 21:14:07 UTC
Created attachment 1686 [details]
Check for valgrind/memcheck.h on the system before using bundled copy.

Recently, in adding support for new architectures in Fedora, several issues were discovered with Valgrind, and addressed in 3.10.0. A bug was fixed with ppc32 and support was added for aarch64 and ppc64le. These changes require an updated valgrind.h and memcheck.h.

R is currently bundling a version of valgrind.h and memcheck.h that do not incorporate these fixes. Please update the bundled copies of these files to come from at least 3.10.0.

In addition, it would be useful for compilations of R with the --with-valgrind-instrumentation option set above 0 to first search for the system valgrind headers, and if found, use them in the compile instead of the bundled headers, to attempt to "future-proof" this scenario. I have attached a patch which does this.
Comment 1 Brian Ripley 2014-11-11 11:34:21 UTC
Unfortunately it is not as simple as that: the names of the valgrind macros used in memory.c have been changed in valgrind 3.10.0, so allowing system headers would not 'future-proof' against further name changes.

We will investigate updating the included headers, but that needs considerable checking of the changes needed in memory.c.
Comment 2 Brian Ripley 2014-11-11 16:16:56 UTC
It is unclear when the valgrind macros changed: it seems to be <= 3.8.0 but is not in their release notes.

The current status is that system headers can be used in R-devel via --with-system-valgrind-headers, but for valgrind >= 3.8.0, only instrumentation levels 1 and 2 are usable.  It may take a long time to find out what they changed which breaks level 3.
Comment 3 Mark Wielaard 2014-11-12 10:48:52 UTC
It looks like the private copy of valgind.h/memcheck.h in the R source tree is more than 8 years old. Note the following from NEWS.old:

Release 3.2.0 (7 June 2006)

- There are some changes to Memcheck's client requests.  Some of them
  have changed names:



  The reason for the change is that the old names are subtly
  misleading.  The old names will still work, but they are deprecated
  and may be removed in a future release.

  We also added a new client request:


  which is like MAKE_MEM_DEFINED but only affects a byte if the byte is
  already addressable.

- The way client requests are encoded in the instruction stream has
  changed.  Unfortunately, this means 3.2.0 will not honour client
  requests compiled into binaries using headers from earlier versions
  of Valgrind.  We will try to keep the client request encodings more
  stable in future.

Release 3.3.0 (7 December 2007)

- The following Memcheck client requests have been removed:
  They were deprecated in 3.2.0, when equivalent but better-named client
  requests were added.  See the 3.2.0 release notes for more details.

As far as I know there were no user visible changes to the macros since then (except for adding support for new client requests, new architectures and bug fixes like the ppc32 one).
Comment 4 Brian Ripley 2014-12-07 06:30:56 UTC
Headers updated to those from valgrind 3.10.1; 
support for instrumentation level 3 withdrawn.