Program executing a statement that is categorically not in my code

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

Hi all,

 

I'm coming to the end of a large control system application that I've been working on for a couple of months, with a little niggle that I can't seem to work around.

 

To condense the problem to it's most bare bones form, my application contains several arrays containing text strings, for information such as fault codes and button functions, both of which are written out onto the system display. In all areas these work as per expected in the application, with one exception.

 

In a very specific portion of my code, where starting off or resuming a stopped/paused test program, a string of text appears in the wrong datafield on my display. One of the button function fields contains the 0 index string of the fault_strings array, which is "No Fault". At first, I thought I'd just referenced the wrong array in one of the functions, but searched the entire code to no avail. The datafield should display "No Function", and does in every one case bar this.

 

Most interestingly however, is this string is printing onto the screen in BLACK. Nowhere else in the application does this string EVER get printed in anything other than GREEN.

 

To make this line of text appear on the screen, in this color would require one of the following statements:

display_write_string("No Fault", BLACK);
display_write_string(fault_strings[x], BLACK);

However, the only time this string ever get's printed, it's from the following line of code:

display_write_string(fault_strings[0], GREEN);

The function that displays the button function, is the same function called repeatedly throughout the code, whereby it updates data fields on the display marked as out of date, by whatever function caused them to be out of date.

 

At first, I realized that both the 0 index entry in both my fault code string array, and button function string array were actually identical, so I though the compiler must have noticed this, and was the root cause of the problem. Having made them different, the exact problem as above still arises.

 

I'm running through some JTAG debug work now, and will post if anything else interesting comes up, but otherwise - I'm totally lost. How can a line of code, that doesn't exist, be executed?

 

EDIT - I'm using -Os & -g2 optimisation and debug settings.

 

 

This topic has a solution.
Last Edited: Sun. Feb 2, 2020 - 12:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 2

jtw_11 wrote:
How can a line of code, that doesn't exist, be executed?
The short answer is "it isn't".  It only looks that way.

 

You are likely seeing the effects of a bug in your code somewhere.  Probably an uninitialised pointer or other variable, or a buffer overflow, or a stack overflow, or an incorrectly terminated string, or...

 

Unless you post some code for others to examine, there isn't much else that can be offered beyond speculation.

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

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

Ah, I just found the problem debugging over JTAG.

 

I'd added a new button function recently, which had a #define value of 17, but hadn't added a new entry in the array of strings for this new function.

 

