• State Resolved
  • Locked Locked
  • Replies 11 replies
  • Answers 2 answers
  • Subscribers 54 subscribers
  • Views 1665 views
  • Users 0 members are here
Support feedback
Options
Options
Related

This thread has been locked.

If you have a related question, please click the "Ask a related question" button in the top right corner. The newly created question will be automatically linked to this question.

Compiler/TMS320VC33: Two versions of generated assembly code to loop string. One works, one does not

roy wu
Intellectual 260 points

Part Number: TMS320VC33

Tool/software: TI C/C++ Compiler

Hi:

I meet with a problem of compiler may have bugs for TMS320VC33. The compiler version is 4.70. 

The assembly generated from two version of the function 

security_NercCompliantPW.  It is simply to check whether the password string has one upper, one lower, one number and one special char.
I attach two versions of the code in C and generated assembly files.
One working fine when check the end the of string by '\0'
One fail (result is not correct) when check the end of the string by Length.
The part of the source file and generated asm files are attached
  • 0
    TI__Guru**** 253270 points

    The C30 compiler is no longer supported.  Release 4.70 is over 20 years old.  If we identified the bug, we would not fix it.  We would only tell you how to workaround it.  However, you have already found a workaround.  So, I'm not sure what more is needed.

    Thanks and regards,

    -George

  • 0 in reply to George Mock
    Intellectual 260 points
    Hi George:

    Thanks for the feedback. I understand the toolchain is not supported anymore.

    What I can do is try to confirm whether it is an known bug or in which condition it will happen.

    We have a lot of code still working on the chip and if we can not find the pattern when the bug will appear it is difficult for programmer to know when to use the workaround.

    It will appreciated if you can supply more information.

    Many Thanks.

    Roy Wu
  • 0 in reply to roy wu
    TI__Guru**** 253270 points

    We will try to better characterize the circumstances which cause the bug.  But I need further information from you.

    1. Reduce the test case, if possible
    2. A test case we can build
    3. The build options
    4. What goes wrong when it fails

    It would be ideal if you reduced the test case down so there is only one call to the problem function security_NercCompliantPW.  If you can't do that, then you need to indicate which call is the one that fails.

    We would like to build the test case ourselves.  The code you sent cannot be built because it relies on several types and #define constants which are not supplied.  Two examples are BOOLEAN and MAX_NERC_CHECK_LEN.  Rather than guess at these, it would be better if you preprocessed the code and sent us the output.  Build as before, but add the options -pl -pn.  This creates a file with same name as the base file, but the extension changed to .pp.  This is the preprocessed file we need.  Attach it to your next post.  Because the forum only accepts a few different file types, add the file extension .txt to it.

    I see these build options in the assembly code you sent ...

    ;       U:\Tools\texas\ti\c30\bin\ac30.exe -v31 -q -mr -x -dNDEBUG -dNU_NO_ERROR_CHECKING -dCR_INC_STATUS -dCR_INC_RX_MSTAG -i\Tools\tex
;       U:\Tools\texas\ti\c30\bin\opt30.exe -v31 -q -r -a -f -O2 C:\Users\224474\AppData\Local\Temp\security.if C:\Users\224474\AppData\
;       U:\Tools\texas\ti\c30\bin\cg30.exe -v31 -q -p -a -c -i -a C:\Users\224474\AppData\Local\Temp\security.opt security.asm C:\Users\224474\AppData\Local\Temp\security.tmp

    I presume this exactly matches how you invoke the compiler.  If not, then we need to see the compiler options exactly as the compiler sees them.

    When the code fails, exactly how do you know?  What are you looking at?  What do you expect, and what do you see instead?  If you have any information on where the compiler generated assembly is incorrect, please share that as well.

    Thanks and regards,

    -George

  • 0 in reply to George Mock
    Intellectual 260 points

    George:

    Yes I can narrow down the test cases. This is simple API and itself has no dependency on other parts. security_strstrn & security_NercCompliantPW are all the functions I need verify. I created a unit test API named security_NercCompliantPW_Test. It will be called by the security_InitSecurity.

    I found the following code fail because it failed the unit test , it shall pass the check of one upper, one lower, one number and one special character.

    /*positive case*/
