CopyMemory causes Access Violation on Win8

Question

I have a piece of code that compiles using Delphi XE3 into 64-bit COM DLL.

function TRPMFileReadStream.Read(var Buffer; const Count: Longint): Longint;
begin
  if ((Self.FPosition >= 0) and (Count > 0)) then
  begin
    Result := Self.FSize - Self.FPosition;
    if ((Result > 0) and (Result >= Count)) then
    begin
      if (Result > Count) then
      begin
        Result := Count;
      end;
      CopyMemory(
        Pointer(@Buffer),
        Pointer(LongWord(Self.FMemory) + Self.FPosition),
        Result
      );
      Inc(Self.FPosition, Result);
      Exit;
    end;
  end;
  Result := 0;
end;

On Win7-64bit, the above works fine. but On Win8-64bit, The same DLL file will throw Access Violation on CopyMemory. The CopyMemory is implemented in WinAPI.windows unit.

It is like this.

procedure CopyMemory(Destination: Pointer; Source: Pointer; Length: NativeUInt);
begin
  Move(Source^, Destination^, Length);
end;

Any ideas? Thanks.

why u use CopyMemory instead of standard PAscal `Move` procedure ? Or , if you don;'t like standard functions, an optimized one from http://FastCode.sf.net ? — Arioch 'The, Apr 10 '13 at 15:55
@Arioch'The The 32 bit compiler uses the FastCode `Move`, there is no 64 bit FastCode `Move`. — David Heffernan, Apr 10 '13 at 15:58
I can't speak for Doctorlai, @Arioch, but *I* prefer `CopyMemory` because I find pointers easier to grok than untyped reference parameters, and I work in multiple languages daily, so having a common API is helpful. Besides, since the API forwards everything *directly* to the built-in function, it hardly makes a difference which one anyone chooses. — Rob Kennedy, Apr 10 '13 at 16:01
Since `CopyMemory` is inlined the same code is emitted by the compiler. So, it comes down to personal choice. — David Heffernan, Apr 10 '13 at 16:03
@RobKennedy i prefer to use language conventions. In C++, where references are not popular, i'd use pointers, but in pascal i'd use references, just to be consistent with most of code. — Arioch 'The, Apr 10 '13 at 16:29
@Arioch'The In modern C++, pointers are seldom used and references are common. — David Heffernan, Apr 10 '13 at 17:02
@David i heard that FastCode was banend from AnyDAC after purchase, i wonder if it would also be banned from XE4 or XE5... — Arioch 'The, Apr 11 '13 at 14:21

score 7 · Accepted Answer · answered Apr 10 '13 at 15:52

7

At this point:

Pointer(LongWord(Self.FMemory) + Self.FPosition)

you truncate a 64 bit pointer to 32 bit. Hence the access violation. Instead you need

Pointer(NativeUInt(Self.FMemory) + Self.FPosition)

Your code is just as broken on Win7, but somehow you were unlucky and only ever ran this code with pointers with address < 4GB.

You should run some top-down memory allocation testing to flush out any other such errors.

answered Apr 10 '13 at 15:52

David Heffernan

601,492
42
1,072
1,490

thanks. great help. yes. i just realised this stupid mistake. I will do a quick replace to all LongWord(x) to NativeUInt() for all memory pointers. – justyy Apr 10 '13 at 15:57
Heed my advice and run top down allocation tests. You will find more errors I am sure. – David Heffernan Apr 10 '13 at 15:58
1

How specific to run top down allocation tests? – justyy Apr 10 '13 at 16:04
Look here: http://stackoverflow.com/questions/544296/as-a-programmer-what-do-i-need-to-worry-about-when-moving-to-64-bit-windows#545097 – David Heffernan Apr 10 '13 at 16:06
2

@DoctorLai I rejected your suggested edit. Sorry. I really meant unlucky. It is unlucky that the bug did not manifest until now. Any code that is out in the wild has the bug. If the bug had manifested earlier, less flawed code could have escaped. Perhaps you haven't released this code yet, but you know what I mean. – David Heffernan Apr 10 '13 at 16:17
ok. thanks.... I meant 'lucky' with the quotes. :) yes, it is good to find this bug at this stage. On my laptop Win7-64bit 8GB, previously testing didn't reveal any bugs. – justyy Apr 10 '13 at 16:23
@DoctorLai, do a search for `Cardinal(x)` as well. – LU RD Apr 10 '13 at 16:54
@LURD and `Integer()` and `Longint()` too. – David Heffernan Apr 10 '13 at 17:02
thanks. It is usually more likely to cast 32-bit Pointer to LongWord other than signed 32-integers e.g. Integer, Longint. – justyy Apr 10 '13 at 19:10
@DoctorLai: it would be better to change your code to use actual pointer arithmetic instead of type-casting your pointers to use integer arithmetic. That way, the compiler can handle the pointer sizes for you regardless of whether they are 32-bit or 64-bit in size. – Remy Lebeau Apr 10 '13 at 19:41
I agree with @Remy, the code in his answer is preferable to that in my answer. – David Heffernan Apr 10 '13 at 19:43
Oh, great! then let me ask for `DWORD(`, `Int32(` and `UInt32(` typecasts to be searched for too :-D – Arioch 'The Apr 11 '13 at 14:23

score 4 · Answer 2 · answered Apr 10 '13 at 19:39

4

David pointed out the root of your problem - your pointer type-casting is wrong for 64-bit. A better solution is to use pointer arithmetic instead and let the compiler handle the pointer sizes for you:

CopyMemory(
  @Buffer,
  PByte(Self.FMemory) + Self.FPosition,
  Result
);

Or:

Move(
  (PByte(Self.FMemory) + Self.FPosition)^,
  Buffer,
  Result
);

answered Apr 10 '13 at 19:39

Remy Lebeau

555,201
31
458
770

thanks.. so this PByte thing will require {$POINTERMATH ON} to the trick, right?. Both produce same code, right? – justyy Apr 10 '13 at 21:19
`POINTERMATH` is automatically turned `ON` by the RTL for `PByte` only, but it needs to be turned `ON` manually for any other pointer type you want to use pointer arithmetic with. – Remy Lebeau Apr 10 '13 at 22:07
I think by historical accident it is on for `PAnsiChar` also. – David Heffernan Apr 10 '13 at 22:43

CopyMemory causes Access Violation on Win8

2 Answers2