quick offset question

Go To Last Post
46 posts / 0 new
Author
Message
#1
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0


 

Hi freaks,

 

have a look at this piece of code.

 

 

why is the first byte in the "ByteMap" offset at what appears to be int? instead of at *(ByteMap - 0).

Thanks

 

EDITED

sector 10240 * 512 = 0x500,000

 

 

print out

 

 

 

This topic has a solution.
Last Edited: Tue. May 14, 2019 - 02:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Not sure why OP is using negative indices on ByteMap.

Not sure why OP is displaying code as a screenshot.

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay then.

 

int get_cluster(void){


	char * ByteMap = malloc(512);
	char * ptr_temp = ByteMap;


	hardware_read_block (1,  data_to_cpu32(exMasterBootRecord->ClusterHeapOffset), 1, ByteMap, 0);

	printf("\n\r byte %x", *(ByteMap - 0));
	printf("\n\r byte %x", *(ByteMap - 1));
	printf("\n\r byte %x", *(ByteMap - 2));
	printf("\n\r byte %x", *(ByteMap - 3)); // first byte at address 10240!!!


	int block = 0;
	int block_offset = 0;

	for(block = 0; (* ByteMap == 0xff); block++, ByteMap++){

		if((block % 512) == 0){

			ByteMap = ptr_temp;
			block_offset++;
			hardware_read_block (1,  data_to_cpu32(exMasterBootRecord->ClusterHeapOffset) + block_offset, 1, (char *)ByteMap, 0);
		}
	}
	int Bit = free_bit[(int)* ByteMap];


	// set free bit
	* ByteMap |= 1 << Bit;

	hardware_write_block (1,  data_to_cpu32(exMasterBootRecord->ClusterHeapOffset) + block_offset, 1, (char *)ptr_temp, 0);

	Bit = (block * 8) + Bit + 2;
	return Bit;
}

I'm using negative indices to highlight the problem.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Note that accessing off the front of the allocated block like *(Bytemap -1) is actually undefined behaviour.

 

You can print out the address of where Bytemap is pointing to with

printf("%p\n", (void *)Bytemap);

 

Bytemap will be pointing to the start of the memory allocated by malloc.

 

Show the declaration of hardware_read_block.

 

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hardware Read Block is defined as follows:

