Bug 204843

Fixed in r10860.

... together with r10863.

Can you explain a bit more about why it worked before and how it
got broken?  I see a bunch of #if HAVE_PROC that wasn't there before.
I'd prefer not to have #ifdef'd code unless we really need it.

In other words .. if it worked OK on 3.4.1 without the #ifdef'd code,
why do we need #ifdefs now?

(In reply to comment #4)
> In other words .. if it worked OK on 3.4.1 without the #ifdef'd code,
> why do we need #ifdefs now?

The addition of AC_CHECK_FILES() in r10156 broke cross-compilation. What I did is to remove the AC_CHECK_FILES() configure test and to replace #ifdef HAVE_PROC by VG_(have_proc_filesystem)().

(In reply to comment #5)
> The addition of AC_CHECK_FILES() in r10156 broke cross-compilation. What I did
> is to remove the AC_CHECK_FILES() configure test and to replace #ifdef
> HAVE_PROC by VG_(have_proc_filesystem)().

Hmm, I'm not happy about either the existence of AC_CHECK_FILES or the
fix.  What is the purpose of AC_CHECK_FILES in the first place?

My understanding of this is that

* on darwin, there is no /proc filesystem
* on linux, the target platform must have a /proc filesystem, and
  we don't care if the build platform has a /proc filesystem

Is that a correct analysis?

(In reply to comment #6)
> Hmm, I'm not happy about either the existence of AC_CHECK_FILES or the
> fix.  What is the purpose of AC_CHECK_FILES in the first place?
> 
> My understanding of this is that
> 
> * on darwin, there is no /proc filesystem
> * on linux, the target platform must have a /proc filesystem, and
>   we don't care if the build platform has a /proc filesystem
> 
> Is that a correct analysis?

How about embedded Linux systems where the procfs filesystem has not been mounted ? Do we support these ? If yes, then it has to be tested whether the currently implemented solution works on such a system. If we do not want to support Linux systems where procfs has not been mounted on /proc, then a check should be added during Valgrind startup that lets valgrind abort if VG_(have_proc_filesystem)() == 0.

> How about embedded Linux systems where the procfs filesystem has not been
> mounted ? Do we support these ?

No, we don't.  We need to read /proc/self/maps at startup in order to
find out the initial memory mapping.  If that isn't possible then the
run is aborted.

I believe the following would be sufficient:

* get rid of the configure test

* for Darwin, use existing code that is #if defined(VGO_darwin) to
  find out the initial memory map.  That uses some Mach magic and 
  doesn't make any use of /proc

* for Linux, using existing code that is #if defined(VGO_linux) to read
  /proc/self/maps.  If that can't be opened then we simply give up
  (which must have always been the behaviour on Linux anyway)

I am fully in support of having V cross compilable on Linux.  But I
think the introduction of the feature test is an unnecessary complication
which in turn has necessitates your fix, whereas in fact neither are
necessary.  (I would also be the first to say that I'm not 100% confident
I understand all the constraints yet :-)

(In reply to comment #8)
> I believe the following would be sufficient:
> 
> * get rid of the configure test
> 
> * for Darwin, use existing code that is #if defined(VGO_darwin) to
>   find out the initial memory map.  That uses some Mach magic and 
>   doesn't make any use of /proc
> 
> * for Linux, using existing code that is #if defined(VGO_linux) to read
>   /proc/self/maps.  If that can't be opened then we simply give up
>   (which must have always been the behaviour on Linux anyway)

How about reintroducing the HAVE_PROC preprocessor symbol and defining this symbol on Linux but not on Darwin ?

> How about reintroducing the HAVE_PROC preprocessor symbol and defining this
> symbol on Linux but not on Darwin ?

Why bother?  We already build aspacem and everything else with the relevant
VGO_os symbol defined; we can key everything needed from that.

(In reply to comment #10)
> > How about reintroducing the HAVE_PROC preprocessor symbol and defining this
> > symbol on Linux but not on Darwin ?
> 
> Why bother?  We already build aspacem and everything else with the relevant
> VGO_os symbol defined; we can key everything needed from that.

Inserting #ifdef VGO_linux everywhere #ifdef HAVE_PROC occurred would be a bad idea. This would not only make the source code less readable but also harder to port to new operating systems.

(In reply to comment #10)
> Why bother?  We already build aspacem and everything else with the relevant
> VGO_os symbol defined; we can key everything needed from that.

Implemented in r10867 and r10868. A more portable solution can be implemented later on. Note: the code touched by these revisions is not covered by any regression tests yet. I have verified proper operation of the touched code by verifying the output of the following command:
$ ./vg-in-place --trace-children=yes bash -c 'od -c /proc/self/cmdline; ls -l /proc/self/exe'

A regression test has been added via r10871 - r10873.

3_5 rev is 10877.