OSDev.org

Posted: **Thu Mar 17, 2016 2:41 am**

Hello.

In some of my classes I use the following code to zero the entire class.

FIL void tSystem::tOS_GenericOS::tCPU::Init()
	{
		ZeroThis();

Code: Select all

Util.Zero(this);

Code: Select all

TCF void Zero(T& Me)
	{
		::memset(&Me, 0, sizeof(T));
	}

This generates this code, which is not what I am after. I clears from 0 to sizeof(T)

Code: Select all

0000000000306cd0 <_ZN6Kernel7tSystem13tOS_GenericOS4tCPU4InitEv.constprop.113>:
  306cd0:	31 c0                	xor    eax,eax
  306cd2:	66 0f 1f 44 00 00    	nop    WORD PTR [rax+rax*1+0x0]
  306cd8:	c6 00 00             	mov    BYTE PTR [rax],0x0
  306cdb:	c6 40 01 00          	mov    BYTE PTR [rax+0x1],0x0
  306cdf:	48 83 c0 08          	add    rax,0x8
  306ce3:	c6 40 fa 00          	mov    BYTE PTR [rax-0x6],0x0
  306ce7:	c6 40 fb 00          	mov    BYTE PTR [rax-0x5],0x0
  306ceb:	c6 40 fc 00          	mov    BYTE PTR [rax-0x4],0x0
  306cef:	c6 40 fd 00          	mov    BYTE PTR [rax-0x3],0x0
  306cf3:	c6 40 fe 00          	mov    BYTE PTR [rax-0x2],0x0
  306cf7:	c6 40 ff 00          	mov    BYTE PTR [rax-0x1],0x0
  306cfb:	48 3d 18 02 36 00    	cmp    rax,0x360218
  306d01:	75 d5                	jne    306cd8 <_ZN6Kernel7tSystem13tOS_GenericOS4tCPU4InitEv.constprop.113+0x8>

Is there a way to do this?

Regards, Ali

Posted: **Thu Mar 17, 2016 2:44 am**

You haven't explained the problem:

What do you want the code to do? (A deep clean or a shallow clean, for example.)

What does the generated code actually do?

What is the difference between what you want and what is happening?

Posted: **Thu Mar 17, 2016 2:55 am**

iansjack wrote:You haven't explained the problem:

What do you want the code to do? (A deep clean or a shallow clean, for example.)

What does the generated code actually do?

What is the difference between what you want and what is happening?

Cheers, I want it to zero the class, all the data.

Posted: **Thu Mar 17, 2016 3:11 am**

Wow, the compiler is smart.

I left an old piece of code in xxx = null.

Hence it started from 0.

Must be time for bed. lol

Ali

Posted: **Thu Mar 17, 2016 3:12 am**

Undefined behaviour. Don't do this.

Now please explain why you want to do this, and we can perhaps help you with the real problem.

Posted: **Thu Mar 17, 2016 3:17 am**

Solar wrote:Undefined behaviour. Don't do this.

Now please explain why you want to do this, and we can perhaps help you with the real problem.

I had a T* null, this is what caused it.

I have a bug somewhere and I am trying to find it.

Many thanks, Ali

Posted: **Thu Mar 17, 2016 3:43 am**

You misunderstood.

Code: Select all

TCF void Zero(T& Me)
   {
      ::memset(&Me, 0, sizeof(T));
   }

Unless "T" is a POD(*) type, this is undefined behaviour, even for non-null parameters. This might well be (part of) the bug you're looking for.

(What is TCF, by the way?)

(*): POD, plain old data -- structures having no constructors, destructors, virtual member functions, or non-POD member variables.

Posted: **Thu Mar 17, 2016 4:53 am**

So you are not concerned about freeing any memory used by pointers in the class? (The answer may depend upon you are dealing with an existing instance or a new instance.)

Posted: **Thu Mar 17, 2016 8:27 am**

Not even mentioning the "funny" things that could happen post-lobotomy if

a member of Me is called,
Me gets copied and a copy constructor encounters only zeroes where it might expect to find member objects,
T goes out of scope and gets its (member's) destructor called.

Posted: **Thu Mar 17, 2016 12:14 pm**

I'm not sure but you might also run into problems when you zero the pointer to the vftable. I guess you would reference a NULL pointer when calling virtual functions.

Posted: **Thu Mar 17, 2016 12:48 pm**

Wow guys, thank you for the great comments.

Solar wrote:You misunderstood.
Code: Select all
TCF void Zero(T& Me)
   {
      ::memset(&Me, 0, sizeof(T));
   }
Unless "T" is a POD(*) type, this is undefined behaviour, even for non-null parameters. This might well be (part of) the bug you're looking for.

(What is TCF, by the way?)

(*): POD, plain old data -- structures having no constructors, destructors, virtual member functions, or non-POD member variables.

Right you are, sorry, it was meant to be (T* Me)
Note: the /* and */ comments are for Windows, these are removed before compiling by the VS attached program.
#define NIL /*!!__attribute__ ((noinline))!!*/
#define AIL /*!!__attribute__((always_inline))!!*/
#define FIL inline
#define TC template<class T>
#define TCF template<class T> FIL
#define TCN template<class T> NIL

iansjack wrote:So you are not concerned about freeing any memory used by pointers in the class? (The answer may depend upon you are dealing with an existing instance or a new instance.)

It is used in the "constructor" which I call "Init" in my classes, and directly after memory is allocated. Sometimes use to zero an array or struct of data, again in Init.

Solar wrote:Not even mentioning the "funny" things that could happen post-lobotomy if

a member of Me is called,

Me gets copied and a copy constructor encounters only zeroes where it might expect to find member objects,

T goes out of scope and gets its (member's) destructor called.

No destructors, the zero is only called when class created in memory.

dseller wrote:I'm not sure but you might also run into problems when you zero the pointer to the vftable. I guess you would reference a NULL pointer when calling virtual functions.

No virtual functions in these classes.

Many thanks, Ali

Posted: **Fri Mar 18, 2016 12:02 am**

This is wrong, for many different reasons, and hints at a severely wrong understanding of constructors, destructors, and class instance lifecycles.

Unfortunately, you still haven't provided any hint as to what it is you are intending with that memset, or a short compilable example, so I don't even know where to begin explaining.

Posted: **Fri Mar 18, 2016 12:26 pm**

Solar wrote:This is wrong, for many different reasons, and hints at a severely wrong understanding of constructors, destructors, and class instance lifecycles.

Unfortunately, you still haven't provided any hint as to what it is you are intending with that memset, or a short compilable example, so I don't even know where to begin explaining.

Thanks, This is for the Kernel, so I am not using new, delete.

Code: Select all

#define ForAll(Var, To) for (DWORD Var = 0; Var < To; Var++)
#define Fori(To) ForAll(i, (To))
#define Forn(To) ForAll(n, (To))
#define Forc(To) ForAll(c, (To))
#define Fore(To) ForAll(e, (To))
#define Forz(To) ForAll(z, (To))
#define Forx(To) ForAll(x, (To))
#define Fory(To) ForAll(y, (To))

Here is how I create the class:

Code: Select all

Forc(ABIT.CPUs.CPUCount)
		{
			CPU[c] = ABIT.FastMem.Alloc<tSystem::tOS_GenericOS::tCPU>();
			CPU[c]->Init();
		}

And this is the class->Init()

Code: Select all

	FIL void tSystem::tOS_GenericOS::tCPU::Init()
	{
		ZeroThis();
	}

This line "ABIT.FastMem.Alloc<tSystem::tOS_GenericOS::tCPU>();" just allocates memory and returns a pointer.

The intention of memset is to clear the entire class to 0.

Thanks for your responses, Ali

Posted: **Fri Mar 18, 2016 1:22 pm**

tsdnz wrote:Note: the /* and */ comments are for Windows, these are removed before compiling by the VS attached program.
Code: Select all
#define NIL /*!!__attribute__ ((noinline))!!*/
#define AIL /*!!__attribute__((always_inline))!!*/

Might I recommend using a preprocessor switch here instead?

Code: Select all

#ifdef WINDOWS_TESTBED
#define NIL 
#define AIL
#elif
#define NIL __attribute__ ((noinline))
#define AIL __attribute__((always_inline))
#endif

tsdnz wrote:This is for the Kernel, so I am not using new, delete.

OK, but that still doesn't answer the question.

tsdnz wrote:

Code: Select all

#define ForAll(Var, To) for (DWORD Var = 0; Var < To; Var++)
#define Fori(To) ForAll(i, (To))
#define Forn(To) ForAll(n, (To))
#define Forc(To) ForAll(c, (To))
#define Fore(To) ForAll(e, (To))
#define Forz(To) ForAll(z, (To))
#define Forx(To) ForAll(x, (To))
#define Fory(To) ForAll(y, (To))

facepalm You really, really shouldn't do this. C/C++ pre-processor macros are far too brittle for this to be a reasonable thing to use them for. I know that the standard for() construct is awkward for many of the common use cases, but this is not the solution. I also know that the Linux kernel uses this sort of BS all over the place, too, but just because a Big Name Player does something that is a bad idea, that doesn't mean it isn't a bad idea.

If you are going use these macros despite this warning, I would recommend at least putting the parens around To in ForAll() rather than repeating them in each of the individual specialization macros. I don't know if you can think of a use case where you would want to re-evaluate the To clause on each pass, but none of those which I can think of are ones which you would use this approach for in the first place.

Also, rather than using DWORD for a (presumably) 32-bit value, I would recommend using the <cstdint>-defined uint32_t type, but that's a stylistic issue rather than any actual technical advice.

tsdnz wrote: Here is how I create the class:
Code: Select all
Forc(ABIT.CPUs.CPUCount)
		{
			CPU[c] = ABIT.FastMem.Alloc<tSystem::tOS_GenericOS::tCPU>();
			CPU[c]->Init();
		}
And this is the class->Init()
Code: Select all
	FIL void tSystem::tOS_GenericOS::tCPU::Init()
	{
		ZeroThis();
	}
This line "ABIT.FastMem.Alloc<tSystem::tOS_GenericOS::tCPU>();" just allocates memory and returns a pointer.

The intention of memset is to clear the entire class to 0.

So this is for initialization only, not for release? That's rather important information, and exactly the sort of thing we needed. Good.

Also, it is actually clearing the individual object being constructed, not the class itself. Even better.

HOWEVER... it still isn't clear why this is needed, or where it is being used - I almost am wondering why it would be in the kernel code at all, rather than part of the compiler or library. In C++, there's no actual requirement that memory allocated for new objects be cleared, and with good reason - aside from the extra time it takes, it can mask some types of errors by making the buggy code appear to work. If a member variable is getting used before it is explicitly initialized by the c'tor, or the c'tor leaves it uninitialized and later member methods try to use the uninitialized variable, that is a bug in the class code, not a fault in the standard initializer.

BTW, what toolchain (compiler, assembler, linker) are you using? The fact that it is emitting Intel-syntax assembly tells me that it is probably not GCC, so... VC++, maybe? That could be relevant here.

Posted: **Fri Mar 18, 2016 2:56 pm**

It is much worse than that.

Once the object has been constructed, that means all the member constructors have been run as well.

Then he just slaps zeroes on top of it all.

When the object goes out of scope, the destructors will run, assuming well-formed member instances.

This is desaster waiting to happen. It might work for the POD type he is working with now, but as soon as any of his constructors do any significant work, it will blow up.

Init() and memset() are the very things C++ objects were meant to REPLACE....

OSDev.org

Zero This

Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This

Re: Zero This