Thread Rating:
  • 0 Vote(s) - 0 Average
  • 1
  • 2
  • 3
  • 4
  • 5
Clarification of Rule 17.4
#1
According to the description of the rule 17.4:
Quote:Array indexing shall only be applied to objects defined as an array type.
.
Given this I do not understand why the following is allowed:
Code:
uint8_t a[10];
uint8_t *p;

p = a;
p[5] = 0; /* why is this compliant? */
<t>Graham Andrews<br/>
Edinburgh Design Centre<br/>
Analog Devices Inc</t>
#2
Graham Andrews Wrote:According to the description of the rule 17.4:
Quote:Array indexing shall only be applied to objects defined as an array type.
.
Given this I do not understand why the following is allowed:
Code:
uint8_t a[10];
uint8_t *p;

p = a;
p[5] = 0; /* why is this compliant? */

There seems to be a bit of lax language here -- I assume the intent is more like "array indexing shall only be used to access objects of array type" (which you do in your example). So what matters is the object you access via the dereference, not the operand of the *.

Limiting the language so that a pointer cannot be used to access an array would be crippling. There wouldn't be a way of passing arrays into functions for example.


stephen
#3
I would suspect that was the case, but in the example for 17.4 it is contradicted by parameter p1. The example suggests that
Quote:p1[5]
is not compliant due to its declaration.
Code:
void my_fn(uint8_t *p1, uint8_t p2[])
{
...
    p1[5] = 0; /* not compliant - p1 was not declared as an array */
...
}

uint8_t a1[16];
uint8_t a2[16];

void test(void)
{
uint8_t *p;

   my_fn(a1,a2); /*actual p1 would be pointing at an array, but the static
                            check in my_fn still fails it */
   p = a1;
   my_fn(a1,p); /* Even worse p not an array, but not detected in
                           my_fn as actual parameter p2 declared as an array
                           type.  */
   p[5] = 0; /* ok, dynamically check */
}
The problem is the inconsistency between parameter passing and assignment. Parameters are statically checked using their declaration, other variables are dynamically checked by looking at the actual address they point at!
<t>Graham Andrews<br/>
Edinburgh Design Centre<br/>
Analog Devices Inc</t>
#4
This has been corrected in TC1.

Code:
uint8_t a[10];
uint8_t *p;

p = a;
p[5] = 0; /* NOT compliant */

In all cases, array indexing shall only be permitted on objects explicity declared tp be arrays. p[5] is not compliant as p is not explicity declared as an array.
Posted by and on behalf of the MISRA C Working Group
#5
I don't see the purpose of this TC or the part of the rule not allowing pointers to use the index operator.

