Documentation for uxTaskGetStackHighWaterMark should be updated?

jankok wrote on Wednesday, March 14, 2018:

At https://www.freertos.org/uxTaskGetStackHighWaterMark.html it gives the return type as UBaseType_t, however, at least in Amazon’s portmacro.h, UBaseType_t is typedef’d as unsigned char. This caused me considerable puzzlement when I increased the initial stack size for a task and it caused the reported high water mark to decrease, as the high water mark number rolled over from 255 to 0.

Amazon’s portmacro.h for Arduino contains:

#define portSTACK_TYPE  uint8_t
typedef portSTACK_TYPE StackType_t;
typedef signed char BaseType_t;
typedef unsigned char UBaseType_t;

and FreeRTOSconfig.h contains:

#define configMINIMAL_STACK_SIZE            ( ( portSTACK_TYPE ) 192 )
#define configSTACK_DEPTH_TYPE              uint16_t

Does this look correct? It looks odd to me since portSTACK_TYPE ends up defined as uint8_t, which might cause problems if one were to try to increase configMINIMAL_STACK_SIZE beyond 255.

In any case, it seems to me the documentation for uxTaskGetStackHighWaterMark should be updated to show some other return type.

rtel wrote on Wednesday, March 14, 2018:

This has been discussed multiple times, so you will find info in the
forum. Some changes were made in V10 to allow you to use a different
data type - I think using vTaskGetInfo() rather than
uxTaskGetStackHighWaterMark().

In any case, it seems to me the documentation for
uxTaskGetStackHighWaterMark should be updated to show some other return
type.

As can be seen in the header file and the source file, the prototype for
the function is:

UBaseType_t uxTaskGetStackHighWaterMark( TaskHandle_t xTask );

So the return type is shown correctly in the documentation. The ‘base
type’ is the natural (most efficient) type for the architecture so is
different in different ports - normally 32-bits - but not on an AVR.

jankok wrote on Wednesday, March 14, 2018:

Well, task.h shows

#if configENABLE_BACKWARD_COMPATIBILITY == 1
UBaseType_t uxTaskGetStackHighWaterMark( TaskHandle_t xTask ) PRIVILEGED_FUNCTION;
#else
configSTACK_DEPTH_TYPE uxTaskGetStackHighWaterMark( TaskHandle_t xTask ) PRIVILEGED_FUNCTION;
#endif /* configENABLE_BACKWARD_COMPATIBILITY */

So the return type depends on whether configENABLE_BACKWARD_COMPATIBILITY == 1. In the Arduino FreeRTOS library, configENABLE_BACKWARD_COMPATIBILITY is not defined at all, so the return type is configSTACK_DEPTH_TYPE which is uint16_t. And indeed the function can return a value greater than 255. I’ve tested it.

The documentation says the return type is UBaseType_t, but that’s not true for the Arduino port, and that can lead users like me to get nonsensical values for the high water mark, when taking the usage example code for uxTaskGetStackHighWaterMark at face value.

Should portSTACK_TYPE , BaseType_t and UBaseType_t be defined as 16-bit types in the Arduino port? If not, I think the documentation for uxTaskGetStackHighWaterMark should be updated to warn about the confusing type issues.

In any case, thanks for the help, Richard, and thanks for making FreeRTOS available to the world - for free! :slight_smile:

rtel wrote on Wednesday, March 14, 2018:

Well, task.h shows

#if configENABLE_BACKWARD_COMPATIBILITY == 1
UBaseType_t uxTaskGetStackHighWaterMark( TaskHandle_t xTask ) PRIVILEGED_FUNCTION;
#else
configSTACK_DEPTH_TYPE uxTaskGetStackHighWaterMark( TaskHandle_t xTask ) PRIVILEGED_FUNCTION;
#endif /* configENABLE_BACKWARD_COMPATIBILITY */

So the return type depends on whether
configENABLE_BACKWARD_COMPATIBILITY == 1. In the Arduino FreeRTOS
library, configENABLE_BACKWARD_COMPATIBILITY is not defined at all, so
the return type is configSTACK_DEPTH_TYPE which is uint16_t. And indeed
the function can return a value greater than 255. I’ve tested it.

Curious - I think if configENABLE_BACKWARD_COMPATIBILITY is not defined
at all then it will default to 1. We always try to ensure FreeRTOS
versions default to being backward compatible to ensure upgrading is
easy. If that is the case then uxTaskGetStackHighwaterMark() should
have a return type of UBaseType_t - are you 100% positive
configENABLE_BACKWARD_COMPATIBILITY is not defined anywhere in your
application?

Should portSTACK_TYPE , BaseType_t and UBaseType_t be defined as 16-bit
types in the Arduino port?

No - portSTACK_TYPE is dictated by the architecture, you can’t choose a
value for it as if it is wrong the code just won’t work.

