Re: Win64: Fwd: [PATCH] Don't replace avalue with stack memory

[Dan Witte <dwitte at mozilla dot com>]

Ah, I see. Which isn't a problem for any of the other types, since they get pushed on the stack or into registers.

FWIW, it seems natural to me that the args given to ffi_call are stompable.

Attached is a patch to go with Makoto's, adding a comment to the docs and the changelog.

Dan

----- "Timothy Wall" <twall@users.sf.net> wrote:

> I think the original intent was to avoid allowing the callee to write
> to any of the original inputs (preserving call by value semantics for
> a structure).  If the tests still pass without it, it can go away
> (since no other targets seem to care about it), although something
> should be put into the documentation about structure arguments passed
> by value are not necessarily unmodified for the caller.
> 
> 
> On May 11, 2010, at 11:41 AM, Dan Witte wrote:
> 
> > Timothy should probably look at this; fwding just to make sure.
> > 
> > (I've no idea why it copied to the stack in the first place.)
> > 
> > Cheers,
> > Dan
> > 
> > 
> > ----- Forwarded Message -----
> > From: "Makoto Kato" <m_kato@ga2.so-net.ne.jp>
> > To: libffi-discuss@sourceware.org
> > Sent: Monday, May 10, 2010 7:32:02 PM
> > Subject: [PATCH] Don't replace avalue with stack memory
> > 
> > Hi, fork.
> > 
> > ffi_call may overrides avalue when type is structure or long
> double.
> > When overriding it, ffi_call doesn't restore it.  The caller don't
> > know whether it is on stack or on heap, so this may causes heap
> > corruption.
> > 
> > I think that it is unnecessary to replace it with stack memory by
> > alloca().  If needed, we should restore avalue.
> > 
> > This is Win64 implementation only.
> > 
> > 
> > -- 
> > Makoto Kato <m_kato@ga2.so-net.ne.jp>
> > <0001-PATCH-Don-t-replace-avalue-with-stack-memory.patch>

diff --git a/ChangeLog b/ChangeLog
index 97994bc..ec252bf 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,8 +1,12 @@
+2010-05-11  Makoto Kato <m_kato@ga2.so-net.ne.jp>
+
+	* src/x86/ffi.c (ffi_call): Don't copy structs passed by value.
+
 2010-05-05  Michael Kohler <michaelkohler@live.com>
 
 	* src/dlmalloc.c (dlfree): Fix spelling.
 	* src/ia64/ffi.c (ffi_prep_cif_machdep): Ditto.
 	* configure.ac: Ditto.
 	* configure: Rebuilt.
 
 2010-04-13  Dan Witte  <dwitte@mozilla.com>
diff --git a/doc/libffi.texi b/doc/libffi.texi
index cbd78aa..53ff595 100644
--- a/doc/libffi.texi
+++ b/doc/libffi.texi
@@ -166,17 +166,19 @@ result and must be suitably aligned; it is the caller's responsibility
 to ensure this.  If @var{cif} declares that the function returns
 @code{void} (using @code{ffi_type_void}), then @var{rvalue} is
 ignored.  If @var{rvalue} is @samp{NULL}, then the return value is
 discarded.
 
 @var{avalues} is a vector of @code{void *} pointers that point to the
 memory locations holding the argument values for a call.  If @var{cif}
 declares that the function has no arguments (i.e., @var{nargs} was 0),
-then @var{avalues} is ignored.
+then @var{avalues} is ignored. Note that argument values may be modified by
+the callee (for instance, structs passed by value); the burden of copying
+pass-by-value arguments is placed on the caller.
 @end defun
 
 
 @node Simple Example
 @section Simple Example
 
 Here is a trivial example that calls @code{puts} a few times.