Char* in struct not being written to RAM and hard faults when pointer tries to change value

[dboland]

I am encountering an issue where a hard fault occurs when the "char * pszCommand" variable is written to because it is trying to change data that is not in the RAM.

Here is the related code that gives me the hard fault:

typedef struct
{
	char * pszCommand; // command string
	void (* pfnHandler)(Parsed_t * ParsedCmd); // handler function
	int iParent; // index of command one level up in tree (if any)
	int nRequiredLength; // number of significant chars in command
	int nTreeLevel; // this commands depth in menu tree
	unsigned short bBranchEnd:1; // set if this command is the end of branch
	unsigned short bQueryAllowed: 1; // set if this command accepts queries
} CmdTreeEntry_t;


CmdTreeEntry_t * pTreeEntry;
// initialize the root entry
pTreeEntry = &g_CmdTree[0];
*pTreeEntry->pszCommand = ITEM_SEPARATOR; <------ The hard fault occurs after execution of this line
pTreeEntry->iParent = -1;
pTreeEntry->nRequiredLength = 1;
pTreeEntry->nTreeLevel = 1;
pTreeEntry->bBranchEnd = FALSE;
pTreeEntry->bQueryAllowed = FALSE;

CmdTreeEntry_t g_CmdTree[] =
{
 /* command string, handler */
 { " ", NULL }, /* MUST BE FIRST ENTRY IN TABLE ! */

 { ":*CAL??", calibration_query},
 { ":*CLS", clear_status_cmd},
 .... (A bunch of commands)

 { NULL, NULL} /* MUST BE LAST ENTRY IN TABLE!!! */
};

I believe this is somehow related to the linker file not allocating the memory properly for the pszCommand pointer, but I am unable to find where the issue may be in the linker. 
Here is the linker file:

/* Entry Point */
ENTRY(Reset_Handler)

/* Highest address of the user mode stack */
_estack = ORIGIN(RAM) + LENGTH(RAM); /* end of "RAM" Ram type memory */

_Min_Heap_Size = 0x200; /* required amount of heap */
_Min_Stack_Size = 0x400; /* required amount of stack */

/* Memories definition */
MEMORY
{
 RAM (xrw) : ORIGIN = 0x20000000, LENGTH = 64K
 FLASH (rx) : ORIGIN = 0x8000000, LENGTH = 512K
}

/* Sections */
SECTIONS
{
 /* The startup code into "FLASH" Rom type memory */
 .isr_vector :
 {
 . = ALIGN(4);
 KEEP(*(.isr_vector)) /* Startup code */
 . = ALIGN(4);
 } >FLASH

 /* The program code and other data into "FLASH" Rom type memory */
 .text :
 {
 . = ALIGN(4);
 *(.text) /* .text sections (code) */
 *(.text*) /* .text* sections (code) */
 *(.glue_7) /* glue arm to thumb code */
 *(.glue_7t) /* glue thumb to arm code */
 *(.eh_frame)

 KEEP (*(.init))
 KEEP (*(.fini))

 . = ALIGN(4);
 _etext = .; /* define a global symbols at end of code */
 } >FLASH

 /* Constant data into "FLASH" Rom type memory */
 .rodata :
 {
 . = ALIGN(4);
 *(.rodata) /* .rodata sections (constants, strings, etc.) */
 *(.rodata*) /* .rodata* sections (constants, strings, etc.) */
 . = ALIGN(4);
 } >FLASH

 .ARM.extab : {
 . = ALIGN(4);
 *(.ARM.extab* .gnu.linkonce.armextab.*)
 . = ALIGN(4);
 } >FLASH

 .ARM : {
 . = ALIGN(4);
 __exidx_start = .;
 *(.ARM.exidx*)
 __exidx_end = .;
 . = ALIGN(4);
 } >FLASH

 .preinit_array :
 {
 . = ALIGN(4);
 PROVIDE_HIDDEN (__preinit_array_start = .);
 KEEP (*(.preinit_array*))
 PROVIDE_HIDDEN (__preinit_array_end = .);
 . = ALIGN(4);
 } >FLASH

 .init_array :
 {
 . = ALIGN(4);
 PROVIDE_HIDDEN (__init_array_start = .);
 KEEP (*(SORT(.init_array.*)))
 KEEP (*(.init_array*))
 PROVIDE_HIDDEN (__init_array_end = .);
 . = ALIGN(4);
 } >FLASH

 .fini_array :
 {
 . = ALIGN(4);
 PROVIDE_HIDDEN (__fini_array_start = .);
 KEEP (*(SORT(.fini_array.*)))
 KEEP (*(.fini_array*))
 PROVIDE_HIDDEN (__fini_array_end = .);
 . = ALIGN(4);
 } >FLASH

 /* Used by the startup to initialize data */
 _sidata = LOADADDR(.data);

 /* Initialized data sections into "RAM" Ram type memory */
 .data :
 {
 . = ALIGN(4);
 _sdata = .; /* create a global symbol at data start */
 *(.data) /* .data sections */
 *(.data*) /* .data* sections */
 *(.RamFunc) /* .RamFunc sections */
 *(.RamFunc*) /* .RamFunc* sections */

 . = ALIGN(4);
 _edata = .; /* define a global symbol at data end */

 } >RAM AT> FLASH

 /* Uninitialized data section into "RAM" Ram type memory */
 . = ALIGN(4);
 .bss :
 {
 /* This is used by the startup in order to initialize the .bss section */
 _sbss = .; /* define a global symbol at bss start */
 __bss_start__ = _sbss;
 *(.bss)
 *(.bss*)
 *(COMMON)

 . = ALIGN(4);
 _ebss = .; /* define a global symbol at bss end */
 __bss_end__ = _ebss;
 } >RAM

 /* User_heap_stack section, used to check that there is enough "RAM" Ram type memory left */
 ._user_heap_stack :
 {
 . = ALIGN(8);
 PROVIDE ( end = . );
 PROVIDE ( _end = . );
 . = . + _Min_Heap_Size;
 . = . + _Min_Stack_Size;
 . = ALIGN(8);
 } >RAM

 /* Remove information from the compiler libraries */
 /DISCARD/ :
 {
 libc.a ( * )
 libm.a ( * )
 libgcc.a ( * )
 }

 .ARM.attributes 0 : { *(.ARM.attributes) }
}