Because there is absolutely no difference between pointer function parameters and array function parameters in ISO C (as long as the array size isn't static and typed out explicitly). It becomes quite ridiculous if you put it this way:

Code:
void my_fn (uint8_t* p1, uint8_t p2[])
{
  p1[5] = 0;     /* this segmentation fault is not compliant */
  p2[5] = 0;     /* this segmentation fault is compliant */
}

int main()
{
  uint8_t array [2];

  my_fn(array, array);
}


The chances of the index for p1 to be calculated incorrectly are the very same as the chances for the p2 index. The array [] syntax doesn't help compilers nor static analysis tools to spot out-of-bounds bugs, unless the size of the array pointer is typed out explicitly. Neither does C support boundary checks of arrays.

I also don't understand why performing for example increment operations on integer indices is considered less error-prone than pointer increments. It is just as easy for the programmer to calculate integer index values incorrectly as it is to calculate pointer calues. I don't see how pointers are less \"clean\" either.

I would even call integer indices less safe, because of the following situation:

Code:
short index = 32767;
  short* ptr = array + 32767;

  index++;
  ptr++;

  array[index] = 0; /* possible bug depending on int \"signedness\" */
  *ptr = 0;         /* will work fine on all systems*/


No matter how you put it, this is all in the hands of the programmer, who may produce as many out-of-bounds bugs as they wish regardless of this rule.

If anything should be banned, it is using the syntax

*(array + n) /* compliant if array type? */

rather than

array[n]

because I think most will agree that the second version is easier to read and understand.
#6
To sum up the above. This is MISRA's explanation why it isn't allowed:

\"Any explicitly calculated pointer value has the potential to access unintended or invalid memory addresses. Pointers may go out of bounds of arrays or structures, or may even point to effectively arbitrary locations.”

While that is true, the very same applies to array indexing using integers. The only way to avoid it would be to not calculate array indices at all, which would efficiently ban the usage of arrays in all C programs.
#7
There is one significant difference between a pointer and an index: you can only compare pointers within the same aggregate object (or with NULL). So you can check if an integer index is within the range of an array, but given a pointer you can't check that the pointer is in a valid range (unless you invoke the undefined behaviour in ANSI C90 6.3.8).

For example, given:
Code:
int32 a[N];

you can do:

Code:
int32 lookup(int32 v)
{
   int32 r;

   if ((v < 0) || (v >= N))
   {
       /* handle error. */
       r = 0;
   }
   else
   {
        r = a[v];
   }
   return r;
}

but you can't (portably) do:

Code:
int32 lookup(int32 const *vp)
{
   int32 r;

   if ((vp < a) || (vp >= (a + N))
   {
       /* handle error. */
       r = 0;
   }
   else
   {
        r = *vp;
   }
   return r;
}

Is this important? I suspect not very much in practice.

stephen
#8
Sure you can do that (rare) case portable:

Code:
uint32 lookup(uint32 const *vp)
{
   uint32 r;
   uint32 ptr_address   = (uint32)vp;
   uint32 array_address = (uint32)a;

   if ( (ptr_address <  array_address) ||
        (ptr_address >= array_address + N) )
   {
       /* handle error. */
       r = 0;
   }
   else
   {
        r = *vp;
   }
   return r;
}


The code won't be as efficient as the integer version, but that is always the case with pointers. We are discussing safety and not efficiency, so that is irrelevant.

It would also go against the advisory rule 11.3, but as long as you know the size of the system address bus (and document it), and use \"uint32\" rather than \"int32\", there won't be any problems.

The argument MISRA wrote was that pointers may go out of bounds, and I still don't see why integer indices would be less prone to do that.
#9
Code:
uint32 ptr_address   = (uint32)vp;

This is not portable code. See ANSI C90 6.3.4. You're invoking implementation defined behaviour.

stephen
#10
sparker Wrote:
Code:
uint32 ptr_address   = (uint32)vp;

This is not portable code. See ANSI C90 6.3.4. You're invoking implementation defined behaviour.

That would be why I made a note about MISRA rule 11.3 in my post. Still, this is off-topic.
#11
sparker Wrote:
Code:
uint32 ptr_address   = (uint32)vp;

This is not portable code. See ANSI C90 6.3.4. You're invoking implementation defined behaviour.

stephen

BTW at no time does MISRA reference ANSI C

The standard is ISO 9899:1990 (+A1, TC1,TC2) or ISO-C

ANSI-C is the local US standard and ISO is the International Version. (Otherwise MISRA could have refenced BSI-C as they are UK based) The reason being is local standards such as ANSI my or may not be the same as the ISO version

In the case of C there are differences between the two documents.
#12
sparker Wrote:There is one significant difference between a pointer and an index: you can only compare pointers within the same aggregate object (or with NULL). So you can check if an integer index is within the range of an array, but given a pointer you can't check that the pointer is in a valid range (unless you invoke the undefined behaviour in ANSI C90 6.3.8).
...
Is this important? I suspect not very much in practice.

stephen

So do I. This concession was made only for segmented processor architectures which are rapidly disappearing, even in the embedded space. All modern architectures have a linear address space and use the same arithmetic for pointers and unsigned integers.
<t>Konrad Schwarz<br/>
Siemens AG<br/>
CT SE 2 (Corporate Technology, Software & Engineering - Embedded Systems)</t>
#13
Also, something I just realized... though I might be wrong. Isn't the MISRA example is using two pointers rather than a pointer and an array?

void my_fn (uint8_t* p1, uint8_t p2[])

p1 is a pointer.
p2 is a pointer.

void func(uint8_t p3[10]);

p3 is pointer.

I believe there are no arrays in either of these examples.

I can't cite ISO-C 1990, but here is from ISO C 1999:

ISO 9899:1999 6.7.5.3 Wrote:7 A declaration of a parameter as ‘‘array of type’’ shall be adjusted to "qualified pointer to type", where the type qualifiers (if any) are those specified within the [ and ] of the array type derivation.

I suppose ISO C 1990 might state this differently, but I very much doubt it.
#14
MISRA appreciate that function parameters declared as an array or as a pointer are equivalent.

While there is no difference between both forms, MISRA insist on the use of the array notation if it is being used to address an array.
Posted by and on behalf of the MISRA C Working Group


Forum Jump:


Users browsing this thread: 2 Guest(s)