Therefore, when trying to print from index 17 of the string (which didn't exist), in reality I was reading from index 0 of the next string stored in data. Problem solved.

 

However, what I find interesting - the gcc compiler likes to warn about all sorts of what-if cases, including things like bit shifting by the upper bound of the type width, but my program knows at compile time that it is possible for the variable which will be passed to a function in which the variable is used as an index to an array, can/will be larger than the max array size.

 

Am I the only one surprised by a lack of compiler warning here? Clearly it's my fault, but still?

 

Is there any way to manually enforce this, such as if I try and pass an index bigger than the max array index, compilation will fail?

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

jtw_11 wrote:
Am I the only one surprised by a lack of compiler warning here?

As  joeymorin  said, we have no idea what the context of "here" is - so it's impossible to say whether it's surprising or not.

 

However, it is very well known that 'C' has no array bounds checking whatsoever - so I really don't think it would come as a surprise.

 

Is there any way to manually enforce this, such as if I try and pass an index bigger than the max array index, compilation will fail?

No.

 

You would have to do run-time bounds checking.

 

EDIT: Maybe use an ASSERT ...

 

See Tip #5 in my signature (below; may not be visible on mobile)

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...
Last Edited: Tue. Jan 28, 2020 - 03:08 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jtw_11 wrote:
Am I the only one surprised by a lack of compiler warning here? Clearly it's my fault, but still?
Sounds like you probably want to use Ada not C.

 

C gives the programmer lots of freedom (which is why it's so popular) but with that freedom comes serious potential danger too. If I write:

void print(char * str) {
    while (*str) {
        UART_putchar(*str++);
    }
}

int main(void) {
    print((char *)rand());
}

nothing in C does any kind of validity checking on the pointer to make sure it is valid. Similarly:

void foo(int * data) {
    data[317] = 1234;
}

int main(void) {
    int array[10];
    foo(array);
}

nothing in C can possibly "know" that the passed pointer does not point to at least a 318 element array. This is like the difference between strcpy() and strncpy(). Sure C compilers these days may give you all kinds of warning about "don't you realise how dangerous strcpy() is, wouldn't it be much safer to use strncpy()?" but they can't "know" when limits are being exceeded.

 

A more complex analyser (Klockwork, QAC, Lint, etc) may well be able to analyze this kind of thing and point out some potential warnings but plain C compiler don't do analysis to that depth.

Last Edited: Tue. Jan 28, 2020 - 03:49 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

awneil wrote:
EDIT: Maybe use an ASSERT ...

https://en.wikibooks.org/wiki/C_Programming/Language_Reference#ISO_C_(C11)

...

  • _Static_assert

...

for compile-time

 

"Dare to be naïve." - Buckminster Fuller

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

and Pascal and Checked C.

Otherwise, a C coding standard and source code review (self then, if possible, peer)

Dynamic analysis can be in addition to linting and static analysis (if one can afford a static analyzer)

 

https://www.e-lab.de/pdf/AVRinf_en.PDF

[page 1, in next to last paragraph]

Optional runtime errorhandling of software stack and string/array checks.

via E-LAB Computers Pascal-scm for Atmel AVR

It’s Time to Use a Safer C | Electronic Design (Checked C)

ARR30-C. Do not form or use out-of-bounds pointers or array subscripts - SEI CERT C Coding Standard - Confluence

[after mid-page]

Automated Detection

[two linters]

Valgrind: Tool Suite - Experimental Tools

[second]

SGCheck

SGCheck is a tool for finding overruns of stack and global arrays. It works by using a heuristic approach derived from an observation about the likely forms of stack and global array accesses.

 

edit :

AVR - Free Pascal wiki via Lazarus Homepage

 

"Dare to be naïve." - Buckminster Fuller

Last Edited: Wed. Feb 5, 2020 - 10:55 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jtw_11 wrote:
Am I the only one surprised by a lack of compiler warning here?
"somehow" in C++?

jtw_11 wrote:
Clearly it's my fault, but still?
Welcome to the shop.

We're on coffee and tea here as due to the earlier Jolt and Red Bull too many of us were ending up in the ER (A&E)

wink

jtw_11 wrote:
Is there any way to manually enforce this, such as if I try and pass an index bigger than the max array index, compilation will fail?
Would a C interpreter catch such via the operating system's kernel?

 

edit : emoji

 

"Dare to be naïve." - Buckminster Fuller

Last Edited: Tue. Jan 28, 2020 - 05:12 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

jtw_11 wrote:
Am I the only one surprised by a lack of compiler warning here? Clearly it's my fault, but still?

I'm sure avr-gcc DOES warn about that.

 

You didn't provide any code, so I made up this:

 

extern void WriteString (const char s[]);

#define STRING_NUMBER 3

char myStr[2][12] = {
		"Hello",
		"World"
};

int main(void)
{
	WriteString(myStr[STRING_NUMBER]);
	return 0;
}

 

nigel@E6420:~/AVRfreaks$ avr-gcc --version
avr-gcc (AVR_8_bit_GNU_Toolchain_3.6.1_1752) 5.4.0

 

nigel@E6420:~/AVRfreaks$ avr-gcc -Os -Wall -Wextra -mmcu=atmega329p -c -o freak.o freak.c

freak.c: In function 'main':
freak.c:12:2: warning: array subscript is above array bounds [-Warray-bounds]
  WriteString(myStr[STRING_NUMBER]);
  ^

 

That's a pretty clear warning for me.

Perhaps my example is too simple / obvious to the compiler.

 

Last Edited: Tue. Jan 28, 2020 - 08:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:
Perhaps my example is too simple / obvious to the compiler.

Probably - because both the definition of the array

char myStr[2][12] = { ...

and the use of if

WriteString(myStr[STRING_NUMBER]);

appear explicitly within the same compilation unit - and with the index in the reference as a literal constant.

 

I suspect none of them is true in the "real" code ...

 

EDIT

 

In fact, even in your example, I bet you'd get no warning if WriteString() tried to access index [12] of the string passed to it ...

 

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...
Last Edited: Tue. Jan 28, 2020 - 08:51 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

"Dare to be naïve." - Buckminster Fuller

Last Edited: Wed. Jan 29, 2020 - 06:45 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 1

gchapman wrote:

for C99 and sub.

Until I moved to C11, I rolled my own:

#define STATIC_ASSERT ( condition,
    message     )   

A static assert mechanism.

Parameters:

  condition  Boolean expression
  message  Message to emit when condition is false

An alternative to _Static_assert specified in C11.

When condition is false, results in the declaration of a type composed of an array of negative size, which results in a compile-time error.

When condition is true the same type is declared but with an array of positive size, so no error occurs.

The name of the array is the unique concatenation of 'STATIC_ASSERT_FAILED___', message, the source line number (via __LINE__), and a unique ID number (via __COUNTER__).

The message parameter must contain only characters valid for a C identifier (0-9,a-z, A-Z, and _). No other special characters, including 'space', may be used. It is not interpreted as a string literal, so it must not be delimited by double-quotes. While message may itself be a macro, its final expansion must satisfy the above constraints.

The typedef is declared with __attribute__ ((__unused__)) to avoid warnings from -Wunused-local-typedefs and errors from -Werror=unused-local-typedefs

Works both at function scope and at file scope.

Example:

 #define LEN 300
 ...
 STATIC_ASSERT(LEN <= 255, LEN_too_big);

This will result in something like the following error message:

 error: size of array 'STATIC_ASSERT_FAILED___LEN_too_big___LINE_91___UID_7'
 is negative

External references:

Definition at line 1546 of file convenience.h.

/// \brief A static assert mechanism
///
/// \param condition Boolean expression
/// \param message   Message to emit when \a condition is false
///
/// An alternative to \c _Static_assert specified in C11.
///
/// When \a condition  is false, results in the declaration  of a type composed
/// of an array of negative size, which results in a compile-time error.
///
/// When \a condition  is true the same  type is declared but with  an array of
/// positive size, so no error occurs.
///
/// The   name   of   the   array   is  the   unique   concatenation   of   '\c
/// STATIC_ASSERT_FAILED___', \a  message, the  source line number  (via <tt><a
/// href="
/// https://gcc.gnu.org/onlinedocs/cpp/Standard-Predefined-Macros.html
/// ">__LINE__</a></tt>), and a unique ID number (via <tt><a href="
/// https://gcc.gnu.org/onlinedocs/cpp/Common-Predefined-Macros.html
/// "> __COUNTER__</a></tt>).
///
/// The  \a message  parameter  must  contain only  characters  valid  for a  C
/// identifier (0-9,a-z,  A-Z, and _).  No other special  characters, including
/// 'space', may  be used.  It  is not interpreted as  a string literal,  so it
/// must not be  delimited by double-quotes.  While \a message  may itself be a
/// macro, its final expansion must satisfy the above constraints.
///
/// The   typedef  is   declared  with   \c  __attribute__   ((__unused__))  to
/// avoid  warnings   from  \c  -Wunused-local-typedefs  and   errors  from  \c
/// -Werror=unused-local-typedefs
///
/// Works both at function scope and at file scope.
///
/// \b Example:
///
/// \par
/// \code
/// #define LEN 300
/// ...
/// STATIC_ASSERT(LEN <= 255, LEN_too_big);
/// \endcode
///
/// \par
/// This will result in something like the following error message:
///
/// \par
/// \code
/// error: size of array 'STATIC_ASSERT_FAILED___LEN_too_big___LINE_91___UID_7'
/// is negative
/// \endcode
///
/// \par External references:
///   - http://stackoverflow.com/questions/3385515/static-assert-in-c
///
/// \hideinitializer
///
#ifdef __DOXYGEN__
  #define STATIC_ASSERT(condition, message)
#else
  #ifndef STATIC_ASSERT
    #define STATIC_ASSERT(condition, message)               \
      typedef char                                          \
      CAT(                                                  \
          CAT(                                              \
              CAT(                                          \
                  CAT(                                      \
                      CAT(STATIC_ASSERT_FAILED___,          \
                          message),                         \
                      ___LINE_),                            \
                  __LINE__),                                \
              ___UID_),                                     \
          __COUNTER__)                                      \
        [2*(!!(condition))-1] __attribute__ ((__unused__))
  #else
    #error MACRO COLLISION. Macro 'STATIC_ASSERT' is already defined.
  #endif
#endif
/// \brief Concatenate the parameters \a a and \a b to form one token.
///
/// If \a a or \a b are macros they are expanded first before concatenation.
///
/// \b Example:
///
/// \par
/// \code
/// #define FLAGS_REG 0
/// #define ERROR_FLAG 3
/// ...
///   CONCATENATE(GPIOR, FLAGS_REG) |= (1<<ERROR_FLAG);
/// \endcode
///
/// \par
/// This will expand to:
///
/// \par
/// \code
///   GPIOR0 = (1<<3);
/// \endcode
///
/// \par
/// Which will of course be further expanded as per the definition of GPIOR0.
///
/// \par External references:
///   - https://gcc.gnu.org/onlinedocs/cpp/Concatenation.html#Concatenation
///   - http://stackoverflow.com/questions/1489932
///
/// \hideinitializer
///
#ifdef __DOXYGEN__
  #define CONCATENATE(a, b)
#else
  #ifndef CONCATENATE
    #define CONCATENATE(a, b) CONCATENATE_(a, b)
    #ifndef CONCATENATE_
      #define CONCATENATE_(a, b) a ## b
    #else
      #error MACRO COLLISION. Macro 'CONCATENATE_' is already defined.
    #endif
  #else
    #error MACRO COLLISION. Macro 'CONCATENATE' is already defined.
  #endif
#endif

/// \brief Alias for macro #CONCATENATE().
///
/// \hideinitializer
///
#ifdef __DOXYGEN__
  #define CAT
#else
  #ifndef CAT
    #define CAT CONCATENATE
  #else
    #error MACRO COLLISION. Macro 'CAT' is already defined.
  #endif
#endif

 

"Experience is what enables you to recognise a mistake the second time you make it."

"Good judgement comes from experience.  Experience comes from bad judgement."

"Wisdom is always wont to arrive late, and to be a little approximate on first possession."

"When you hear hoofbeats, think horses, not unicorns."

"Fast.  Cheap.  Good.  Pick two."

"We see a lot of arses on handlebars around here." - [J Ekdahl]

 

Last Edited: Wed. Jan 29, 2020 - 04:43 PM
  • 1
  • 2
  • 3
  • 4
  • 5
Total votes: 0

N.Winterbottom wrote:

jtw_11 wrote:
Am I the only one surprised by a lack of compiler warning here? Clearly it's my fault, but still?

I'm sure avr-gcc DOES warn about that.

 

You didn't provide any code, so I made up this:

 

extern void WriteString (const char s[]);

#define STRING_NUMBER 3

char myStr[2][12] = {
		"Hello",
		"World"
};

int main(void)
{
	WriteString(myStr[STRING_NUMBER]);
	return 0;
}

 

nigel@E6420:~/AVRfreaks$ avr-gcc --version
avr-gcc (AVR_8_bit_GNU_Toolchain_3.6.1_1752) 5.4.0

 

nigel@E6420:~/AVRfreaks$ avr-gcc -Os -Wall -Wextra -mmcu=atmega329p -c -o freak.o freak.c

freak.c: In function 'main':
freak.c:12:2: warning: array subscript is above array bounds [-Warray-bounds]
  WriteString(myStr[STRING_NUMBER]);
  ^

 

That's a pretty clear warning for me.

Perhaps my example is too simple / obvious to the compiler.

 

 

Hi,

 

Interesting... such an example throws a warning for me too, however in the full application it does not. The way in which the array in question is indexed is as per below. switch0_function can have 1 one of many values defined in the header file, of which could have been a larger value than the upper array bound, however it depends upon the user input into the system, it was possible that if a certain sequence of button presses never occurred, then the array would never have been indexed out of bounds, which of course the compiler can't possible predict.

 

display_write_string(button_function_strings[switch0_function], RED);

 

As always, thanks all

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

jtw_11 wrote:
The way in which the array in question is indexed is as per below.

Which is as I suggested in #10.

 

 

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

I did experiment with my simple example by writing a faulty definition of WriteString() in another file.

The compiler failed to spot my attempt to access beyond the end of the string; even with the -fwhole-program flag.

 

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

Indeed, I did note your reply came to the same conclusion as I - in fact, I tried to make two posts (including yours) as the solution, but could only do so for one!