topic Uninitialized memory access in struct member padding in Analyzers

Uninitialized memory access in struct member padding

Bada__Peter — Wed, 23 Nov 2011 16:38:03 GMT

Hi,

We got Uninitialized memory access when the program copies structures allocating them using new keyword.
Inside the actual structure there are some padding bytes. (I know we should design structures to avoid padding, but we have tons of legacy sources we don't have resource to refactor old codes.) I think this is false alarm, because the programnever accesses padding bytes when reads or writes the struct members. We should differentiate or filtersomehow this kid of problems from real uninitializes memory accesses.

Is there a way to do it?

Thanks,
Peter

Uninitialized memory access in struct member padding

IDZ_A_Intel — Thu, 24 Nov 2011 08:44:53 GMT

Hi,

I just built a simple test case but I haven't seen any uninitialized memory access for padding bytes.

Please let me know if I understood your question correctly, or you can provide a test case to help of reproducing this problem.

I'm using Inspector XE 2011 IXE Update 7.

Thanks, Peter

[cpp]# inspxe-cl -version
Intel Inspector XE 2011 Update 7 (build 189290) Command Line tool
Copyright (C) 2009-2011 Intel Corporation. All rights reserved.

/* test_uninitial_struct.c */
#include 
struct my_struct{
int int_a;
short short_b;
short pad;
};

int main()
{
  struct my_struct *theStruct, theStruct1;

  theStruct = (struct my_struct*)malloc(sizeof(struct my_struct));
  theStruct->int_a = 10;
  theStruct->short_b = 20;
  memcpy(&theStruct1, theStruct, sizeof(struct my_struct));
  printf("integer is %d, short is %dn", theStruct1.int_a, theStruct1.short_b);

  free(theStruct);
  return 0;
}
 
# gcc -g test_uninitial_struct.c -o test_uninitial_struct
# inspxe-cl -collect mi3 -- ./test_uninitial_struct  Used suppression file(s): []
integer is 10, short is 20

0 new problem(s) found

[/cpp]

Uninitialized memory access in struct member padding

Bada__Peter — Fri, 25 Nov 2011 12:57:36 GMT

Hi,

Thanks for your quick answer but thereis no struct padding.

Try this:
struct padd {
int m1;
bool m2;
// some(3)padding bytes here (0xCD or 0xCC), these are uninitilized
int m3;
};
// no pragma padding,I guess the default is 4
padd *ppadd = new padd(); // or malloc or auto
ppass->m1 = 1;
ppass->m2 = true;
ppass->m3 = 2;

If you dump the ppadd, you can see that there are 0xcd (or 0xcc) bytesbetween m2 and m3.
I see these bytes in my debugger after the allocation and where the Inspector signs the problem.
We use VS2010 x64 compiler linker and Inspector XE 2011 Update 7
At copy contructorsor simple struct copy Inspector always displays red memory problem even all struct member were initialized properly.

The real problem is that during one of our regression test we got 100 uninit problems but 97-98 are this kind.
What is your observation in this case?

Thanks,
Peter

Uninitialized memory access in struct member padding

SergeyKostrov — Fri, 25 Nov 2011 14:24:52 GMT

What is your default alignment? It looks like 4 bytes. So,a C/C++ compiler is trying to align all struct members, especially m3 int type member,and this is a correct action. I would consider small code modifications andhere are two possiblesolutions:

// Solution 1
...
struct padd
{
int m1;
int m3;
bool m2;
}
...

// Solution 2
...
struct padd
{
int m1;
bool m2;
bool dummy[3]; // needs to be initialized with 0x0later
int m3;
}
...

Uninitialized memory access in struct member padding

Peter_W_Intel — Sat, 26 Nov 2011 02:46:01 GMT

Sergey,

Finally I can reproduce this problem, but I don't think that this is a bug of Inspector XE!

If I insertedthe dummy tomy structure, and copy this structure to otherspace - actually dummy has been accessed and not initialized, detected by Inspector XE.

# inspxe-cl -collect mi3 -- ./test_uninitial_struct Used suppression file(s): []
integer a is 10, integer c is 20, bool is 1

2 new problem(s) found
1 Uninitialized memory access problem(s) detected
1 Uninitialized partial memory access problem(s) detected

The scenario is - the user has to copy data structure in the project, many places. Theuser won't see these unnecessary error reports (dummy uninitialized), because the user never uses these dummys.

The request is - how to filter theseunintialized error reports? Use Suppression? Sometime isnot feasible,if there are hundred errors.

Orcan we exclude "uninitialized accesses" from mi2/m3 ?

Regards, Peter Wang

[cpp]#include 

typedef enum { true=1, false=0} bool;
struct my_struct{
int int_a;
bool bool_b;
bool dummy[3];
int int_c;
};

int main()
{
  int i;
  struct my_struct *theStruct, theStruct1;

  theStruct = (struct my_struct*)malloc(sizeof(struct my_struct));
  theStruct->int_a = 10;
  theStruct->int_c = 20;
  theStruct->bool_b = true;

  /*
   for (i=0; i<3; i++) theStruct->dummy= false;
   */

  memcpy(&theStruct1, theStruct, sizeof(struct my_struct));
  printf("integer a is %d, integer c is %d, bool is %dn", theStruct1.int_a, theStruct1.int_c, theStruct1.bool_b);


  free(theStruct);
  return 0;
}
[/cpp]

Uninitialized memory access in struct member padding

SergeyKostrov — Sat, 26 Nov 2011 05:43:42 GMT

Here is another idea:

You could try to use calloc CRT-function instead of malloc. By default, calloc initializesallocated memory with zeros ( 0x0 ). But,calloc is a little bit slower than mallocandhas two arguments.So, you will need a macro-wrapper. Anyway,this is how it could look like:

...
#define malloc2( iSize ) calloc( 1, iSize )
...
theStruct=( structmy_struct * )malloc2( sizeof( structmy_struct ) );
...

Take a look at MSDN for more technical detailsoncalloc CRT-function.

Best regards,
Sergey

Uninitialized memory access in struct member padding

Bada__Peter — Sat, 26 Nov 2011 10:38:53 GMT

Hi All,

Thanks for your suggestion. We know how we could suppress (henceforward I think it is false) alarms with additional work changing the sources. Redesing structures (insert dummy fields) could be a huge effort. (we develop image processing and OCR engines, millions lines of pretty old but working sources. We inherited them from earlier companies.)

In new sources structures are designed without pack sensivity and without padding spaces.

It is a good idea to use zero init allocation but it hides the real uninitialization problems. The situtation could better, because the program will always behave same wayand there is a better chance to debug the actual problem (except stackvariables)But in this case Inspector would be unnecessary finding uninitialized struct and class members.

Many thanks,
Peter

Uninitialized memory access in struct member padding

Peter_W_Intel — Sat, 26 Nov 2011 10:39:13 GMT

Quoting Sergey Kostrov

This is a great idea of using calloc instead of malloc, for initialization.

Thanks Sergey!

Regards, Peter

Uninitialized memory access in struct member padding

Bada__Peter — Sat, 26 Nov 2011 10:59:55 GMT

Peter,

Supressing or filtering both are OK for us. Changing structures is risky. We always fighting with OCR accuracy. Small changes in the code could cause significant changes on the output. This is a real pain of old legacy modules.
Currently we don't have time to refactor old stuff. I know we should do it somehow sometime in the near future, but I would like to make some improvements using great tool: Inspector.
The actual problem with this (let say not 100% accurate or not valid) alarm is the developer got 40-50 uninit problems. He or she starts to investigate them and the first 10 issue are not a real problem for them. In addition it is not obvoius. He see in the reportthere areuninit members but they don't understand (because it is false). I suggest them todebug to the certain line of source code wher Inspector signs and dump actual the structure and inspect the memory dump to hunt 0xcd. If there is 0xcdensure they are padding bytes only. So after a while they easy give up, because of unnecessary work.
I usually force this work because Inspector found some real and valid uninint problems what is great for us. But it could be much better.

Thanks,
Peter

Uninitialized memory access in struct member padding

Bada__Peter — Sat, 26 Nov 2011 11:12:17 GMT

With respect: It is great idea, if we want to suppress not to find certain uninit problems. I think...

Uninitialized memory access in struct member padding

SergeyKostrov — Sun, 27 Nov 2011 03:09:01 GMT

Peter ( I mean the owner of the thread ),

In myprevious postI've suggested an ideaofusing the calloc CRT-function instead of malloc through
the malloc2 macro.

Sorry, one more time, the calloc function will initialize all members of a structure to zerosas soon
as memory allocated and Intel'sInspector won't see any problems ( that is, uninitialized bytes ).

In your case with "uninitialized-padded-bytes"some source codesmodifications are inevitable but
with the malloc2 macro you don't need to change structures.

You could also use a more dangerous way ofusing a _declspec( align(1) ) directive before
declaration of astructure and in that case none of bytes will be padded. But, it is possible thatpush()
and pop() directives will need to used to save and restorecurrent alignments of codes.

Best regards,
Sergey

Uninitialized memory access in struct member padding

SergeyKostrov — Tue, 29 Nov 2011 01:53:18 GMT

Here is another way of eliminatingpadded bytes in a structure...

In order to provide a completebinary-compatibility of some data structures a #pragma pack( n ) directive
could be used beforedeclaration of a structure. After modification(s)appliedcodes have to be tested as
better as possible.

Take a look at MSDN's article "C/C++ Preprocessor Reference" for pack( n ) keyword.

...
// Let's say default alignment of an applicationis 4...
...
#pragma pack( 1 )
typedef struct tagTESTDATA
{
int m1;
bool m2;
int m3;
}TESTDATA;
#pragma pack()
// Restores default alignment...
...
void main( void )
{
TESTDATA td;
td.m1 = 0x22222222;
td.m2 = true;
td.m3 = 0x33333333;
}

A memory dump of 'td' variable forthe application compiledwith #pragma pack( 1 ) directive:
...
222222220133333333
...

A memory dump of 'td' variable for the application compiledwithout #pragma pack( 1 ) directive:
...
2222222201cccccc33333333
...

Best regards,
Sergey

Uninitialized memory access in struct member padding

Bada__Peter — Tue, 29 Nov 2011 13:55:44 GMT

Sergey,

Thank you for your advices. We know calloc. But using calloc we hide the real uninitialized memory access. Evenfrom Inspector.The program will not certainly work correctly with zero init. If the algorithm was contructed assuming zero init, it is ok, but in other cases zero allocated memory is uninitialized from the algorithm point of view. In addition Inspector does not have a chance to sign the real problem. We just hide the problem.

And what aboutauto(stack)variables or new?

int foo()
{
padd p1;
.
or
.
padd *p1;
p1 = new padd(); //We can use calloc only if we override new and placement new
.
}

Changing pack()(push/pop)in old sourcesis realy dangerous as you said.There is no plan to be resources forsuch changes. It is serious like changing the order of the members or adding padd dummies.
May be you are right, some how we should refactor our structures. It would be worth in long term.
As I see Inspector will notaddress this problem.

Thank you, Sergey.

Uninitialized memory access in struct member padding

SergeyKostrov — Wed, 30 Nov 2011 02:04:52 GMT

int foo()
{
...
padd p1 = { 0x0 };
...
}

or

...
#define new2( iSize ) calloc( 1, iSize )

{
...
padd *p1 = NULL;
p = new2( sizeof(padd ) ); //Actually, there is a big problemhere, that is, an object constructor
... // won't be called! It isnot recommended way in your case.
}

or

Take a look at: http://software.intel.com/en-us/forums/showthread.php?t=86933and here is some
stuff from the post:

...
>>...I still wonder why would one need to use malloc instead of new...
>>
>>To improveperformance! Becauseconstructor and destructor arenot called,memory
>>allocated anywayand all object attributes could beinitialized later. Icalled it Delayed
>>ObjectInitialization ( DOI ).
>>
>>So, I use that trick with a calloc CRT-function and a C++ template, like:
>>
>>...
>>m_ppvRs = ( RTvoid ** )( TStrassenHBCResultSet< RTint > ** )calloc(
>>m_uiNumOfPartitions,
>>sizeof( TStrassenHBCResultSet< RTint > * ) );
>>if( m_ppvRs == RTnull )
>> return ( RTbool )bOk;
>>...
>>
>>and, by default a block ofmemory is initialized to 0x0s by calloc and I don't need to do anything!
...