void hardware_read_block (char drive, unsigned int block, int nr_blocks, char * block_data, int request_time_out){

	sSdCard * lib_local = NULL;

	if(drive == 1){
		if(drv_1 == NULL)
			drv_1 = api_create_rw_semaphore();

		lib_local = &lib1;
		api_read_semaphore(drv_1, request_time_out);
	}
	else{
		if(drv_0 == NULL)
			drv_0 = api_create_rw_semaphore();

		lib_local = &lib0;
		api_read_semaphore(drv_0, request_time_out);
	}

	{
		api_system_gateway();
		SD_Read(lib_local, block, block_data, nr_blocks, NULL, NULL); }


	if(drive == 1)
		api_post_rd_semaphore(drv_1);
	else
		api_post_rd_semaphore(drv_0);
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

OK so block_data is char *.

Next in line is SD_Read.

Is this tried and tested code or something you are writing?

 

Going back to the original post, you're saying that you expect it to do a byte by byte copy of 512 bytes, copying 0xff 0x03 0x00 0x00 etc. to destination starting at bytemap?

If so, it does look like it has copied to destination starting at bytemap -3.

Either a bug or you're not looking at what you think you're looking at.

I did wonder about some conversion going on to 32 bit int maybe, big endian vs little endian mixups etc. but I can't see how that would cause it.

I think the printf statements are ok (ignoring the -ve index).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If this is some kind of control block why don't you read the anonymous 512 bytes to a buffer then cast an interpreting structure onto it?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

interesting idea, I tried casting it to integer to see if I could work with that but no, this one has me completely stuck.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

After the malloc, I would write some known value to bytemap[-n] up to bytemap[511] where n = some smallish value (yes the negative index is dodgy but will probably do what you expect) , then print out the values at those locations, then do the hardware_read_block, then print out the values again, verify what has changed.

 

EDIT: and check that malloc doesn't return NULL  (I don't know what you're running this on so no idea if it is likely to fail)

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

MrKendo wrote:

After the malloc, I would write some known value to bytemap[-n] up to bytemap[511] where n = some smallish value (yes the negative index is dodgy but will probably do what you expect) , then print out the values at those locations, then do the hardware_read_block, then print out the values again, verify what has changed.

 

EDIT: and check that malloc doesn't return NULL  (I don't know what you're running this on so no idea if it is likely to fail)

 

I'll do that tomorrow MrKendo.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

probably ByePtr[-n] will point at the data that malloc() itself uses to maintain the heap.  Modifying those bytes would be a bad idea if you ever want to free() the block, and maybe even if you want to ever allocate anything else.   The amount of this overhead varies pretty substantially depending on the implementation; usually there's a least a size and pointer or two.

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

interesting idea, I tried casting it to integer to see if I could work with that but no, this one has me completely stuck.

I'd suggest it's more than an "interesting idea" but is the way folks would almost always do something like reading FAT. Consider code like:

unsigned char anonymous_byte_buffer[512];

typedef struct {
	/********************frist 36 Bytes in BPB(same for FAT32 & FAT16)********************/
	BYTE BS_jmpBoot[3];//jump instruction
	BYTE OEMName[8];//OEM name
	WORD BPB_BytsPerSec;//bytes per sector
	BYTE BPB_SecPerClus;//sector per cluster
	WORD BPB_RsvdSecCnt;//reserved sectors nums
	BYTE BPB_NumFATs;//the count of FAT data structures
	WORD BPB_RootEntCnt;//entries in the root directory(FAT16/12)
	WORD BPB_TotSec16;//count of all sectors(FAT16/12)
	BYTE BPB_Media;//0xF8 is the standard value for non-removable media
	WORD BPB_FATSz16;//count of sectors occupied by ONE FAT
	WORD BPB_SecPerTrk;//sector per track for interrupt 0x13
	WORD BPB_NumHeads;//number of heads for interrupt 0x13
	DWORD BPB_HiddSec;//count of hidden sectors in this FAT volume
	DWORD BPB_TotSec32;//32-bit total count of sectors on the volume
	/********************After 36 byte Infomation(54 byte total)********************/
	//frist 28 bytes,information just for FAT32
	DWORD BPB_FATSz32;//32-bit count of sectors occupied by ONE FAT
	WORD BPB_ExtFlags;//Bits 0-3 -- Zero-based number of active FAT. Only valid if mirroring is disabled.
				//Bit 7 -- 0 means the FAT is mirrored at runtime into all FATs.
				//	  -- 1 means only one FAT is active; it is the one referenced in bits 0-3.
	WORD BPB_FSVer;//FAT32 version
	DWORD BPB_RootClus;//cluster number of the first cluster of the root directory
	WORD BPB_FSInfo;//Sector number of FSINFO structure in the reserved area of the FAT32 volume
	WORD BPB_BkBootSec;//If non-zero, indicates the sector number in the reserved area of the volume of a copy of the boot record
	BYTE BPB_Reserved[12];//Reserved,fill 0
	//down 26 bytes,information both use for FAT32/16/12
	BYTE BS_DrvNum;//Int 0x13 drive number(0x00 for floppy disks, 0x80 for hard disks)
	BYTE BS_Reserved1;//Reserved (used by Windows NT),to be 0
	BYTE BS_BootSig;//Extended boot signature (0x29)
	DWROD BS_VolID;//Volume serial number
	BYTE BS_VolLab[11];//Volume label
	BYTE BS_FilSysType[8];//File System Type String(just a string!!!)
} bpb_t;

typedef struct {
	BYTE DIR_Name[11];//file name
	BYTE DIR_Attr;//attribute byte
	BYTE DIR_NTRes;//Reserved for use by Windows NT
	BYTE DIR_CrtTimeTenth;//Millisecond stamp at file creation time
	WORD DIR_CrtTime;//Time file was created
	WORD DIR_CrtDate;//Date file was created
	WORD DIR_LstAccDate;//Last access date
	WORD DIR_FstClusHI;//High word of this entry’s first cluster number(FAT32)
	WORD DIR_WrtTime;//Time of last write
	WORD DIR_WrtDate;//Date of last write
	WORD DIR_FstClusLO;//Low word of this entry’s first cluster number
	DWORD DIR_FileSize;//32-bit DWORD holding this file’s size in bytes
} dir_ent_t;

sector_read(0, anonymous_byte_buffer);
bpb_t * pBPB = (bpb_t *)anonymous_byte_buffer;
// now use pBPB->BPB_FATSz16, pBPB->BPB_NumFATs etc to find dir etc
sector_read(calculated_dir_sect, anonymous_byte_buffer);
dir_ent_t * pDir = (dir_ent_t *)anonymous_byte_buffer;
// now use pDir->DIR_FstClustHI, pDir->DIR_FirstClustLO etc to locate file

This is not "real code". Just an example to show how you would have one anonymous array of 512 bytes with no particular meaning associated to any of the bytes to just keep reading blocks of 512. But each time you (hopefully) know what kind of sector it is you just read so then you cast that interpretation onto it and read the fields from it.

 

Ultimately when you get to the data of the file you have no idea what format it is in so in that case you just treat the whole thing as the "uchar buffer[512];" that it actually is.

 

This is all fairly standard stuff.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Hi Clawson, I'm reading the exFAT bitmap cluster. Its the same as the FAT Table in FAT32 only in bytes, hence I'm not casting to a struct but more just byte wise math.

This has me stumped. The address in the of read and writes to the byte pointer is always out by three places. 

 

Wm.

Last Edited: Fri. May 10, 2019 - 03:17 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This has me completely stump, I could try working with a array but that crashes the program.  Something seriously is a miss here.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:
, I could try working with a array but that crashes the program.
An array is what you should have started with.

In embedded programming, malloc is pretty much always a wrong choice.

In this case, you know the size and number ahead of time.

If an array crashes your system,

the choice of an array is not the problem.

Trying to use more memory than you have might be.

How much memory does your chip have?

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Well I immediately see a couple of problems:

 

1. You don't check if malloc() returns NULL. You assume it succeeds.

2. You haven't called free(ByteMap); So you're going to run out of memory after repeated calls to get_cluster()

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:
I could try working with a array but that crashes the program

In addition to other points already raised, if you're saying that replacing the malloc with

char bytemap[512];

crashes the program, I think that's further indication that you're either writing outside the bounds, or maybe you just don't have enough RAM available.

If you do think you have enough RAM, for testing purpose you could allocate a bigger array and do the copy to some offset into the array

eg. char bytemap[512 + 8];

hardware_read_block(1, sector, 1, bytemap+8, 0);

and then investigate what range actually gets written to (I'm thinking here of your suspicion that you're writing off the front of the array).

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

One more thing about (some versions of) malloc,

they like to allocate power-of-2 sized chunks of

memory.  But as westfw pointed out, some extra

bytes are allocated for book-keeping purposes, so

if you malloc(512), it may actually create a 1024-

byte block containing a few bytes for itself, your

512 bytes, and the rest wasted.  The 0x03FF (1023)

hints that this is the case here.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can you actually define "crash"?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay, I've used a array allocated of the stack.  But the problem remains!  It crashes if I don't offset the array due to the bug.

 

int get_cluster(void){


	char ByteMap[520];



	int block = 0;
	int block_offset = 0;


	hardware_read_block (1, data_to_cpu32(exMasterBootRecord->ClusterHeapOffset), 1, (char *)&ByteMap[4], 0);

	for(block = 0; ByteMap[block] == 0xff; block++){

		if((block % 512) == 0){


			block_offset++;
			hardware_read_block (1,  data_to_cpu32(exMasterBootRecord->ClusterHeapOffset) + block_offset, 1, (char *)&ByteMap[4], 0);
		}
	}
	int Bit = free_bit[ByteMap[block]];


	// set free bit
	ByteMap[block] |= 1 << Bit;

	hardware_write_block (1,  data_to_cpu32(exMasterBootRecord->ClusterHeapOffset) + block_offset, 1, (char *)&ByteMap[4], 0);

	Bit = (block * 8) + Bit + 2;
	return Bit;
}

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Please define "crashes"? If this is a 32 bit CPU do you mean some kind of hard fault or exception?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Yes, you don't give much to go on :)

You could well have multiple bugs in the code.

My suggestion to use the bigger array was simply to find out once and for all if your suspicion from post #1 is correct. ie where is hardware_read_block writing to. That's it, nothing more. One step at a time.

I imagined you would do something like

char Bytemap[520];
memset(Bytemap, Oxa5, 520);
/* print out eg. the first 10 and last 10 locations in Bytemap, should get 0xa5 for all of them */
hardware_read_block (1, sector, 1, Bytemap + 8, 0);
/* print out eg. the first 10 and last 10 locations in Bytemap, see what you get now */

Of course if you have more advance debug facilities you could just go and look at the memory content of Bytemap before and after the call, I'm assuming you don't and so need to use the printfs.

Or you could just go through the code and work out what it does :)

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Okay lads, I've stripped the problem down to just reading a sector (512 data) using the following:

 

	char ByteMap[520];

	hardware_read_block (1, data_to_cpu32(exMasterBootRecord->ClusterHeapOffset), 1, (char *)&ByteMap[0], 0);
	printf("test %x", ByteMap[0]);

