CCS C Software and Maintenance Offers
FAQFAQ   FAQForum Help   FAQOfficial CCS Support   SearchSearch  RegisterRegister 

ProfileProfile   Log in to check your private messagesLog in to check your private messages   Log inLog in 

CCS does not monitor this forum on a regular basis.

Please do not post bug reports on this forum. Send them to support@ccsinfo.com

Problem calling function when passing structure *Workaround*

 
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion
View previous topic :: View next topic  
Author Message
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

Problem calling function when passing structure *Workaround*
PostPosted: Thu Jun 03, 2021 11:06 am     Reply with quote

Hello guys,

I am working with a PIC24FJ256GA106, and CCSC version 5.103, and running into a weird issue with my software.

I have a function that takes uint8_t and a slope typedef structure I defined, which has two doubles, m and b. However, for some reason when I call that function after running my code for a while it doesn't work. The program will just crash and the watchdog timer will reset the PIC.

However, I found that if I break the slope up into two doubles, m and b, and pass them individually instead of as a structure the code works perfectly. Additionally if I call the slope parameter version of the function on startup, it seems to work. Only after the program has been running for a while does it seem to crash. I was hoping you guys would have some insight into what is going on, as I am very confused. I have passed structures before without issue and this structure is very simple. All the relevant code will be listed below.

Here is where I call the two versions of the function from:
Code:

double m = getDouble(&payloadStartAddress);       
double b = getDouble(&payloadStartAddress);
   
slope s = {0, 0};       
s.m = m;       
s.b = b;
       
printf("Connector: %d  Slope: %0.5f  Intercept: %0.5f\r\n", connector, s.m, s.b);       
delay_ms(1000);
       
//realsetslope       
setCalibration(connector, s);                   //doesn't work       
// setCalibration(connector, s.m, s.b);     //works


Here are the functions I'm calling:
Code:

// void setCalibration(uint8_t connector, double m, double b)       //works
void setCalibration(uint8_t connector, slope s)                           //doesn't work
{   
    printf("Here...\r\n");   
    delay_ms(1000);

    //if(!connectorValid(connector)) return;  //this function is working but commented
   
    printf("Valid...\r\n");   
    delay_ms(1000);
   
    printf("Setting J%d -> m:%0.5f  b:%0.5f\r\n", connector, s.m, s.b);   
    // printf("Setting J%d -> m:%0.5f  b:%0.5f\r\n", connector, m, b);       
   
    delay_ms(1000);
}

Finally here is the code that works, no matter which function I use. This is called early on in the programs initializing stage:
Code:

//fakesetslope   
slope s = {0.123123, 0.4635344};   
uint8_t connector = 2;   
setCalibration(connector, s);                             //works
//setCalibration(2, 0.123123, 0.4545645);        //works


Please let me know if anyone needs clarification or more info.


Last edited by samikool on Thu Jun 03, 2021 12:49 pm; edited 1 time in total
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

PostPosted: Thu Jun 03, 2021 11:57 am     Reply with quote

Interesting update.

I found that changing the structure of a slope, can "fix" the issue.

The original slope structure is as follows

Code:

typedef struct {
    double m;
    double b;
} slope;


For testing I changed it to two integers as shown below.

Code:

typedef struct{
    int m;
    int b;
} slope;


This worked without issue, so thinking it could be something to do with structure size I changed it to two int64 instead as follows.

Code:

typedef struct{
    int64_t m;
    int64_t b;
} slope;


It stopped working. I then changed the original structure to four floats, so again it was different datatypes but a total of 16 bytes

Code:

typedef struct {
    float m;
    float b;
    float a;
    float c;
} slope;


This also did not work. The conclusion I'm drawing is that for some reason when the program gets to this point in my code, and I try to pass 16 bytes packed as a single structure the PIC locks up. If I am under 16 bytes in my single structure, or I pass the two doubles separately (2x8 byte parameters) things work. I have emailed CCS, but I'm leaning towards this being some sort of compiler bug, unless anyone can thing of something I'm doing wrong.
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Thu Jun 03, 2021 12:07 pm     Reply with quote