The base type is set to the ‘natural’ size of the architecture to ensure
it is as efficient as possible and is predominantly used for booleans,
rather than holding values. uxTaskGetStackHighWaterMark() returning
such a value is a symptom of FreeRTOS being 15 years old.

jankok wrote on Wednesday, March 14, 2018:

Curious - I think if configENABLE_BACKWARD_COMPATIBILITY is not defined
at all then it will default to 1.

Aparently not. According to http://gcc.gnu.org/onlinedocs/cpp/If.html#If , in the expression part of a #if, “Identifiers that are not macros … are all considered to be the number zero.”

I grepped for COMPATIBILITY in the Arduino library src directory and didn’t see any definitions of configENABLE_BACKWARD_COMPATIBILITY. So the #if is interpreting it as 0.

I think we can agree that in a processor that can have more than 255 words of stack space, the return type of the function should not be an 8-bit type. So, what should the return type of the function be?

My suggestion is to cut the Gordian knot, keep it simple, define it as just:
configSTACK_DEPTH_TYPE uxTaskGetStackHighWaterMark( TaskHandle_t xTask ) PRIVILEGED_FUNCTION;
Forget about configENABLE_BACKWARD_COMPATIBILITY. It seems to me the purpose of that is to avoid problems for users of ancient code if they update to a newer version of the FreeRTOS library. If they don’t update, there should be no problem. If they do update, having them define configSTACK_DEPTH_TYPE seems like a small price to pay. You could add code like this before the declaration of uxTaskGetStackHighWaterMark in task.h:

#ifndef configSTACK_DEPTH_TYPE
	#error Missing definition:  configMAX_STACK_DEPTH_TYPE must be defined in FreeRTOSConfig.h.  See the Configuration section of the FreeRTOS API documentation for details.
#endif

rtel wrote on Wednesday, March 14, 2018:

It should be defaulted in FreeRTOS.h - see line 867 (at the time of
writing) here:
https://sourceforge.net/p/freertos/code/HEAD/tree/trunk/FreeRTOS/Source/include/FreeRTOS.h#l4

jankok wrote on Thursday, March 15, 2018:

All right, so I guess you’re saying that ports to processors, where UBaseType_t is not large enough to hold a stack size value, should include

#define configENABLE_BACKWARD_COMPATIBILITY 0
#define configSTACK_DEPTH_TYPE              uint16_t // or whatever

in FreeRTOSConfig.h. The latest Arduino FreeRTOS library FreeRTOSConfig.h contains the second line but not the first. Arduino_FreeRTOS.h is V10.0.0 and doesn’t contain the code to default configENABLE_BACKWARD_COMPATIBILITY to 1 if undefined.

I guess I can live with that. However, I still think https://www.freertos.org/uxTaskGetStackHighWaterMark.html should be updated, because if someone copies the example usage code, while using an AVR or similar processor, the results won’t make sense if the high water mark is greater than 255. I got bitten by that, and that’s why I came here and started this thread!

(I was having problems that I thought might be due to some sort of memory corruption. I tried investigating stack space using the example code in that web page. I started going crazy because increasing the allocated stack space could sometimes decrease the high water mark, and I never could get the high water mark very large. It was only when I tried seeing exactly how large I could make it, and I increased the stack space by 1 and saw the high water mark go from 255 to 0, that a light bulb turned on and I realized that the high water mark was somehow getting truncated to 8 bits.)

jankok wrote on Tuesday, March 20, 2018:

Rick Richard ran into the same problem, see https://sourceforge.net/p/freertos/discussion/382005/thread/335d645d/

The problem occurs on processors where sizeof(UBaseType_t) != sizeof(configSTACK_DEPTH_TYPE). For example on an 8-bit AVR processor, the stack can grow one byte at a time, but the stack can have more than 255 elements, so UBaseType_t is defined as uint8_t and configSTACK_DEPTH_TYPE is defined as uint16_t.

I think the simplest solution that would maintain backward compatibility with existing user’s code, would be to change the return type of uxTaskGetStackHighWaterMark to be configSTACK_DEPTH_TYPE, and then default the value of configSTACK_DEPTH_TYPE to be UBaseType_t if configSTACK_DEPTH_TYPE is not defined in FreeRTOSConfig.h.

In any case, the documentation for uxTaskGetStackHighWaterMark should be updated to show the correct return type, so users don’t keep tripping over this problem. Would you like me to file this as a bug?

rtel wrote on Tuesday, March 20, 2018:

While I agree the documentation can be updated to make the behaviour
very clear, it is still unclear to me which documentation does not show
the function returning the type it does actually return. Can you let me
know, as then I can fix it.

jankok wrote on Wednesday, March 21, 2018:

https://www.freertos.org/uxTaskGetStackHighWaterMark.html

rtel wrote on Wednesday, March 21, 2018:

That page shows the function returning UBaseType_t - which is what it
does return.