valid_pointer_p

valid_pointer_p

[Eli Zaretskii]

Can someone ``in the know'' please explain what clever idea is behind
the function valid_pointer_p, and whether that idea is supposed to be
portable?  This function was previously used only if !GC_MARK_STACK,
but now its use was extended to all configurations, so I think at the
very least its commentary should be expanded and clarified, even if it
turns out that its method is universally right.

Here's the code of the function, for your convenience:

    /* Determine whether it is safe to access memory at address P.  */
    int
    valid_pointer_p (p)
	 void *p;
    {
      int fd;

      /* Obviously, we cannot just access it (we would SEGV trying), so we
	 trick the o/s to tell us whether p is a valid pointer.
	 Unfortunately, we cannot use NULL_DEVICE here, as emacs_write may
	 not validate p in that case.  */

      if ((fd = emacs_open ("__Valid__Lisp__Object__", O_CREAT | O_WRONLY | O_TRUNC, 0666)) >= 0)
	{
	  int valid = (emacs_write (fd, (char *)p, 16) == 16);
	  emacs_close (fd);
	  unlink ("__Valid__Lisp__Object__");
	  return valid;
	}

	return -1;
    }

Re: valid_pointer_p

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Can someone ``in the know'' please explain what clever idea is behind
> the function valid_pointer_p, and whether that idea is supposed to be
> portable?

While I agree that the function is non-portable at best, it is only used
for debugging (called via safe_debug_print by some gdb macros), nothing
inside Emacs is calling it.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: valid_pointer_p

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: emacs-devel@gnu.org
> Date: Sat, 29 Jul 2006 12:45:58 +0200
> 
> While I agree that the function is non-portable at best, it is only used
> for debugging (called via safe_debug_print by some gdb macros), nothing
> inside Emacs is calling it.

Sure, but GDB is available on Windows as well.

Thanks for the feedback, anyway.

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

> Can someone ``in the know'' please explain what clever idea is behind
> the function valid_pointer_p, and whether that idea is supposed to be
> portable?  

If you have some better way to do this on some platforms, please tell me.

>            This function was previously used only if !GC_MARK_STACK,
> but now its use was extended to all configurations, 

It is used specifically to validate OBJ in

   pp obj

where obj is a SUBR (i.e. built-in DEFUN).

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Sun, 30 Jul 2006 02:05:25 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Can someone ``in the know'' please explain what clever idea is behind
> > the function valid_pointer_p, and whether that idea is supposed to be
> > portable?  
> 
> If you have some better way to do this on some platforms, please tell me.