I'd actually try reversing the variables in the function declaration.
Problem is having an int_8, then a structure. The structure needs to be
word aligned. Now the compiler should ensure this is the case, but I'm
always careful and pass things 'largest type first'. Similarly in structures
themselves.
It seems to prevent some oddities...
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

PostPosted: Thu Jun 03, 2021 12:32 pm     Reply with quote

Quote:

I'd actually try reversing the variables in the function declaration.


I can confirm that this works.

So then this is definitely something in the compiler. I will add this to the email thread I have with CCS and hopefully they can find a fix for this.

As of now though, thank you for the solution!
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Fri Jun 04, 2021 1:16 am     Reply with quote

What you can do is explicitly add the aligned keyword.

__attribute__((aligned(0x2))

to the structure double declarations.

In the compiler help, it specifically says:
Quote:

By default the compiler will allocate a variable in the first free memory location.


Now the problem with this is that if you have something that requires
'word' alignment, following something that uses byte alignment, you can
end up with word memory fetches being attempted to on odd memory
location. Result should actually be an exception error. However it sounds as
if instead it is handling the access incorrectly... Sad

It is interesting that standard int16's, it does correctly word align. It
seems to be the larger types where it is failing to do this.

I remember finding a while ago, that you had to be 'careful' in structure
allocation, or it could give incorrect alignments, so have taken to explicitly
using 'largest first' order, or adding alignment declarations.
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

PostPosted: Mon Jun 07, 2021 8:11 am     Reply with quote

On emailing with CCS they told me to include pcd_traps.c which I had not used before but indeed it seems to give a memory access violation.

Error:
Code:

ADDRESS FAULT PC:73A2 W0:DE3 W1:E12 W2:750A W3:C030 W4:AB5 W5:0 W6:DC8 W7:0 W8:0 W9:0 W10:0 W11:0 W12:0 W13:0 W14:6000 W15:46F8 PSVPAG:0 TBLPAG:0 INTCON2:0 CORCON:8
STACK (4) 0 8152 0 84FE


The program at 73A2:
Code:

                       ..............................         slope s = {0, 0};
0734E 20DE44         MOV     #DE4,W4        : W4 = DE4
07350 887094         MOV     W4,E12         : E12 = W4
07352 026F74 000000  CALL    6F74           :
07356 B7ADEE         MOV     W0,DEE         : DEE = W0
07358 886F81         MOV     W1,DF0         : DF0 = W1
0735A 886F92         MOV     W2,DF2         : DF2 = W2
0735C 886FA3         MOV     W3,DF4         : DF4 = W3
0735E 20DE44         MOV     #DE4,W4        : W4 = DE4
07360 887094         MOV     W4,E12         : E12 = W4
07362 026F74 000000  CALL    6F74           :
07366 B7ADF6         MOV     W0,DF6         : DF6 = W0
07368 886FC1         MOV     W1,DF8         : DF8 = W1
0736A 886FD2         MOV     W2,DFA         : DFA = W2
0736C 886FE3         MOV     W3,DFC         : DFC = W3
0736E EF2DFE         CLR     DFE            : DFE = 0
07370 EF2E00         CLR     E00            : E00 = 0
07372 EF2E02         CLR     E02            : E02 = 0
07374 EF2E04         CLR     E04            : E04 = 0
07376 EF2E06         CLR     E06            : E06 = 0
07378 EF2E08         CLR     E08            : E08 = 0
0737A EF2E0A         CLR     E0A            : E0A = 0
0737C EF2E0C         CLR     E0C            : E0C = 0
                         ..............................         s.m = m;
0737E F80DEE         PUSH    DEE            : PUSH DEE to TOS
07380 F90DFE         POP     DFE            : POP TOS to DFE
07382 F80DF0         PUSH    DF0            : PUSH DF0 to TOS
07384 F90E00         POP     E00            : POP TOS to E00
07386 F80DF2         PUSH    DF2            : PUSH DF2 to TOS
07388 F90E02         POP     E02            : POP TOS to E02
0738A F80DF4         PUSH    DF4            : PUSH DF4 to TOS
0738C F90E04         POP     E04            : POP TOS to E04
                         ..............................         s.b = b;
0738E F80DF6         PUSH    DF6            : PUSH DF6 to TOS
07390 F90E06         POP     E06            : POP TOS to E06
07392 F80DF8         PUSH    DF8            : PUSH DF8 to TOS
07394 F90E08         POP     E08            : POP TOS to E08
07396 F80DFA         PUSH    DFA            : PUSH DFA to TOS
07398 F90E0A         POP     E0A            : POP TOS to E0A
0739A F80DFC         PUSH    DFC            : PUSH DFC to TOS
0739C F90E0C         POP     E0C            : POP TOS to E0C
                         ..............................
                         ..............................         setCalibration(connector, s);    //doesn't work (v5.104) - hoping ccs will fix so leaving here
0739E 20DE30         MOV     #DE3,W0        : W0 = DE3
073A0 20E121         MOV     #E12,W1        : W1 = E12
073A2 09000F         REPEAT  #F             : Repeat next instruction (F + 1) times
073A4 7818B0         MOV     [W0++],[W1++]  : [W1++] = [W0++]
073A6 20DFE0         MOV     #DFE,W0        : W0 = DFE
073A8 20E141         MOV     #E14,W1        : W1 = E14
073AA 09000F         REPEAT  #F             : Repeat next instruction (F + 1) times
073AC 7818B0         MOV     [W0++],[W1++]  : [W1++] = [W0++]


Thank you for the info Ttelmah, you've been very helpful. I'll try to keep the thread updated as I talk with CCS and hopefully there can be some sort of permanent fix, rather than having to order parameter based on data size.
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Mon Jun 07, 2021 11:15 am     Reply with quote

An address error trap is what I'd have expected. The alignment
declaration is the correct fix really. However what is odd is that the
compiler does automatically align int16's, but is not doing so for the
larger types. That is really a compiler fault. It ought to word align
all things unless you specifically use the packed declaration... Sad

Looking at the assembler, you can see it is doing a 16bit transfer from
an odd address (#DE3).
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Tue Jun 08, 2021 12:35 am     Reply with quote

OK. Have done some playing.

If you declare a structure, it correctly 'word aligns', whatever you do.
The thing that goes wrong is very specifically passing a set of values to
a function, where there is a structure requiring word alignment, following
something that is an int8. However by default this seems to be handed
correctly.

The following things all fix this:
Code:

void setCalibration(uint16_t connector, slope s)  //make sure the first
//thing is 'word sized.

void setCalibration(slope s, uint8_t connector)   //put the non word sized
//thing after the structure

typedef struct __attribute__((aligned(2))) {
    double m;
    double b;
} slope;
//Force the 'slope' data type to be word aligned


You can very easily 'see' if the problem is going to happen, by looking at
the symbol table, and at where 'setCalibration.s' is actually located.
If this is on an odd address there is going to be a problem.

Now it is interesting, since a basic test, has this on an even address. I tried
dozens of configurations and all correctly gave even addresses. The only
way I could actually make it go wrong, was by explicitly using
the packed attribute on the structure.

So the compiler does seem to be correctly respecting the alignment rules.
Something in the code must be triggering it to permit this misalignment.
Have no idea 'what'!...

Do you want to post the whole of your processor 'setup' (fuses, #OPT,
etc. etc.). and what standard libraries you are using (obviously stdint.h etc.)
something is changing the default behaviour of the compiler in this regard.
Trying to work out what it is.
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

PostPosted: Tue Jun 08, 2021 7:54 am     Reply with quote

Oh wow very interesting find.

Right now our main.h doesn't seem too crazy:

Code:

#ifndef MAIN_H
#define MAIN_H

#case

/******************************************************************************
 *                               Include Files
 *****************************************************************************/
#include <24FJ256GA106.h>


/******************************************************************************
 *                                 Constants
 *****************************************************************************/
#define CODE_PROTECTION_ON      0
#define DEBUGGER_ON             1 // Set both debugger #defines to use printf ().
#define DEBUGGER_WITH_PRINTF    1 // Set both debugger #defines to use printf ().

/*---------------------------------------------------------------------------*/
// #device
/*---------------------------------------------------------------------------*/
// Sets the debugger to ON
#if DEBUGGER_ON == 1
    // Allows DEBUGGER
    #device ICSP=TRUE
    #device ICD=3
#endif

/*---------------------------------------------------------------------------*/
// #use
/*---------------------------------------------------------------------------*/

#use delay(clock=32MHz,crystal=8MHz)


// Allows both the debugger and the printf ()
#if DEBUGGER_WITH_PRINTF == 1
    // Turn on the RS-232 functionality.
    //#use rs232 (ICD,XMIT=PIN_B4,RCV=PIN_B5)         // ICD-U80 console

    //#use rs232 (BAUD=115200,XMIT=PIN_G8,RCV=PIN_G7) // TP pins.
    // Hardware UART1 using TP pins.
    #pin_select U1TX=PIN_G8
    #pin_select U1RX=PIN_G7
    #use rs232(UART1, baud=115200, stream=TP)
    // Or, for ICD Debugger I/O:
    //#use rs232(ICD, stream=TP)


    #define DEBUG_PRINTF printf
    #define DEBUG_PRINT printf
#else
    #define DEBUG_PRINTF(...)
#endif

// SPI
#pin_select SCK1OUT=PIN_D4
#pin_select SDI1=PIN_D5
#pin_select SDO1=PIN_D3
#use spi(MASTER, SPI1, MODE=0, BITS=8, stream=SPI1_MODE0, BAUD=2000000)
#use spi(MASTER, SPI1, MODE=1, BITS=8, stream=SPI1_MODE1, BAUD=2000000)

// SPI2
#pin_select SCK2OUT=PIN_B1
#pin_select SDI2=PIN_B2
#pin_select SDO2=PIN_B0
#use spi(MASTER, SPI2, MODE=0, BITS=8, stream=SPI2_MODE0, BAUD=2000000)
#use spi(MASTER, SPI2, MODE=1, BITS=8, stream=SPI2_MODE1, BAUD=2000000)

#WORD PMCON = getenv("PMCON")
#bit PMPEN = PMCON.15

/*---------------------------------------------------------------------------*/
// #FUSES
/*---------------------------------------------------------------------------*/

#FUSES NOWDT                     //No Watch Dog Timer
#FUSES CKSFSM                    //Clock Switching is enabled, fail Safe clock monitor is enabled
#FUSES NOIESO                    //wait for PLL start, to prevent changing SPI baud rate
#FUSES NOPROTECT                // Disable code protection.





#endif   /* MAIN_H */


I can double check but I do believe this is the only place we set/use fuses.

Library wise we use
Code:

#include <stdint.h>
#include <stdlib.h> // atoi ()
#include <stdlibm.h>
#include <stddef.h>     // size_t
#include <stdbool.h>    // bool
#include "pcd_traps.c"
#include "socket.c"


Otherwise everything else is our own libraries we've written for the project.

Hope that's helpful.
samikool



Joined: 01 Apr 2021
Posts: 15

View user's profile Send private message

PostPosted: Tue Jun 08, 2021 8:06 am     Reply with quote

Another very interesting find.

After finding and solving some other bugs in the program, I reran the old structure set up and the fault changed. This time I got an oscillator fault.

Code:
OSCILLATOR FAULT PC:108AA W0:DE3 W1:E12 W2:31B0 W3:C033 W4:AB5 W5:0 W6:DC8 W7:0 W8:0 W9:0 W10:0 W11:0 W12:0 W13:0 W14:6000 W15:46D8 PSVPAG:0 TBLPAG:0 INTCON2:0 CORCON:8
STACK (4) 1 15CC 0 458C


The asm at that location seems to be the same line of that caused the previous crash, just a different reason.

Code:
                         ..............................         s.m = m;
10886 F80DEE         PUSH    DEE            : PUSH DEE to TOS
10888 F90DFE         POP     DFE            : POP TOS to DFE
1088A F80DF0         PUSH    DF0            : PUSH DF0 to TOS
1088C F90E00         POP     E00            : POP TOS to E00
1088E F80DF2         PUSH    DF2            : PUSH DF2 to TOS
10890 F90E02         POP     E02            : POP TOS to E02
10892 F80DF4         PUSH    DF4            : PUSH DF4 to TOS
10894 F90E04         POP     E04            : POP TOS to E04
                         ..............................         s.b = b;
10896 F80DF6         PUSH    DF6            : PUSH DF6 to TOS
10898 F90E06         POP     E06            : POP TOS to E06
1089A F80DF8         PUSH    DF8            : PUSH DF8 to TOS
1089C F90E08         POP     E08            : POP TOS to E08
1089E F80DFA         PUSH    DFA            : PUSH DFA to TOS
108A0 F90E0A         POP     E0A            : POP TOS to E0A
108A2 F80DFC         PUSH    DFC            : PUSH DFC to TOS
108A4 F90E0C         POP     E0C            : POP TOS to E0C
                         ..............................
                         ..............................         setCalibration(connector, s);    //doesn't work (v5.104) - hoping ccs will fix so leaving here
108A6 20DE30         MOV     #DE3,W0        : W0 = DE3
108A8 20E121         MOV     #E12,W1        : W1 = E12
108AA 09000F         REPEAT  #F             : Repeat next instruction (F + 1) times
108AC 7818B0         MOV     [W0++],[W1++]  : [W1++] = [W0++]
108AE 20DFE0         MOV     #DFE,W0        : W0 = DFE
108B0 20E141         MOV     #E14,W1        : W1 = E14
108B2 09000F         REPEAT  #F             : Repeat next instruction (F + 1) times
108B4 7818B0         MOV     [W0++],[W1++]  : [W1++] = [W0++]
108B6 02FEBA 000000  CALL    FEBA           :
jeremiah



Joined: 20 Jul 2010
Posts: 1314

View user's profile Send private message

PostPosted: Wed Jun 09, 2021 12:33 pm     Reply with quote

Do you put your chip to sleep per chance? I've run into some PIC24 chips that have a chip errata where putting the chip to sleep can cause various TRAP conflicts (OSC, ADDR, etc). I don't know if this is one of them, but you might check the chip errata document for your chip on the Microchip website.
Ttelmah



Joined: 11 Mar 2010
Posts: 19195

View user's profile Send private message

PostPosted: Wed Jun 09, 2021 11:35 pm     Reply with quote

Good suggestion.
A few things that might cause issues:

#pin_select SCK1OUT=PIN_D4

As a general comment, you should always map the SCK1IN as well (and
the same for SCK2). Some of the chips have the input clock path separate
from the output clock path, and not mapping both can cause erratic behaviour. So add:

#pin_select SCK1IN=PIN_D4

and a similar entry for SCK2.

Now Jeremiah's question about if you do use sleep is important. If you do,
you should change the clock to a non PLL based source before switching.
The chip has issues with restarting the PLL based clocks from sleep.
Also add a delay_cycles(1) as the instruction before the sleep. Again
there is an issue if any read instruction occur before the sleep.

The chip also has issues with restarting from a brownout.

Is it possible you use RB5 for anything needing open collector operation?.
I2C etc.. If so there is a hardware problem with this pin...

Are you running in the debugger mode?. If so, there is a really nasty
erratum about the UART. Could you switch to using UART2 instead of
UART1?.

I don't see a PSV setup?. If you are using PSV, there is an erratum on
this that can result in a false address trap.

Now I notice this:
Quote:

33.Module:Oscillator (Two-Speed Start-up)
Two-Speed Start-up is not functional. Leaving the
IESO Configuration bit in its default state
(Two-Speed Start-up enabled) may result in
unpredictable operation.
Work around
None. Always program the IESO Configuration bit
to disable this feature (CW2[15] = 0).


Check the fuses actually being generated by the compiler. Two speed
startup is the default on a lot of chips, and if this is being programmed
to the default, this could cause "unpredictable operation". Sounds rather
an 'ouch' one to me!....
The end of the .lst file shows the fuses.
Display posts from previous:   
Post new topic   Reply to topic    CCS Forum Index -> General CCS C Discussion All times are GMT - 6 Hours
Page 1 of 1

 
Jump to:  
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum


Powered by phpBB © 2001, 2005 phpBB Group