CODEWITHSUNDEEP
c++c
thangktran
thangktran

Reputation: 78

Strange error with preprocessor

Currently I am having a very strange error with the preprocessor. I have searched and couldn't find anything relavent and therefore would like to post it here. Below is my sample code 

#include <stdio.h>

#define DDR_BASEADDR  0x000000000U + 128*1024*1024

int main()
{
     uint32_t* test2 = (uint32_t*) DDR_BASEADDR;

     uint32_t whatIsThis = DDR_BASEADDR;

     uint32_t* test3 = (uint32_t*) whatIsThis;

     printf( "%x %x %x %x\n\r", DDR_BASEADDR, test2, test3, whatIsThis);

     return 0;
}

The output of this code should all be 0x8000000. However, the outputs are:8000000 20000000 8000000 8000000.

I believe it is not the datatype that caused this problem, since it also appears even if i change uint32_t* test2 = (uint32_t*) DDR_BASEADDR; to int32_t* test2 = (int32_t*) DDR_BASEADDR;

I test this both on ARM A9 from the Zedboard and with C++ online compiler and get the same result. Thank you for your time and effort.

Thang Tran

Upvotes: 3

Views: 185

Answers (4)

unalignedmemoryaccess
unalignedmemoryaccess

Reputation: 7441

Your macro was expandend to:

uint32_t* test2 = (uint32_t*) 0x000000000U + 128*1024*1024;

Which is the same as doing this:

//Check multiply by sizeof at the end
uint32_t* test2 = (uint32_t *) (0x000000000U + 128*1024*1024 * sizeof(*test2));

because your size of the pointer type is sizeof(uint32_t) and therefore any operation for increase/decrease multiplies by this size.

What you want is:

uint32_t* test2 = (uint32_t *) (0x000000000U + 128*1024*1024);

Which means you first do calculation of address, then you cast to the desired pointer.

Upvotes: 4

Sergey Kalinichenko
Sergey Kalinichenko

Reputation: 726579

There is absolutely nothing strange about this: on this line

uint32_t* test2 = (uint32_t*) DDR_BASEADDR;

the cast to pointer operation is applied to 0x000000000U, after which an int of 128*1024*1024 is added to the result of the cast, i.e.

uint32_t* test2 = (uint32_t*) 0x000000000U + 128*1024*1024;
//                ^^^^^^^^^^^^^^^^^^^^^^^^   ^^^^^^^^^^^^^
//                       Pointer                Offset

This means the pointer arithmetic is performed, so in integer terms the offset value is multiplied by sizeof(uint32_t), yielding 0x8000000.

However, on this line

uint32_t whatIsThis = DDR_BASEADDR;

there is no cast to pointer, so the addition is done entirely in integers. That is why there is no multiplication by sizeof(uint32_t), and the result is 0x20000000.

If you want all four cases printing 800000000, cast to pointer needs to be inside the macro:

#define DDR_BASEADDR  ((uint32_t*)0x000000000U + 128*1024*1024)

and a cast to int32_t needs to be on the declaration of whatIsThis:

uint32_t whatIsThis = (uint32_t)DDR_BASEADDR;

However, uint32_t is not a portable type for the pointer. Use uintptr_t instead.

Upvotes: 3

Yunnosch
Yunnosch

Reputation: 26703

With 

#define DDR_BASEADDR  0x000000000U + 128*1024*1024

the code line

uint32_t* test2 = (uint32_t*) DDR_BASEADDR;

expands to

uint32_t* test2 = (uint32_t*) 0x000000000U + 128*1024*1024;

which is (because of the ways of pointer arithmetic) practically 

uint32_t* test2 = (uint32_t*) (0x000000000U + (sizeof(uint32_t) *(128*1024*1024));

or 

uint32_t* test2 = (uint32_t*) (0x000000000U + (4 * 0x8000000));

That gives you 20000000 instead of 8000000, both in hex format of course.
Multiplying by 4 in hex, for multiples of 4, is like multiplying by 16 (which in hex is like multiplying by 10 in decimal) and dividing by 4.
I.e. it results in 1/4 the value, shifted left by one hex-digit: 80->200.

The other code lines have the effect of a pair of braces () in some way or another.

If you use 

    #define DDR_BASEADDR  (0x000000000U + 128*1024*1024)

you should be fine, and are, as you have already verified.

Using braces generously is very good practice, especially with macros.

Note: You also should be more careful with printf and using the correct representation of the values you want to print (see comments and other answers). Otherwise the resulting Undefined Behaviour is evil! All kinds of unwanted and inexplicable things can result from UB. However UB is not needed for explaining the values you get in this special case. (You got lucky.)

Upvotes: 1

Bo Persson
Bo Persson

Reputation: 92261

The macro expansion gives you

uint32_t* test2 = (uint32_t*) 0x000000000U + 128*1024*1024;

when you probably expected

uint32_t* test2 = (uint32_t*) (0x000000000U + 128*1024*1024);

That's why it is recommended to use lots of parenthesis in macro definitions, or - even better - use a function.

Upvotes: 8

Related Questions