This is the memory address of pTreeEntry that is being placed in RAM but the address of pszCommand is not in RAM.

Any help would be appreciated.

Thanks,

Dan

[KnarfB]

The line

*pTreeEntry->pszCommand = ITEM_SEPARATOR; 

should give you at least a warning, which you should not ignore. The * in front is wrong. Note that string literals are

purposely stored in Flash, not RAM.

hth

KnarfB

[dboland]

For some reason, the inverse gives me a warning. There is no warning when it compiles with the * in front but when I remove it I get this warning:

 

[KnarfB]

We don't see your definition of ITEM_SEPARATOR, but the name pszCommand suggests, it should be a string like "XYZ". Then,

pTreeEntry->pszCommand = "XYZ";

is valid, but

*pTreeEntry->pszCommand = "XYZ";

is not (warning: assignment to 'char' from 'char *' makes integer from pointer without a cast).

Keep in mind, that pszCommand is only a pointer, pointing to some memory/string, but not an array containing a string. So the strcpy below is dangerous and may fail if the string pszCommand points to is in the Flash.

hth

KnarfB

[dboland]

Here is the definition of ITEM_SEPARATOR:

// separator between items in a command
#define ITEM_SEPARATOR ':'

I see what you are saying now about the * in front of pTreeEntry when I replace ITEM_SEPARATOR with ":" I get the same warning. 

Do you know a better way to accomplish the strcpy that is being attempted here? Sorry I am not super well versed in C and this code was written by someone around 25-30 years ago.

[dboland]

If I run it without the * in front of pTreeEntry the variable gets the correct value as well as a bunch of other repeating stuff.

However, the next time pTreeEntry->pszCommand is written to I get a hard fault again.

// remove leading ':' chars from string
if( iChar )
	strcpy( pTreeEntry->pszCommand, pTreeEntry->pszCommand + iChar );

 

[Pavel A.]

Please debug your program; try to understand why *pTreeEntry->pszCommand does not point to RAM.

If you have not used the debugger yet, this is a perfect opportunity to get know the debugger.

By the way, more clear (less confusing) syntax would be:

pTreeEntry->pszCommand[0] = ITEM_SEPARATOR;

 

 

[dboland]

I have been using the debugger but without any success. I do not understand why it is pointing to ROM and not RAM, because the g_CmdTree has the pszCommand initialized in RAM but when pTreeEntry is set to the address of g_CmdTree[0] it is only seeing the ROM address.

 

[Pavel A.]

Ok so you are already on the right way. Solving issues like this may be a exciting experience, take your time and enjoy.

[Tesla DeLorean]

Use

#define ITEM_SEPARATOR ":" // USE STRING NOT CHAR BRACING

pTreeEntry->pszCommand = ITEM_SEPARATOR; // CHANGE THE POINTER TO THE NEW STRING

pTreeEntry and the pointer are in RAM, they can POINT to something in FLASH which is static.

pTreeEntry->pszCommand = strmalloc(newstring);

[Tesla DeLorean]

Or make a structure which allocates space? And strcpy() or strcat() into that given sufficient space.

typedef struct
{
	char Command[16]; // command string
	void (* pfnHandler)(Parsed_t * ParsedCmd); // handler function
	int iParent; // index of command one level up in tree (if any)
	int nRequiredLength; // number of significant chars in command
	int nTreeLevel; // this commands depth in menu tree
	unsigned short bBranchEnd:1; // set if this command is the end of branch
	unsigned short bQueryAllowed: 1; // set if this command accepts queries
} CmdTreeEntry_t;

Pointers could be used with malloc/free type use of dynamic memory, or from a pool, but got to watch how you free up resources, and not leak, etc.

You can create systems that don't fragment, but you need to release everything you allocate

[Pavel A.]

Maybe you're confusing char vs string constants -   ':' vs ":"

The first one is a char, the 2nd is a pointer (maybe even points to read-only memory).

Of course one cannot assign a pointer to element of a char array.

[dboland]

I found a solution that works, thanks to a coworker of mine that had some helpful insight. I created a new function that will allocate RAM space for the character and copy it from ROM.

static char * ROMtoRAM(char pszCommandContents[]){
	size_t len = strlen(pszCommandContents) + 1;
	char * target = malloc(len);
	if (target == NULL) return NULL;
	memcpy(target, pszCommandContents, len);
	return target;
}

 With this function I can change where pTreeEntry->pszCommand is being pointed to and can actually write to the allocated memory.

pTreeEntry->pszCommand = ROMtoRAM(pTreeEntry->pszCommand);
*pTreeEntry->pszCommand = ITEM_SEPARATOR;

I think this is acceptable to use so that it doesn't hard fault.

[Pavel A.]

:face_with_head_bandage: