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
Status: CLOSED FIXED
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
URL:
Depends on:
Blocks:
 
Reported: 2014-11-10 21:14 UTC by Tom Callaway
Modified: 2014-12-07 06:30 UTC (History)
1 user (show)

See Also:


Attachments
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:

    MAKE_NOACCESS  --> MAKE_MEM_NOACCESS
    MAKE_WRITABLE  --> MAKE_MEM_UNDEFINED
    MAKE_READABLE  --> MAKE_MEM_DEFINED

    CHECK_WRITABLE --> CHECK_MEM_IS_ADDRESSABLE
    CHECK_READABLE --> CHECK_MEM_IS_DEFINED
    CHECK_DEFINED  --> CHECK_VALUE_IS_DEFINED

  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:

    MAKE_MEM_DEFINED_IF_ADDRESSABLE(a, len)

  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:
    VALGRIND_MAKE_NOACCESS
    VALGRIND_MAKE_WRITABLE
    VALGRIND_MAKE_READABLE
    VALGRIND_CHECK_WRITABLE
    VALGRIND_CHECK_READABLE
    VALGRIND_CHECK_DEFINED
  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.