It works when placed in main.  Outside of main it fails because it is some how offset by 3 bytes. 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sounds like a static allocation issue. Again define "crash". Also what CPU? What compiler?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

CPU = SAMA5D44,

COMPILER = GCC ARM NONE EABI

512MBytes or RAM

 

UPDATED the compiler.  still fails.

Last Edited: Sat. May 11, 2019 - 08:46 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Can we get back to this tomorrow.  I think it may be the malloc implementation I'm using.  I got it off git hub and never replaced it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Eh? Didn't the C compiler you use come with a complete libc then??

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

At the start I was using a implementation of malloc from GitHub.  I just never replaced it.   When I use the stdlib the kernel hangs, I'm sure I can figure that out and then I'll re-examine the error.  I also upgraded the compiler to the newest addition.

I'll get back to yous when I have that done.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

You probably need to configure the heap settings in the linker file.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Or another approach is simply to avoid 520 byte locals.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

Okay lads, I've stripped the problem down to just reading a sector (512 data) using the following:

 

	char ByteMap[520];

	hardware_read_block (1, data_to_cpu32(exMasterBootRecord->ClusterHeapOffset), 1, (char *)&ByteMap[0], 0);
	printf("test %x", ByteMap[0]);

It works when placed in main.  Outside of main it fails because it is some how offset by 3 bytes. 

 