if (!security_NercCompliantPW("ADMINISTRATOR", "Wq1#12345"))
return __LINE__;

    In the .pp file, line 15384, I detect failure of the following lines.

    if ((1 | 2 | 4 | 8) != NercFlags)
{
return FALSE;
}

    The generated pp file are attached.security_fail.asmsecurity_fail.pp.txtsecurity_ok.asmsecurity_ok.pp.txt


  • 0 in reply to roy wu
    TI__Guru**** 253270 points

    I'm not sure I understand.  I think you are saying that when this code executes ...

        if ((1 | 2 | 4 | 8) != NercFlags)
    {
        return FALSE;
    }

    ... the variable NercFlags has all 4 flags set (it is equal to 0xF), yet the return FALSE still executes. The code should not return FALSE, but continue on to the next thing.  Is this a correct description of what you see?

    I need one more detail ... I need to see all the options passed to the compiler shell command cl30, exactly as it occurs when it you build.

    Thanks and regards,

    -George

  • 0 in reply to George Mock
    Intellectual 260 points

    Hi George:

    For the test case,  

    security_NercCompliantPW("ADMINISTRATOR", "Wq1#12345")

    It shall return true.( Mean it is compliant)

    But the NercFlags is not 0x0F (which should be) , so it return FALSE ( in the security_fail file). In the securtiy_fail.asm , it looks the asm code do strange comparison. sub 59 then compare with value. e.g. the first compare is with 7  ( the ASCII of ‘A' is 65 < 7+59).

    ******************************************************
* FUNCTION DEF : _security_NercCompliantPW
******************************************************
_security_NercCompliantPW:
	PUSH	R4
*
* R3	assigned to variable  NercFlags
* AR2	assigned to variable  r1
* IR1	assigned to parameter pUsername
* BK	assigned to parameter pPassword
*
	LDI	AR2,IR1
	LDI	R2,BK
	CMPI	0,IR1
	BZ	L173
	CMPI	0,BK
	BZ	L173
* >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> ENTERING strlen()
	LDI	BK,AR2
	SUBI	1,AR2
L157:
	LDI	*++AR2,R0
	BNZ	L157
* <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< LEAVING strlen()
	SUBI	BK,AR2,R4
	CMPI	8,R4
	BLO	L173
	CMPI	16,R4
	BHI	L173
	LDI	0,R3
	CMPI	0,R4
	BZ	L172
	LDI	59,R0
	SUBI	R0,BK,AR2
L162:
	LDI	*++AR2,R2
	CMPI	7,R2
	BLT	L166
	CMPI	32,R2
	BLE	L170
	CMPI	39,R2
	BLT	L166
	CMPI	64,R2
	BGT	L166
	OR	02h,R3
	B	L171
L166:
	CMPI	-10,R2
	BLT	L168
	CMPI	0,R2
	BGE	L168
	OR	04h,R3
	B	L171
L168:
	CMPI	-25,R2
	BLT	L171
	CMPI	-10,R2
	BLT	LL37
	CMPI	0,R2
	BLT	L171
	CMPI	6,R2
	BLE	LL37
	CMPI	33,R2
	BLT	L171
	CMPI	38,R2
	BGT	L171
LL37:
	OR	08h,R3
	B	L171
L170:
	OR	01h,R3
L171:
	SUBI	1,R4
	BNZ	L162
L172:
	CMPI	15,R3
	BNZ	L173
	LDI	BK,AR2
	LDI	IR1,R2
	LDI	3,R3
	LDI	0,RC
	CALL	_security_strstrn
	CMPI	0,R0
	BZ	L174
L173:
	LDI	0,R0
	B	EPI0_23
L174:
	LDI	1,R0
EPI0_23:
	POP	R4
	RETS

    I will attach the compiler options tomorrow.

  • 0 in reply to roy wu
    Intellectual 260 points

    The whole build process is:

    clearmake: Entering directory `U:\50325\build2\makefiles'
	echo P545 Courier...
	cd ..\model_P545\courier & clearmake -f ..\..\makefiles\makefile.mak MODEL_ID=545 MODEL_TYPE=CURRENT_DIFF VARIANT=COURIER copro main_card
clearmake: Entering directory `U:\50325\build2\model_P545\COURIER'
	echo P545-COURIER: Building Coprocessor binary
	cd ..\copro & clearmake -f ..\..\makefiles\copro.mak coff
clearmake: Entering directory `U:\50325\build2\model_P545\copro'
`coff' is up to date.
clearmake: Leaving directory `U:\50325\build2\model_P545\copro'

	echo P545-COURIER: Building Platform release library
	clearmake -e -f ..\..\makefiles\platform.mak "COPTS="
clearmake: Entering directory `U:\50325\build2\model_P545\COURIER'
	\Tools\texas\ti\c30\bin\cl30.exe -c -v31 -q -mr -ma -mc -mi -mf -o2 -x -k -dNDEBUG -dNU_NO_ERROR_CHECKING -dCR_INC_STATUS -dCR_INC_RX_MSTAG -i\Tools\texas\ti\c30\bin  -i\50300PL\DEV\COM\INC  -i\50300PL\DEV\pl\INC  -i\50300PL\DEV\psl\INC  -i\50300PL\DEV\util\INC  -i\50300PL\DEV\pl\SRC  -i\50300L1\DEV_C33\Commun_CPU3\INC -i\50300L1\DEV_C33\Affichage\INC -i\50300L1\DEV_C33\Boot_CPU3\INC -i\50300L1\DEV_C33\Communication\INC -i\50300L1\dev_c33\communication\src -i\50300L1\DEV_C33\Date\INC -i\50300L1\DEV_C33\Entree_Sortie\INC -i\50300L1\DEV_C33\Nucleus\INC -i\50300L1\DEV_C33\Memory_CPU3\INC  -i\50222\courier\include  -i\50222\modbus\include  -i\50222\iec870\cs103\include  -i\50300PL\DEV\dr\INC -i\50300PL\DEV\dr\SRC  -i\50319\DEV\com\INC  -i\50319\DEV\pl\INC  -i\50319\DEV\pr\INC  -i\50319\DEV\model_P545  -i..\langtext  -i\50300PL\DEV\pl\INC  -i\50319\DEV\pr\SRC  -i\50319\DEV\pl\SRC  -i\50300PR\DEV\COM\INC  -i\50300PR\DEV\pr\INC  -i\50300PR\DEV\pr\SRC  -i\50222\dnp3\Slave-SCL-V3 -i\50222\dnp3\Slave-SCL-V3\tmwscl\utils -dC_HAS_SLNK -dPLATFORM_UI -dPLATFORM_FCUR  -dCURRENT_DIFF_P545  -dINMON_INIT_ACQ_REL -dNDEBUG -dNEW_CPU2 -dPLATFORM_SERIAL_DEBUG -dSECURITY_SERIAL_DEBUG -dPBKDF2_HMAC_TEST -dUI_SERIAL_DEBUG -dCURRENT_DIFF_P545  -dPLATFORM_KBUS   -dPLAT_DR_UNCOMP -dPLAT_DR_UNCOMP  -dPC_CB_48_SAMPS -dDIST_PASS_BY_POINTER -dFPGA_USE_FLASH \50300PL\DEV\pl\SRC\security.c

clearmake: Leaving directory `U:\50325\build2\model_P545\COURIER'
	echo P545-COURIER: Building Platform Protocol release library
	clearmake -e -f ..\..\makefiles\platform_protocol.mak "COPTS="
clearmake: Entering directory `U:\50325\build2\model_P545\COURIER'
`all' is up to date.
clearmake: Leaving directory `U:\50325\build2\model_P545\COURIER'
	echo P545-COURIER: Building Application release version
	clearmake -e -f ..\..\makefiles\application.mak hex "COPTS="
clearmake: Entering directory `U:\50325\build2\model_P545\COURIER'
`hex' is up to date.
clearmake: Leaving directory `U:\50325\build2\model_P545\COURIER'

clearmake: Leaving directory `U:\50325\build2\model_P545\COURIER'

clearmake: Leaving directory `U:\50325\build2\makefiles'

  • 0 in reply to roy wu
    TI__Guru**** 253270 points

    I can give a partial explanation of what happened.  

    Here is the beginning the key loop in the problem function ...

        while (Index < Length)
    {
        /* Check for uppercase character */
        if ( (pPassword[Index] >= 'A') && (pPassword[Index] <= 'Z'))
        {
            NercFlags = NercFlags | NERC_UPPERCASE_MASK;
        }

    I added the build option -s.  The tells the compiler to add comments to the generated assembly code, which make it easier to understand.  Here is the assembly just before the key loop starts ...

    ***  	-----------------------    U$27 = &pPassword[-59];
	LDI	59,R0
	SUBI	R0,BK,AR2
** 15360	-----------------------    L$2 = C$3;
L162:
***	-----------------------g8:
** 15360	-----------------------    if ( (U$28 = *(++U$27)) < 7 ) goto g12;

    The label L162 is the start of the key loop.  Notice the name U$27.  This is a temporary variable name auto-generated by the optimizer pass of the compiler.  This is the pointer to the password string being checked.  Look how it is initialized.  It should be initialized to &pPassword[-1].  That is, one behind the start of the string.  Then the first reference *(++U$27) works as expected.  But instead of -1, -59 is used. 

    I don't know why the optimizer pass of the compiler made this error.  It would very difficult to work out such a deep detail in a compiler so old.  I hope that this much explanation is enough for your purposes.

    Thanks and regards,

    -George

  • 0 in reply to George Mock
    Intellectual 260 points

    Hi George:

    Thank you for clarify the situation. It confirmed the suspect of the compiler issue. And the last question: is this an known bug for this version of the compiler?

    Best Regards

    Jun 

  • 0 in reply to roy wu
    TI__Guru**** 253270 points

    roy wu said:
    is this an known bug for this version of the compiler?

    No.  -George

  • 0 in reply to George Mock
    Intellectual 260 points

    Thanks George, We can close this post.