memory alignment

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

Guys, I have the following code to align memory of the stack:

	// Allocate Memory of the Stack for the recursive call
	unsigned char buffer[cluster_size(file) + 16)];
	unsigned char * cache;

	// Align Memory Allocation for recursive call
	cache = (unsigned char *)(((unsigned int)&buffer[0] + 16) & ~0x0f); //alignment for 4 bytes

Can anyone see why this is causing issues.  It works if I increase the allocation.  Say to:

unsigned char buffer[cluster_size(file) + 512)];

EDITED: The error appears as though I over stepping the memory boundary.

This topic has a solution.
Last Edited: Fri. Dec 24, 2021 - 05:30 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Surely, this will depend on what processor you're using - it's not just "General Programming" ?

 

And, whatever it is, it's clearly not AVR

Top Tips:

  1. How to properly post source code - see: https://www.avrfreaks.net/comment... - also how to properly include images/pictures
  2. "Garbage" characters on a serial terminal are (almost?) invariably due to wrong baud rate - see: https://learn.sparkfun.com/tutorials/serial-communication
  3. Wrong baud rate is usually due to not running at the speed you thought; check by blinking a LED to see if you get the speed you expected
  4. Difference between a crystal, and a crystal oscillatorhttps://www.avrfreaks.net/comment...
  5. When your question is resolved, mark the solution: https://www.avrfreaks.net/comment...
  6. Beginner's "Getting Started" tips: https://www.avrfreaks.net/comment...
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

If your buffer was already 16byte aligned, your code will bump it up to the NEXT 16bit block, which is beyond the end.

try adding 15 instead.

 

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

