12 Replies - 2806 Views - Last Post: 07 August 2012 - 04:27 PM Rate Topic: -----

#1 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Compiler writes wrong stuff !

Posted 03 August 2012 - 09:40 PM

hello, im working on some project that uses RakNet, everything seems to work fine,
only with my receiving code

Packet* packet =NULL;
unsigned char packetIdentifier;
packet = m_pRakClient->Receive();
	
if(packet )
{
    packetIdentifier = packet->data[0];
    Log("server response %d", packetIdentifier);
}




Packet struct
struct Packet
{
	/// Server only - this is the index into the player array that this playerId maps to
	PlayerIndex playerIndex;

	/// The system that send this packet.
	PlayerID playerId;

	/// The length of the data in bytes
	/// \deprecated You should use bitSize.
	unsigned int length;

	/// The length of the data in bits
	unsigned int bitSize;

	/// The data from the sender
	unsigned char* data;

	/// @internal
	/// Indicates whether to delete the data, or to simply delete the packet.
	bool deleteData;
};



Now the problem comes from accessing packet->data[0], whenever it tries to access it, the program crashs
i went through OllyDbg and debugged it, but it seems that it access the wrong offset

What should be:
mov edx, eax (packet pointer returned by the Receive func)
mov ebx, [eax + 14] (data pointer located at 0x14)
mov al, [ebx] (the packetIdentifier)



The code goes like:
mov edx, eax (packet pointer returned by the Receive func (correct))
mov ebx, [eax + 10] (it access the length int value instead of data, so ebx = 64)
mov al, [ebx] (crash happens (cant do [64]))



i've even tryed to do it that way:
unsigned char *data = *(unsigned char **)packet->data;
packetIdentifier = data[0];



but now it goes to [eax + 1A4]

PS: nothing wrong with the packet structure, the Receive function access the "packet->data[0]" and by debugging it, it access the right (0x14) offset

so what do you think ? why does it compiles that way from here, some kind of memory leak ?

Thanks in advance



Is This A Good Question/Topic? 0
  • +

Replies To: Compiler writes wrong stuff !

#2 GWatt  Icon User is offline

  • member icon

Reputation: 276
  • View blog
  • Posts: 3,075
  • Joined: 01-December 05

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 09:55 PM

If the pointer value stored in data is not a valid address then the program will likely crash when trying to dereference it. Are you sure the pointer is valid?
Was This Post Helpful? 0
  • +
  • -

#3 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 10:02 PM

Yes, im very sure, i've already verifyed that before coming here. I've even went to Olly and manually changed from " mov ebx, [eax + 10]" to "mov ebx, [eax + 14]" and it seems to work, so i verifyed it

the problem is not where the crash happens, cuz i already found it, its why it happens
Was This Post Helpful? 0
  • +
  • -

#4 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 10:07 PM

How did you come up with 0x14 as the offset for the Packet.data member? What is the the base type of PlayerIndex and PlayerID?

I'm trying to figure out how you got length at 0x10 and data at 0x14. Assuming your numbers:
0x00: playerIndex
????: playerId
0x10: length
????: bitSize
0x14: data



So for length to be at 0x10, then playerId and playerId must be 16 bytes total. Possibly to 2 64-bit integers. 2 longs? But for length and bitSize to fit in before data, that would mean that these are 16-bit integers. That looks little weird to have a 64-bit long and a 16-bit int.

Trying the other possibilities:
0x00: playerIndex
0x04: playerId
0x08: length
0x0B: bitSize
0x10: data


That looks a bit better: the length and bitSize variables match up nicely with 32 bit integers, and I can reasonably guess that playerIndex and playerId are integers and/or enums. But that completely breaks the assertion that data should be at 0x14.

Well, there is one way to be sure of the offsets:
printf("offsetof(Packet,data) is %0x\n",offsetof(Packet,data));


Was This Post Helpful? 0
  • +
  • -

#5 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 10:27 PM

sry my bad, i had to tell you about the Packet member types

struct Packet
{
	/// Server only - this is the index into the player array that this playerId maps to
	PlayerIndex playerIndex;  // 0x00 - 0x02 (unsigned short)

	/// The system that send this packet.
	PlayerID playerId; // 0x02 - 0x08 (struct containing one unsigned short and one unsigned int)

	/// The length of the data in bytes
	/// \deprecated You should use bitSize.
	unsigned int length; // 0x08 - 0x0C

	/// The length of the data in bits
	unsigned int bitSize; // 0x0C - 0x10

	/// The data from the sender
	unsigned char* data; // 0x10 - 0x14

	/// @internal
	/// Indicates whether to delete the data, or to simply delete the packet.
	bool deleteData; // 0x14 - 0x18
};



yes, you're right it access the right offset, but that offset contains the value of bitSize, cuz i verifyed that the data pointer is at 0x14