What do you mean by "it is some how offset by 3 bytes"? How does it manifest itself?

 

Also, it is not clear why you needed all this `(char *)&ByteMap[0]` trickery instead of just plain `ByteMap` (yes, it is the same thing, but still...)

 

Last Edited: Sun. May 12, 2019 - 08:33 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

Okay, I've used a array allocated of the stack.  But the problem remains!  It crashes if I don't offset the array due to the bug.

 

In your code you are declaring an uninitialized array `ByteMap`

 

char ByteMap[520];

 

 It contains garbage initially. Then you read some data into that array starting at position `4`

 

hardware_read_block (1, data_to_cpu32(exMasterBootRecord->ClusterHeapOffset), 1, (char *)&ByteMap[4], 0);

The index range `[0, 3]` still contains garbage.

 

Then you do

 

for(block = 0; ByteMap[block] == 0xff; block++){

 

But that reads garbage from `ByteMap[0]` and so on. 

 

How did you expect this to work?

Last Edited: Sun. May 12, 2019 - 08:42 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Your quite correct, but I placed the offset to highlight the error.  You see it doesn't display garbage, it displays valid data.  The offset is there to highlight the error.

see first post.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

Your quite correct, but I placed the offset to highlight the error.  You see it doesn't display garbage, it displays valid data.  The offset is there to highlight the error.

see first post.

 

If so, then the question is: what happens in 

SD_Read(lib_local, block, block_data, nr_blocks, NULL, NULL); 

Where is this function defined?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

avr-mike wrote:

One more thing about (some versions of) malloc,

they like to allocate power-of-2 sized chunks of

memory.  But as westfw pointed out, some extra

bytes are allocated for book-keeping purposes, so

if you malloc(512), it may actually create a 1024-

byte block containing a few bytes for itself, your

512 bytes, and the rest wasted.  The 0x03FF (1023)

hints that this is the case here.

 

While it is true that `malloc` will allocate some extra memory for housekeeping purposes and that it will align the block size. But turning 512 into 1024 is too extreme even for desktop platforms, not even mentioning our platform.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

AndreyT wrote:

... not even mentioning our platform.

 

OP is using a Cortex-A5 with 512MB of memory.

AVR Freaks will discuss almost anything (though

politics and religion are taboo).

 

A company I worked at was using a fast malloc

implementation for a server application which did

what I described.

 

--Mike

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

malloc() is often implemented as a linked list so there are some additional bytes before/after the memory block that is allocated. I wonder if there's a bug in this implementation of malloc() and it's offsetting by the "hidden" link?

 

I really would switch to using the standard implementation of malloc() in libc.

This reply has been marked as the solution. 
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

replaced the malloc function with the stdlib.h and still the problem remained.  Then out of curiosity I tried allocating:

 

	exMasterBootRecord = malloc(512 * 3);
	EXdereive_boot_record(exMasterBootRecord);

and suddenly it worked, where as before I was using the sizeof operator to allocate memory for, "exMasterBootRecord ".

 

However as alreaday mentioned it would be better to use a array allocated from the stack.  This now does not work!

 

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Then it's obvious isn't it? malloc() returns pointers so exMasterBootRecord is clearly a pointer. I imagine on a 32 bit micro sizeof() is probably 4. I guess you meant sizeof(*exMasterBootRecord) ?

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

and suddenly it worked, where as before I was using the sizeof operator to allocate memory for, "exMasterBootRecord ".

 

Um... But where and how exactly did you use `sizeof` operator? The code you posted so far does not contain a single mention of `sizeof`.

Last Edited: Tue. May 14, 2019 - 06:58 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

I used the sizeof operator in the malloc just before I called the routines.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

I used the sizeof operator in the malloc just before I called the routines.

 

Well, show us the exact code with variable declarations, `malloc` and `sizeof`. Show us how you used it.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This the code that cause the bug.

 

    exMasterBootRecord = safe_malloc(sizeof(struct EXMaster_Parameter_Block));
    EXdereive_boot_record(exMasterBootRecord);

 

changed to this works

 

    exMasterBootRecord = safe_malloc(sizeof(512 * 3);
    EXdereive_boot_record(exMasterBootRecord);

 

 

And the struct

 

struct EXMaster_Parameter_Block {

    char

        JumpBoot[3],
        FileSystemName[8],
        MustBeZero[53],
        HiddenSectors[8],
        TotalSectors[8],
        FATFisrtSector[4],
        FATLength[4],
        ClusterHeapOffset[4],
        TotalClusters[4],
        FirstClusterOfRootDirectory[4],
        VolumnSerialNumber[4],
        FileSystemRevision[2],
        VolumnFlag[2],
        BytesPerSectorShift,
        SectorsPerClusterShift,
        NumberOfFats,
        DriveSelect,
        PercentInUseReserved[7],
        BootCode[390],
        BootSigniture[2],
        ExcessSpace[512];
};

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:

This the code that cause the bug.

 

    exMasterBootRecord = safe_malloc(sizeof(struct EXMaster_Parameter_Block));
    EXdereive_boot_record(exMasterBootRecord);

 

changed to this works

 

    exMasterBootRecord = safe_malloc(sizeof(512 * 3);
    EXdereive_boot_record(exMasterBootRecord);

 

Firstly, your second version with `sizeof(512 * 3)` makes no sense whatsoever. What is `sizeof` doing here?

 

Secondly, it doesn't necessarily mean that you found the cause of the bug and fixed it. If your code has any buffer overruns, then changing sizes of completely unrelated memory blocks (or order in which they are allocated) might easily "spook" the crash and produce a temporary illusion of the code "working properly". I see no reason why the variant with `sizeof(struct EXMaster_Parameter_Block)` would not work properly, unless there's something you are not telling us.

 

Most likely: no, you haven't fixed anything. You just shuffled things around, which will now simply lead to the same problem manifesting itself differently.

Last Edited: Tue. May 14, 2019 - 08:14 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Sorry, just ignore the "sizeof(512 * 3)" it really a typo. ;:

Anyway, I'll dig further into this.

  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Fianawarrior wrote:
Sorry, just ignore the "sizeof(512 * 3)" it really a typo.
That is why Odin invented copy and paste.

"Demons after money.
Whatever happened to the still beating heart of a virgin?
No one has any standards anymore." -- Giles