Well, I really don't understand what are the assumptions of the code.
Are you assuming that accessing an invalid pointer inside a system
call (such as `read') will never segfault?  Does Posix really mandate
that?  Should we ask people to try that on different platforms?

It goes without saying that on MS-Windows, the code does segfault if
the argument is an invalid pointer.

As for other ways, we could, for example, set up a temporary signal
handler for SIGSEGV around the call to valid_pointer_p.  That should
work on most, if not all, supported platforms.

Then there's the procfs API, which probably lets you actually read
from the process memory on those platforms where procfs is available.

On Windows, we could try reading from the address using the
ReadProcessMemory API, which is used by debuggers.  (If ptrace allows
reading from the calling process, we could do the same on Posix
platforms.)

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: emacs-devel@gnu.org
>> From: storm@cua.dk (Kim F. Storm)
>> Date: Sun, 30 Jul 2006 02:05:25 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > Can someone ``in the know'' please explain what clever idea is behind
>> > the function valid_pointer_p, and whether that idea is supposed to be
>> > portable?  
>> 
>> If you have some better way to do this on some platforms, please tell me.
>
> Well, I really don't understand what are the assumptions of the code.
> Are you assuming that accessing an invalid pointer inside a system
> call (such as `read') will never segfault?  Does Posix really mandate
> that?  

On the Linux kernel, write returns -1 with errno == EFAULT if the
provided buffer is invalid.

But, POSIX write spec does not say anything about invalid buffer or EFAULT.
So, indeed the current code is not portable.

>       Should we ask people to try that on different platforms?

IMO, it is not worth it.  This is a rare corner case.

> It goes without saying that on MS-Windows, the code does segfault if
> the argument is an invalid pointer.

.. but that's no worse than before I added pp / safe_debug_print.

And do people usually debug emacs with GDB on windows?

>
> As for other ways, we could, for example, set up a temporary signal
> handler for SIGSEGV around the call to valid_pointer_p.  That should
> work on most, if not all, supported platforms.
>
> Then there's the procfs API, which probably lets you actually read
> from the process memory on those platforms where procfs is available.
>
> On Windows, we could try reading from the address using the
> ReadProcessMemory API, which is used by debuggers.  (If ptrace allows
> reading from the calling process, we could do the same on Posix
> platforms.)

All of this sounds more or less complicated, but if someone want to
give one of these methods a try, fine with me.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Mon, 31 Jul 2006 00:13:26 +0200
> 
> > It goes without saying that on MS-Windows, the code does segfault if
> > the argument is an invalid pointer.
> 
> .. but that's no worse than before I added pp / safe_debug_print.

Yes, but that doesn't sound like a good argument to me.  If we thing
that segfaulting is a bug, let's fix it in the best way we can.

> And do people usually debug emacs with GDB on windows?

I do it all the time.  What other debugger can I use to debug Emacs
built with MinGW's port of GCC?  Windows debuggers don't understand
the debug info emitted by GCC.

> > As for other ways, we could, for example, set up a temporary signal
> > handler for SIGSEGV around the call to valid_pointer_p.  That should
> > work on most, if not all, supported platforms.
> >
> > Then there's the procfs API, which probably lets you actually read
> > from the process memory on those platforms where procfs is available.
> >
> > On Windows, we could try reading from the address using the
> > ReadProcessMemory API, which is used by debuggers.  (If ptrace allows
> > reading from the calling process, we could do the same on Posix
> > platforms.)
> 
> All of this sounds more or less complicated, but if someone want to
> give one of these methods a try, fine with me.

If no one else cares about this, I will at least write the code to
DTRT on Windows.  I could also show you the code to set up a signal
handler, if you wish.

Re: valid_pointer_p

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> I could also show you the code to set up a signal handler, if you wish.

A signal handler would not work since all signals are intercepted by the
debugger.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

> If no one else cares about this, I will at least write the code to
> DTRT on Windows.  

Please do.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Date: Mon, 31 Jul 2006 10:30:21 +0200
> Cc: emacs-devel@gnu.org, "Kim F. Storm" <storm@cua.dk>
> 
> A signal handler would not work since all signals are intercepted by the
> debugger.

Right, I forgot about that.

Does ptrace work on the calling process, and if so, can one use it to
read a certain address without triggering a segfault?  How about
procfs?

Re: valid_pointer_p

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> Does ptrace work on the calling process,

A process can only be traced once, at least on Linux.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Mon, 31 Jul 2006 11:01:26 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > If no one else cares about this, I will at least write the code to
> > DTRT on Windows.  
> 
> Please do.

Done.

Btw, you always try to validate a 16-byte region starting at the
address P, which might give a false positive if the object is actually
smaller and happens to be at the end of an allocated page which is
followed by a page that we cannot access.  Am I missing something?

Re: valid_pointer_p

[Eli Zaretskii]

> Date: Sat, 05 Aug 2006 16:04:22 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: emacs-devel@gnu.org
> 
> Btw, you always try to validate a 16-byte region starting at the
> address P, which might give a false positive
                                ^^^^^^^^^^^^^^
Err... make that ``false negative'', meaning that a pointer is
declared invalid while it actually is valid.

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

>> Cc: emacs-devel@gnu.org
>> From: storm@cua.dk (Kim F. Storm)
>> Date: Mon, 31 Jul 2006 11:01:26 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > If no one else cares about this, I will at least write the code to
>> > DTRT on Windows.  
>> 
>> Please do.
>
> Done.
>
> Btw, you always try to validate a 16-byte region starting at the
> address P, which might give a false positive if the object is actually
> smaller and happens to be at the end of an allocated page which is
> followed by a page that we cannot access.  Am I missing something?

I validate a 16 area since the pointer is typically something
"vector-like" where the code that access/print that object will look
at data following the immedate byte/word that the pointer addresses.

So I made the "safe choice" of checking 16 bytes.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Sun, 06 Aug 2006 00:11:30 +0200
> 
> > Btw, you always try to validate a 16-byte region starting at the
> > address P, which might give a false positive if the object is actually
> > smaller and happens to be at the end of an allocated page which is
> > followed by a page that we cannot access.  Am I missing something?
> 
> I validate a 16 area since the pointer is typically something
> "vector-like" where the code that access/print that object will look
> at data following the immedate byte/word that the pointer addresses.

Well, I figured that much.  But we do know the length of each object,
so we could validate exactly what is needed, right?  Just add another
argument to valid_pointer_p that tells it how many bytes to validate.

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

> Well, I figured that much.  But we do know the length of each object,
> so we could validate exactly what is needed, right?  Just add another
> argument to valid_pointer_p that tells it how many bytes to validate.

The problem is that you have to deref the pointer to know the length
of the object...

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Sat, 12 Aug 2006 00:58:05 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Well, I figured that much.  But we do know the length of each object,
> > so we could validate exactly what is needed, right?  Just add another
> > argument to valid_pointer_p that tells it how many bytes to validate.
> 
> The problem is that you have to deref the pointer to know the length
> of the object...

??? Don't we have macros, like INTEGERP, SUBRP, etc. to do that
without dereferencing?  The length of the primitive Lisp types is
known, right?

Even if you are right, dereferencing a pointer accesses a region in
memory whose length is known in advance, so at most we will need to
call valid_pointer_p twice.

Re: valid_pointer_p

[Andreas Schwab]

Eli Zaretskii <eliz@gnu.org> writes:

> ??? Don't we have macros, like INTEGERP, SUBRP, etc. to do that
> without dereferencing?  The length of the primitive Lisp types is
> known, right?

Try to find out the size of a vector object.

    /* Vector of Lisp objects, or something resembling it.
       XVECTOR (object) points to a struct Lisp_Vector, which contains
       the size and contents.  The size field also contains the type
       information, if it's not a real vector object.  */
    Lisp_Vectorlike,

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: valid_pointer_p

[Eli Zaretskii]

> From: Andreas Schwab <schwab@suse.de>
> Cc: storm@cua.dk (Kim F. Storm), emacs-devel@gnu.org
> Date: Sat, 12 Aug 2006 14:36:22 +0200
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > ??? Don't we have macros, like INTEGERP, SUBRP, etc. to do that
> > without dereferencing?  The length of the primitive Lisp types is
> > known, right?
> 
> Try to find out the size of a vector object.
> 
>     /* Vector of Lisp objects, or something resembling it.
>        XVECTOR (object) points to a struct Lisp_Vector, which contains
>        the size and contents.  The size field also contains the type
>        information, if it's not a real vector object.  */
>     Lisp_Vectorlike,

I think something like this should work for such cases:

      struct Lisp_Vector *p = XVECTOR (obj);

      if (valid_pointer_p (p, offsetof (struct Lisp_Vector, size)
      	 		      + sizeof (EMACS_INT))
      	  && valid_pointer_p (p, p->size))
	return 1;  /* valid */

(Modulo the complications of storing other information in the leading
bits of the `size' field of struct Lisp_Vector.)  This is what I meant
when I wrote:

    Even if you are right, dereferencing a pointer accesses a region in
    memory whose length is known in advance, so at most we will need to
    call valid_pointer_p twice.

Am I missing something?

Re: valid_pointer_p

[Kim F. Storm]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Andreas Schwab <schwab@suse.de>
>> Cc: storm@cua.dk (Kim F. Storm), emacs-devel@gnu.org
>> Date: Sat, 12 Aug 2006 14:36:22 +0200
>> 
>> Eli Zaretskii <eliz@gnu.org> writes:
>> 
>> > ??? Don't we have macros, like INTEGERP, SUBRP, etc. to do that
>> > without dereferencing?  The length of the primitive Lisp types is
>> > known, right?
>> 
>> Try to find out the size of a vector object.
>> 
>>     /* Vector of Lisp objects, or something resembling it.
>>        XVECTOR (object) points to a struct Lisp_Vector, which contains
>>        the size and contents.  The size field also contains the type
>>        information, if it's not a real vector object.  */
>>     Lisp_Vectorlike,
>
> I think something like this should work for such cases:
>
>       struct Lisp_Vector *p = XVECTOR (obj);
>
>       if (valid_pointer_p (p, offsetof (struct Lisp_Vector, size)
>       	 		      + sizeof (EMACS_INT))
>       	  && valid_pointer_p (p, p->size))
> 	return 1;  /* valid */
>
> (Modulo the complications of storing other information in the leading
> bits of the `size' field of struct Lisp_Vector.)  This is what I meant
> when I wrote:
>
>     Even if you are right, dereferencing a pointer accesses a region in
>     memory whose length is known in advance, so at most we will need to
>     call valid_pointer_p twice.
>
> Am I missing something?

No.  My simple approach was simply to assume that if you can access the
size field of a vector, you can access the rest as well.  I don't mind
making a more correct implementation, although I didn't see the need --
after all, this is for a corner case of debugging emacs.

-- 
Kim F. Storm <storm@cua.dk> http://www.cua.dk

Re: valid_pointer_p

[Eli Zaretskii]

> Cc: Andreas Schwab <schwab@suse.de>,  emacs-devel@gnu.org
> From: storm@cua.dk (Kim F. Storm)
> Date: Sat, 12 Aug 2006 22:19:50 +0200
> 
> > Am I missing something?
> 
> No.  My simple approach was simply to assume that if you can access the
> size field of a vector, you can access the rest as well.  I don't mind
> making a more correct implementation, although I didn't see the need --
> after all, this is for a corner case of debugging emacs.

It could be annoying to get INVALID for a perfectly valid object, and
fixing the current code seems easy enough.  But I don't have time to
do this myself, so it's your call.