This is some code from the Receive function
else if ( packet->data[ 0 ] == ID_REMOTE_STATIC_DATA )
		{
			bitStream.IgnoreBits( 8 ); // Ignore identifier



now by goin through Olly, i see that it does (0x14) to access the data, and im 100% sure of it
Was This Post Helpful? 0
  • +
  • -

#6 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 10:45 PM

I suspect your sender forgot to do #pragma pack(1) before defining their structs. So when they send you a packet, the data pointer is at offset 0x14, but you correctly did a #pragma pack(1) on your structures so you have an offset of 0x10 for the data pointer.

#include <stdio.h>
#include <stddef.h>

typedef short PlayerIndex;

struct PlayerID1
{
    short id;
    int server;
};

struct Packet1
{
	PlayerIndex playerIndex;
	PlayerID1 playerId;
	unsigned int length;
	unsigned int bitSize;
	unsigned char* data;
	bool deleteData;
};

#pragma pack(push)
#pragma pack(1)
struct PlayerID2
{
    short id;
    int server;
};

struct Packet2
{
	PlayerIndex playerIndex;
	PlayerID2 playerId;
	unsigned int length;
	unsigned int bitSize;
	unsigned char* data;
	bool deleteData;
};
#pragma pack(pop)

int main()
{
    printf("offsetof(Packet1,data) is 0x%0x\n",offsetof(Packet1,data));
    printf("offsetof(Packet2,data) is 0x%0x\n",offsetof(Packet2,data));
}


This post has been edited by Skydiver: 03 August 2012 - 10:55 PM

Was This Post Helpful? 0
  • +
  • -

#7 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 11:09 PM

ok, lets assume that the sender forgot to do #pragma pack(1) and the data pointer is at offset 0x14, i guess that would also crash the Receive function ?

also i've written the code in another way

unsigned char RakClient::uReceive()
{
    Packet *packet = Receive();
    if(packet)
        return packet.data[0];

     return 0;
}



now, i actually succeeded to get the right message ID through calling uReceive function instead of Receive

so, what i can see is that if im trying to access the packet.data from my "CNetwork" class, i failed, but it succeed from RakClient class. even though i succeeded in getting the right message ID. i run into further crashs when dealing with the RakClient from my class

so that is a memory leak ??
Was This Post Helpful? 0
  • +
  • -

#8 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 11:26 PM

No, a memory leak is when you allocate memory and fail to free it.

This post has been edited by Skydiver: 03 August 2012 - 11:27 PM

Was This Post Helpful? 1
  • +
  • -

#9 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 03 August 2012 - 11:39 PM

Or perhaps you inadvertently included a header file that did a #pragma pack(1), but failed to reset back to the default packing of 8 byte alignment? Does the RakNet documentation give you the actual packet layout as byte layout instead of a C/C++ struct?
Was This Post Helpful? 1
  • +
  • -

#10 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Re: Compiler writes wrong stuff !

Posted 04 August 2012 - 12:52 PM

no, it dosent give the packet layout as a byte layout

about the memory leak, i just didnt meant (memory leak)
i remember, last year or before, i experienced something similar, but with my own code
i had a class thats called CGame, its properly initialized, but once i try to access its variables it crashs (even from a local function)

void CGame::DoStuff()
{
    m_iStuff = 0;   // That crashed
}



and the problem was something totally out the CGame class, i had s string with 32 bits and im writing to it a 40 bits text, so it messed up the other addresses. What i think that happened that time is the text messing out with the CGame pointer address cuz the text has overwritten it.

i was wondering if that could be something like that(but i dont think so, cuz i can work properly with my network class), and how to detect it. Or its as you said i included a header file that did a #pragma pack(1), but failed to reset back to the default packing
Was This Post Helpful? 0
  • +
  • -

#11 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 04 August 2012 - 01:03 PM

View PostJames_Alex, on 04 August 2012 - 12:52 PM, said:

and the problem was something totally out the CGame class, i had s string with 32 bits and im writing to it a 40 bits text, so it messed up the other addresses. What i think that happened that time is the text messing out with the CGame pointer address cuz the text has overwritten it.


That is what is called a buffer overrun or memory corruption.
Was This Post Helpful? 1
  • +
  • -

#12 Skydiver  Icon User is offline

  • Code herder
  • member icon

Reputation: 3589
  • View blog
  • Posts: 11,157
  • Joined: 05-May 12

Re: Compiler writes wrong stuff !

Posted 04 August 2012 - 01:18 PM

Do you have a private build of RakNet? The public documentation for the RakNet Packet does not look like your Packet:
http://www.jenkinsso..._1_1Packet.html

Additionally, the RakNet documentation seemed to be pretty conscientious about reminding to pack structures to 1 byte boundaries:
http://www.jenkinsso...ingpackets.html

So if they were that good to remind their users, chances are their own headers would have ensured that their Packet struct/class had the appropriate packing.
Was This Post Helpful? 0
  • +
  • -

#13 James_Alex  Icon User is offline

  • D.I.C Head
  • member icon

Reputation: 7
  • View blog
  • Posts: 156
  • Joined: 21-August 09

Re: Compiler writes wrong stuff !

Posted 07 August 2012 - 04:27 PM

View PostSkydiver, on 04 August 2012 - 08:39 AM, said:

Or perhaps you inadvertently included a header file that did a #pragma pack(1), but failed to reset back to the default packing of 8 byte alignment? Does the RakNet documentation give you the actual packet layout as byte layout instead of a C/C++ struct?


Thank you very much, yes, its a header that did a #pragma pack(1) but failed to reset back to the default packing, so it messed up the RakNet structures,

Thanks for the help, i really appreciate it :D
Was This Post Helpful? 0
  • +
  • -

Page 1 of 1