Properly aligning the buffer to begin with (https://en.cppreference.com/w/c/... if using C, https://en.cppreference.com/w/cp... if using C++) seems like the simplest way to deal with this. C++ also has https://en.cppreference.com/w/cp... which can be used if the buffer can't be properly aligned to begin with.

build-avr-gcc: avr-gcc build script

toolchain-avr-gcc: CMake toolchain for cross compiling for the AVR family of microcontrollers

avr-libcpp: C++ standard library partial implementation (C++17 only) for use with avr-gcc/avr-libc

picolibrary: C++ microcontroller driver/utility library targeted for use with resource constrained microcontrollers

picolibrary-microchip-megaavr: picolibrary HIL for megaAVR microcontrollers

picolibrary-microchip-megaavr0: picolibrary HIL for megaAVR 0-series microcontrollers

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

Is the alignment supposed to be for 4 bytes or for 16 bytes?

The code, unlike the comment, suggests 16.

Moderation in all things. -- ancient proverb

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

no joy.  I'll leave the problem for another day and hope that I have some inspiration.  I have the code working but I'll not be happy until it does what it is suppose to do....

 

Thanks Guys!

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

Fianawarrior wrote:
I have the code working

 

First and foremost, you need to start posting real code. You posted a fake one, which is evident from unbalanced parentheses. And you expect people to somehow guess the problem from it? This is not going to work.

 

Fianawarrior wrote:

// Align Memory Allocation for recursive call
cache = (unsigned char *)(((unsigned int)&buffer[0] + 16) & ~0x0f); //alignment for 4 bytes

 

Also, why are you casting pointer values to `unsigned int`? Where did you get that idea? The language provides you with `uintptr_t` specifically so that you won't have to do something as silly as casting to `unsigned int`.

 

Also also, what's with the misleading comments? The expression obviously tries to align to 16 bytes, but the comment talks about 4.

 

Finally, if you want to align an integer value `v` to a N-byte boundary, the well-known formula is `v = (v + N - 1) / N * N`. For powers of 2 the div-mul part can be "optimized" into an `&` with a bit mask (which is what you have done already, and without a good reason). But the key moment here is that you have to add one less than N. If you want to align to 16-byte boundary, you have to add 15 before &-ing, not 16. Play with different values, see how it works. Should make it clear why you need to add one less than the desired boundary.

Dessine-moi un mouton

Last Edited: Sat. Nov 6, 2021 - 07:27 AM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

This works perfectly!

unsigned char buffer[cluster_size(file)]__attribute__((aligned(16)));
	

EDITED: had problems with recursion in the function, so tried:

unsigned char buffer[cluster_size(file)]__attribute__((aligned(32)));

And it works!!!

 

See the function below:

void delete_folder_contents(FILEX * file){

	int source = file->dir_clusters->sequence;
	dir_insert_cluster(file);

	/**
	 * Using the GNU Compiler Collection (GCC) aligned (alignment) This
	 * attribute specifies a minimum alignment for the variable or
	 * structure field, measured in bytes. For example, the
	 * declaration: int x __attribute__ ( (aligned (16))) = 0;
	 * causes the compiler to allocate the global variable x on a
	 * 16-byte boundary.
	 */
	// Allocate Memory of the Stack for the recursive call
	unsigned char buffer[cluster_size(file)]__attribute__((aligned(32)));
	unsigned char * cache;

	// Align Memory Allocation for recursive call
	cache = (unsigned char *)&buffer[0]; //alignment for 4 bytes

	// perform DMA read of directory location to ensure the data is up-to-date
	dma_hardware_read(1, file->cluster_addr, sectors_per_cluster(file), cache, 0);


	struct directory_entry     * directory = (struct directory_entry * )cache;
	struct stream_extension    * streamExt;
	struct file_name_extension * nameExt;


	char entry_name[255];
	unsigned char violation = 0;

	unsigned char entry_type = 0;

	// begin search
	while(1){

		if(directory->entry_type == DATAEXT){

			if(directory->file_attributes[0] == exFOLDER)
				entry_type = exFOLDER;
			else
				entry_type = exFILEX;

			streamExt = (struct stream_extension    *) directory + 1;


			// handle boundary violation!!!
			if(boudary_configure(file, cache, (unsigned char **)&streamExt, (unsigned char **)&directory, &violation) == 0)
				break;



			if(streamExt->entry_type == STREAMEXT){

				int name_length = streamExt->name_length;

				nameExt = (struct file_name_extension *) streamExt + 1;

				// handle boundary violation!!!
				if(boudary_configure(file, cache, (unsigned char **)&nameExt, (unsigned char **)&directory, &violation) == 0)
					break;



				if(nameExt->entry_type == FILEXEXT){

					unsigned char * ptr = nameExt->file_name;

					/** exFAT files have only one name, which is encoded as Unicode on disk and can have up to 255 characters. */
					int i;
					for(i = 0; i < name_length; i++){
						entry_name[i] = * ptr; ptr += 2;

						if(((i % 14) == 0) && (i != 0)){
							nameExt++;

							// handle boundary violation!!!
							if(boudary_configure(file, cache, (unsigned char **)&nameExt, (unsigned char **)&directory, &violation) == 0)
								break;

							ptr = &nameExt->file_name[0];
						}
					}entry_name[i] = '\0';

					if(violation)
						delete_load_prev_cluster(file);


					if(entry_type == exFOLDER){

						// Open Folder & Delete the contents
						exfat_open_folder(file, &entry_name[0]);

						/**
						 * Recursion: In programming terms, a recursive function can be defined as a routine that calls
						 * itself directly or indirectly. Using the recursive algorithm, certain problems can be solved
						 * quite easily. Towers of Hanoi (TOH) is one such programming exercise. Try to write an
						 * iterative algorithm for TOH. Moreover, every recursive program can be written using iterative
						 * methods.
						 */
						delete_folder_contents(file);
						exfat_return(file);

						c_printf("\r[m]Deleting %s", &entry_name[0]);
						exfat_delete_general_entry(file, &entry_name[0]);
					}
					else{
						c_printf("\r[m]Deleting %s", &entry_name[0]);
						exfat_delete_general_entry(file, &entry_name[0]);
					}

					if(violation)
						delete_load_next_cluster(file);
				}
			}
		}
		directory++;

		// handle boundary violation!!!
		if(boudary_configure(file, cache, (unsigned char **)&directory, (unsigned char **)&directory, &violation) == 0)
			break;

		violation = 0;
	}
	return_to_source(file, source);
}

 

Last Edited: Wed. Nov 10, 2021 - 02:15 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

Last part, I had to allign to 32.  So how do I adopt this code to 32.

 

		file->cache = (unsigned char *)cache_alloc(cluster_size(file) + 15);
		file->cache = (unsigned char *)(((unsigned int)file->cache + 15) &~ 0x0f); //alignment for 4 bytes

 

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

Replace 15 with 31 (and 0x0f is also a 15 that needs to be replaced)

/Lars

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

Thanks Lajon.

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

guys I have a bug, when I try to copy more than 1003 folders I get a filename error.  Basically the function works correctly.  It appears to be the memory allocation and alignment that is causing the problem. When I declare the buffer as thus:

	unsigned char buffer[cluster_size(file_src) + 32]__attribute__((aligned(32)));

with the extra 32 bytes it works perfectly.

This function is a recursive function.

 

Anyone any thoughts on what is going on here?

The function is below, basically it opens a folder a copies it contents, if it finds another folder it will open it and copy the contents and when it has copied it contents it returns back to the calling function.

 

static void copy_folder(FILEX * file_src, FILEX * file_dest, unsigned int copy_cluster, unsigned long long folder_size){

	unsigned int source = file_src->dir_clusters->sequence;
	dir_insert_cluster(file_src);

	unsigned int copy_cluster_addr = cluster_addr(file_dest, copy_cluster);

	// allocate cluster buffer for the recursive call
	/**
	 * Using the GNU Compiler Collection (GCC) aligned (alignment) This
	 * attribute specifies a minimum alignment for the variable or
	 * structure field, measured in bytes. For example, the
	 * declaration: int x __attribute__ ( (aligned (16))) = 0;
	 * causes the compiler to allocate the global variable x on a
	 * 16-byte boundary.
	 */
	unsigned char buffer[cluster_size(file_src) + 32]__attribute__((aligned(32)));
	unsigned char * cache;

	// Align Memory Allocation for recursive call
	cache = (unsigned char *)&buffer[0]; //alignment for 4 bytes

	// perform DMA read of directory location to ensure the data is up-to-date
	dma_hardware_read(1, file_src->cluster_addr, sectors_per_cluster(file_src), cache, 0);


	struct directory_entry     * directory = (struct directory_entry * )cache;
	struct stream_extension    * streamExt;
	struct file_name_extension * nameExt;


	char entry_name[255];
	unsigned char violation = 0;

	unsigned char entry_type = 0;
	unsigned int source_file_cluster = 0;

	// begin search
	while(1){

		if(directory->entry_type == DATAEXT){

			// find entry type - later use
			if(directory->file_attributes[0] == exFOLDER)
				entry_type = exFOLDER;
			else
				entry_type = exFILEX;

			streamExt = (struct stream_extension *) directory + 1;
			directory++;

			// handle boundary violation!!!
			if(boudary_configure_copy(file_src, cache, (unsigned char **)&streamExt, (unsigned char **)&directory, &violation, &folder_size) == 0)
				break;



			if(streamExt->entry_type == STREAMEXT){

				// prepare to copy file data for new folder/file
				unsigned long long data_size = data_to_cpu64(&streamExt->data_length[0]);
				unsigned int name_length = streamExt->name_length;

				source_file_cluster = data_to_cpu32(&streamExt->first_cluster[0]);


				nameExt = (struct file_name_extension *) streamExt + 1;
				directory++;

				// handle boundary violation!!!
				if(boudary_configure_copy(file_src, cache, (unsigned char **)&nameExt, (unsigned char **)&directory, &violation, &folder_size) == 0)
					break;



				if(nameExt->entry_type == FILEXEXT){

					unsigned char * ptr = &nameExt->file_name[0];

					/** exFAT files have only one name, which is encoded as Unicode on disk and can have up to 255 characters. */
					int i;
					for(i = 0; i < name_length; i++){

						if(((i % 15) == 0) && (i != 0)){
							nameExt++;
							directory++;

							// handle boundary violation!!!
							if(boudary_configure_copy(file_src, cache, (unsigned char **)&nameExt, (unsigned char **)&directory, &violation, &folder_size) == 0)
								break;

							ptr = &nameExt->file_name[0];
						}
						entry_name[i] = * ptr; ptr += 2;
					}entry_name[i] = '\0';

					// if transaction occurred on a boundary then back step to open folder correctly
					if(violation)
						copy_load_prev_cluster(file_src);



					unsigned int source_cluster = file_src->cluster_nr;
					unsigned int source_addr = file_src->cluster_addr;

					if(entry_type == exFOLDER){

						c_printf("\r[B][b]Creating Copy %s", &entry_name[0]);


						file_dest->cluster_nr = copy_cluster;
						file_dest->cluster_addr = copy_cluster_addr;

						// do we need to create the destination folder???
						unsigned int destination_folder_cluster = search_directory(file_dest,  &entry_name[0]);

						if(destination_folder_cluster == 0)
							destination_folder_cluster = exfat_create_general_entry_with_length(file_dest, &entry_name[0], exFOLDER, 0, 0, 0);



						// Open Source Folder
						file_src->cluster_nr = source_cluster;
						file_src->cluster_addr = source_addr;

						// open source folder and prepare to copy it
						exfat_open_folder(file_src, &entry_name[0]);

						// open source folder and prepare to copy it - note update file size will
						// use the destination folder to update. Because we will not be updating
						// the copy folder but the destination folder
						tree_insert_cluster(file_dest, &entry_name[0], copy_cluster);

						/** Recursion: In programming terms, a recursive function can be defined as a routine that calls
						  * itself directly or indirectly. Using the recursive algorithm, certain problems can be solved
						  * quite easily. Towers of Hanoi (TOH) is one such programming exercise. Try to write an
						  * iterative algorithm for TOH. Moreover, every recursive program can be written using iterative
						  * methods.
						  */
						copy_folder(file_src, file_dest, destination_folder_cluster, data_size);

						// and return from both folders
						tree_remove_cluster(file_dest);
						exfat_return(file_src);
					}

					// else the entry must be file
					else{

						// Create file in Destination folder
						c_printf("\r[B][b]Creating Copy %s", &entry_name[0]);

						file_dest->cluster_nr = copy_cluster;
						file_dest->cluster_addr = copy_cluster_addr;


						// do we need to create the destination file???
						unsigned int destination_file_cluster = search_directory(file_dest,  &entry_name[0]);

						if(destination_file_cluster == 0)
							destination_file_cluster = exfat_create_general_entry_with_length(file_dest, &entry_name[0], exFILEX, 0, 0, 0);

						// else it's a file with existing data, free data clusters
						else
							free_cluster_chain(file_dest, destination_file_cluster, data_size);

						// copy data from source to destination file
						copy_file_data(file_src, source_file_cluster, file_dest, destination_file_cluster, data_size);
					}

					// Open Source Folder
					file_src->cluster_nr = source_cluster;
					file_src->cluster_addr = source_addr;

					// move to next cluster
					if(violation)
						copy_load_next_cluster(file_src);

				}
			}
		}
		directory++;

		// handle boundary violation!!!
		if(boudary_configure_copy(file_src, cache, (unsigned char **)&directory, (unsigned char **)&directory, &violation, &folder_size) == 0)
			break;

		violation = 0;
	}
	return_to_source(file_src, source);
}

 

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

Found the solution.

 

	unsigned char * cache = (unsigned char *)cache_alloc(cluster_size(file_src) + 32);
	cache = (unsigned char *)(((unsigned int)cache) &~ 0x0000001f); //alignment for 32 bytes

I had to change it to:

 

0x0000001f

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

Lajon wrote:
Replace 15 with 31 (and 0x0f is also a 15 that needs to be replaced)

As I wrote in post #10 (and note it's overkill to add 32 when you are aligning with 32).

/Lars

 

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

actually failed during a test so I've had to unmark the solution.  It works recursively with the cache memory on the stack but there is an error in reading a folder name when say at folder 1004. A folder would come up has fold@ when is should be say folder 1004.  If its allocated from the heap the error disappears but the function fails when making recursive calls.  

 

Has anyone an idea what I could try???

 

HELP ME!!

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

As per other thread, the error (ghost) disappeared when I run the exFAT on my RTOS.