[Bug] Wrong Exception Handling : Destructor not called immediately

classic Classic list List threaded Threaded
15 messages Options
Reply | Threaded
Open this post in threaded view
|

[Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
Gentle Ping !! Please help to verify if this is a bug and if my analysis is correct.


On Tue, Apr 1, 2014 at 10:18 PM, suyog sarda <[hidden email]> wrote:
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda



--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

Matthew Del Buono
I think the test case looks correct but my analysis is inconsistent with yours. However I agree there is a bug. Interestingly, while looking into it just now I wrote my own test case which revealed a separate (but perhaps related) bug in an old version of g++ (4.1.2) where destructors were being called twice. The current version appears to be correct though. Anyway, my test case follows:

#include <iostream>
class Foo
{
private:
   static int n;
   static bool thrown;
   int m_n;
public:
   Foo():m_n(n++) { std::cout << "Constructor " << m_n << std::endl; }
   ~Foo() noexcept(false) { std::cout << "Destructor " << m_n << std::endl; 
      if(!thrown) { 
         thrown = true; 
         std::cout << "Throwing exception" << std::endl; 
         throw 0; 
      }
   }
};

int Foo::n = 0;
bool Foo::thrown = false;

Foo bar(Foo a, Foo b) { return Foo(); }

int main()
{
   try 
   {
      Foo foo = bar(Foo(), Foo());
   }
   catch(...)
   {
      std::cout << "In exception handler" << std::endl;
   }
   std::cout << "Out of exception handler" << std::endl;
}


I expect the following output:
Constructor 0
Constructor 1
Constructor 2
Destructor 1
Throwing exception
Destructor 2
Destructor 0
In exception handler
Out of exception handler

With clang++ 3.4 I'm getting the following output:
Constructor 0
Constructor 1
Constructor 2
Destructor 1
Throwing exception
Destructor 0
In exception handler
Out of exception handler


So it looks like the destructor for the returned object is not being called at all (not in the wrong order). I get the same behavior in C++11 mode, too (though you have to add noexcept(false) to the destructor due to the implicit noexcept).

-- Matthew P. Del Buono


On Fri, Apr 4, 2014 at 9:39 AM, suyog sarda <[hidden email]> wrote:
Gentle Ping !! Please help to verify if this is a bug and if my analysis is correct.


On Tue, Apr 1, 2014 at 10:18 PM, suyog sarda <[hidden email]> wrote:
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda



--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

Halfdan Ingvarsson
In reply to this post by suyog sarda
Possibly related to this: http://llvm.org/bugs/show_bug.cgi?id=14223

We still compile specific projects with -O1 to get around this issue.

 - ½

On 14-04-04 12:39 PM, suyog sarda wrote:
Gentle Ping !! Please help to verify if this is a bug and if my analysis is correct.


On Tue, Apr 1, 2014 at 10:18 PM, suyog sarda <[hidden email]> wrote:
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda



--
With regards,
Suyog Sarda


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
The test case in Bug 14223 is working fine with the latest trunk version.


On Sat, Apr 5, 2014 at 12:25 AM, Halfdan Ingvarsson <[hidden email]> wrote:
Possibly related to this: http://llvm.org/bugs/show_bug.cgi?id=14223

We still compile specific projects with -O1 to get around this issue.

 - ½


On 14-04-04 12:39 PM, suyog sarda wrote:
Gentle Ping !! Please help to verify if this is a bug and if my analysis is correct.


On Tue, Apr 1, 2014 at 10:18 PM, suyog sarda <[hidden email]> wrote:
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda



--
With regards,
Suyog Sarda


_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev




--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

Halfdan Ingvarsson
On 14-04-07 05:43 AM, suyog sarda wrote:
> The test case in Bug 14223 is working fine with the latest trunk version.
>
>
And so it is. I've resolved the bug, adding the clang svn revision.

Thanks!

  - ½

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

Richard Smith
In reply to this post by suyog sarda
See:

  http://lists.cs.uiuc.edu/pipermail/llvmdev/2012-June/050886.html

... where this exact testcase was last discussed.

On Fri, Apr 4, 2014 at 9:39 AM, suyog sarda <[hidden email]> wrote:
Gentle Ping !! Please help to verify if this is a bug and if my analysis is correct.


On Tue, Apr 1, 2014 at 10:18 PM, suyog sarda <[hidden email]> wrote:
Hi all,

This is a test g++ test case for checking correct exception handling.

#ifdef __GXX_EXPERIMENTAL_CXX0X__
#define NOEXCEPT_FALSE noexcept (false)
#else
#define NOEXCEPT_FALSE
#endif

extern "C" void abort ();
 
int thrown;

int as;
struct a {
  a () { ++as; }
  ~a () NOEXCEPT_FALSE { --as; if (thrown++ == 0) throw 42; }
};
 
int f (a const&) { return 1; }
int f (a const&, a const&) { return 1; }

int bs;
int as_sav;
struct b {
  b (...) { ++bs; }
  ~b ()   { --bs; as_sav = as; }
};

bool p;
void g()
{
  if (p) throw 42;
}

int main () {
  thrown = 0;
  try {
    b tmp(f (a(), a()));

    g();
  } 
  catch (...) {}

  // We throw when the first a is destroyed, which should destroy b before
  // the other a.
  if (as_sav != 1)
    abort ();

  thrown = 0;
  try {
    b tmp(f (a()));

    g();
  } 
  catch (...) {}
 
  if (bs != 0)
    abort ();
}


This Test Case aborts on ARM as well as X86 with clang latest trunk while with g++ no aborts are seen .

Here, when first temporary object 'a' gets destroyed, its destructor is called where we throw an exception. This should immediately call destructor of 'b' and before calling constructor of second temporary object 'a'. Instead, with clang, it is waiting for second temporary object 'a' to get destroyed and then calling its own destructor.

In my opinion, the compiler is not generating correct landing pad for the exception. Can someone please help in validating this reason? If it is a genuine bug then i will file a bug and try to work out solution for it.

-- 
With regards,
Suyog Sarda



--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev



_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
One more doubt i had.

If suppose there is heirarchy of temporary objects like :

b(b(b(a()))) ;

and an exception occurs in the destructor of innermost object, should only the immediate parent object's destructor should be called or cleanup should occur 
for the whole heirarchy of parent objects? If so, should it be true if there is a function call somewhere in between the heirarchy like :

b(b(func(b(a))));

Regards,
Suyog Sarda 

  

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
Ok. sorry for my silly question above. i tested the code for the question i had, and it seems that destructor of all temporaries 
as well as other objects created before the current line are being called when exception occurs, except the destructor of the 
outermost object. 

From the discussion in the link shared in the previous mail and the results of the test case, i conclude that for 
clang, outermost object temp of class b is not considered constructed until all the inner temporaries are destroyed.

Since the outermost 'temp' is considered yet to be constructed fully, its destructor is not pushed in 'EHStack'. 
As a result whenever exception occurs in the destructor of temporaries, the EHStack does not contain destructor of outermost
object and hence its cleanup doesn't take place. 

Please correct me if i am wrong? Also, while debugging i came across, following piece of code,
 
which after several function calls in between leads me to below (for init-temp1.C test code) :

CodeGenFunction::emitAutoVarTypeCleanup
{
.....
// Use an EH cleanup in array destructors iff the destructor itself
  // is being pushed as an EH cleanup.
  bool useEHCleanup = (cleanupKind & EHCleanup);
  EHStack.pushCleanup<DestroyObject>(cleanupKind, addr, type, destroyer,
                                     useEHCleanup);

}

At this point the destructor is pushed to EHStack. When i traced the code for init-temp1.C, i hit the EHStack.pushcleanup function above, still the destructor of outermost object is not being called.

Can someone please help me if i am looking at right place? If my analysis for clang is right - outermost object not considered created until innermost temporaries are destroyed - where should i look to correct this problem?

If by design, clang doesn't consider outermost object to be created until inner temporaries are destroyed, would this be a resource leak (i don't think it is leak, as the object is yet to be constructed, hence no destruction required)?

Your help would be highly appreciated. 

--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

John McCall
On Apr 21, 2014, at 12:21 PM, suyog sarda <[hidden email]> wrote:
Ok. sorry for my silly question above. i tested the code for the question i had, and it seems that destructor of all temporaries 
as well as other objects created before the current line are being called when exception occurs, except the destructor of the 
outermost object. 

From the discussion in the link shared in the previous mail and the results of the test case, i conclude that for 
clang, outermost object temp of class b is not considered constructed until all the inner temporaries are destroyed.

Since the outermost 'temp' is considered yet to be constructed fully, its destructor is not pushed in 'EHStack'. 
As a result whenever exception occurs in the destructor of temporaries, the EHStack does not contain destructor of outermost
object and hence its cleanup doesn't take place. 

Please correct me if i am wrong? Also, while debugging i came across, following piece of code,
 
which after several function calls in between leads me to below (for init-temp1.C test code) :

CodeGenFunction::emitAutoVarTypeCleanup
{
.....
// Use an EH cleanup in array destructors iff the destructor itself
  // is being pushed as an EH cleanup.
  bool useEHCleanup = (cleanupKind & EHCleanup);
  EHStack.pushCleanup<DestroyObject>(cleanupKind, addr, type, destroyer,
                                     useEHCleanup);

}

At this point the destructor is pushed to EHStack. When i traced the code for init-temp1.C, i hit the EHStack.pushcleanup function above, still the destructor of outermost object is not being called.

Can someone please help me if i am looking at right place? If my analysis for clang is right - outermost object not considered created until innermost temporaries are destroyed - where should i look to correct this problem?

If by design, clang doesn't consider outermost object to be created until inner temporaries are destroyed, would this be a resource leak (i don't think it is leak, as the object is yet to be constructed, hence no destruction required)?

Your help would be highly appreciated. 

If the constructor has completed, the destructor needs to be called later.  IIRC, the problem that makes solving this so annoying is that, as soon as one temporary destructor terminates abnormally, the next thing to destroy is always the local variable (since it was constructed most recently), and only then proceeding to destroy the remainder of the temporaries.

That is, if we’re creating local variable X, and we needed to construct temporaries A, B, and C to do that, then if C’s destructor throws, we need to destroy X, then B, then A; and if B’s destructor throws, we need to destroy X before A.

This is not achievable with a simple stack mechanism.

John.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

Richard Smith
On Mon, Apr 21, 2014 at 1:47 PM, John McCall <[hidden email]> wrote:
On Apr 21, 2014, at 12:21 PM, suyog sarda <[hidden email]> wrote:
Ok. sorry for my silly question above. i tested the code for the question i had, and it seems that destructor of all temporaries 
as well as other objects created before the current line are being called when exception occurs, except the destructor of the 
outermost object. 

From the discussion in the link shared in the previous mail and the results of the test case, i conclude that for 
clang, outermost object temp of class b is not considered constructed until all the inner temporaries are destroyed.

Since the outermost 'temp' is considered yet to be constructed fully, its destructor is not pushed in 'EHStack'. 
As a result whenever exception occurs in the destructor of temporaries, the EHStack does not contain destructor of outermost
object and hence its cleanup doesn't take place. 

Please correct me if i am wrong? Also, while debugging i came across, following piece of code,
 
which after several function calls in between leads me to below (for init-temp1.C test code) :

CodeGenFunction::emitAutoVarTypeCleanup
{
.....
// Use an EH cleanup in array destructors iff the destructor itself
  // is being pushed as an EH cleanup.
  bool useEHCleanup = (cleanupKind & EHCleanup);
  EHStack.pushCleanup<DestroyObject>(cleanupKind, addr, type, destroyer,
                                     useEHCleanup);

}

At this point the destructor is pushed to EHStack. When i traced the code for init-temp1.C, i hit the EHStack.pushcleanup function above, still the destructor of outermost object is not being called.

Can someone please help me if i am looking at right place? If my analysis for clang is right - outermost object not considered created until innermost temporaries are destroyed - where should i look to correct this problem?

If by design, clang doesn't consider outermost object to be created until inner temporaries are destroyed, would this be a resource leak (i don't think it is leak, as the object is yet to be constructed, hence no destruction required)?

Your help would be highly appreciated. 

If the constructor has completed, the destructor needs to be called later.  IIRC, the problem that makes solving this so annoying is that, as soon as one temporary destructor terminates abnormally, the next thing to destroy is always the local variable (since it was constructed most recently), and only then proceeding to destroy the remainder of the temporaries.

That is, if we’re creating local variable X, and we needed to construct temporaries A, B, and C to do that, then if C’s destructor throws, we need to destroy X, then B, then A; and if B’s destructor throws, we need to destroy X before A.

This is not achievable with a simple stack mechanism.

Also of note is http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_active.html#1634, which will introduce a "full expression storage duration" for temporaries. Right now, the destruction order is not wonderfully well-specified; under that change, we'll need to clarify whether X is destroyed before the temporaries, or whether full-expression storage duration objects are destroyed before automatic storage duration objects in all cases (which lets us use a simple stack).

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
Thanks to all. I wrote a test case to see if the constructor of outermost variable completes before destructor of inner temporaries is called.

int as;
struct a {
  a () {  printf("Construct A\n"); }

  ~a () NOEXCEPT_FALSE { printf("Destruct A\n"); throw 42; }
};

int f (a const&) { return 1; }

struct b {
  b (...) { printf("Construct B\n"); }
  ~b ()   { printf("Destruct B\n"); }

  int x(){return 1;}
};

int main () {
  thrown = 0;
  try {
       printf("%d\n",b(f(a())).x());
      }
  catch (...) {}

}

Output with clang :

Construct A
Construct B
1
Destruct B
Destruct A


This indicates that Constructor of outermost object completes before destructor of inner temporaries is called (as it was able to call function x() ).

As per the link provided by Richard, it is not yet clear what should be the order of destructor to be called.
GCC calls destructor of outer local variable as soon as the destructor of any one of the temporaries throws, destructor of other temporaries are called later.
This is not easy to achieve in clang with EHStack approach.

Now as we are not sure of the order of destructors to be called, should we approach with 'full-expression storage duration objects are destroyed before automatic storage duration objects in all cases', which would be achievable with EHStack approach? Because right now, as the destructor of local variable is not called at all (in our original test case 'init-temp1.C'), there is a resource leak. Or should we wait for things to get clear and then only go ahead for solving this bug?


--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda
Hi all,

Finally !! Some interesting findings from my side on why this issue is happening. I will try to explain my findings with pseudo code :

Sample pseudo code and how clang process it :

Consider our code having three class a,b and c

main()
{
    try {
           b tmp1();
           c tmp2(b(a));
     }

   catch {...}
}

A piece of code in clang to focus :

void CodeGenFunction::EmitCXXTryStmt(const CXXTryStmt &S) {
  EnterCXXTryStmt(S);
  EmitStmt(S.getTryBlock());
  ExitCXXTryStmt(S);
}

There is an EHStack, which will contain entries which will be popped out to emit cleanup code (destructor) in case of exception happening. 

When try-catch block is encountered, we visit 'EmitStmt' function, after some more function call we visit following code for emitting code for local variables tmp1 and tmp2:

void CodeGenFunction::EmitAutoVarDecl(const VarDecl &D) {
  AutoVarEmission emission = EmitAutoVarAlloca(D);
  EmitAutoVarInit(emission);
  EmitAutoVarCleanups(emission);
}


First we visit EmitAutoVarInit where some initialization happens by visiting inner expression. For tmp1 object inner expression is empty, so next function EmitAutoVarCleanup will be called and entry of this object tmp1 will be put into EHStack. Since the scope of tmp1 is till end of try block, it will remain in EHStack till try block exits.

Next, We visit second object tmp2 construction, where inner expression consist of temporaries. We visit function EmitAutoVarInit where we process inner expression. Entries of Inner temporary are added into EHStack till all temporary entries are processed, but as soon as the processing of all temporaries gets over i.e. their scope gets over, these entries are popped out of EHstack and their destructor is called as a part of cleanup code. 
Please note we are yet to add entry of tmp2 in EHStack as we are yet to visit EmitAutoVarCleanups function.

Now, The main problem starts - if any of the destructor of temporaries throw, we visit EHStack, pop out each entry from EHStack and emit cleanup code (destructor) for each entry. Note that, the entry of outermost local variable is still not present in the EHStack as we are yet to visit function EmitAutoVarCleanups, as a result of this, its cleanup code (destructor) is not emitted and we land into resource leak!!

I just altered the order of function calls - first call EmitAutoVarCleanups and then EmitAutoVarInit - which ensures that entry of auto variables are added in EHStack before we process inner expressionThis resolves our original issue 'init-temp1.c', however it introduces 10 regressions all related to location of destructors of temporaries. 

This is a resource leak issue and hence i am pressing more on it (Order in which destructors should be called is not clear, but we should eliminate atleast the resource leak). I am not sure although if my approach to resolve this issue is ok. Basically, we somehow need to ensure that auto object entry gets added in EHStack (may be a call to EmitAutoVarCleanups function) before we pop out entries for temporaries.

All your suggestions/comments/rectifications are most awaited.

--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

John McCall

On Apr 23, 2014, at 11:02 AM, suyog sarda <[hidden email]> wrote:

Hi all,

Finally !! Some interesting findings from my side on why this issue is happening. I will try to explain my findings with pseudo code :

Sample pseudo code and how clang process it :

Consider our code having three class a,b and c

main()
{
    try {
           b tmp1();
           c tmp2(b(a));
     }

   catch {...}
}

A piece of code in clang to focus :

void CodeGenFunction::EmitCXXTryStmt(const CXXTryStmt &S) {
  EnterCXXTryStmt(S);
  EmitStmt(S.getTryBlock());
  ExitCXXTryStmt(S);
}

There is an EHStack, which will contain entries which will be popped out to emit cleanup code (destructor) in case of exception happening. 

When try-catch block is encountered, we visit 'EmitStmt' function, after some more function call we visit following code for emitting code for local variables tmp1 and tmp2:

void CodeGenFunction::EmitAutoVarDecl(const VarDecl &D) {
  AutoVarEmission emission = EmitAutoVarAlloca(D);
  EmitAutoVarInit(emission);
  EmitAutoVarCleanups(emission);
}


First we visit EmitAutoVarInit where some initialization happens by visiting inner expression. For tmp1 object inner expression is empty, so next function EmitAutoVarCleanup will be called and entry of this object tmp1 will be put into EHStack. Since the scope of tmp1 is till end of try block, it will remain in EHStack till try block exits.

Next, We visit second object tmp2 construction, where inner expression consist of temporaries. We visit function EmitAutoVarInit where we process inner expression. Entries of Inner temporary are added into EHStack till all temporary entries are processed, but as soon as the processing of all temporaries gets over i.e. their scope gets over, these entries are popped out of EHstack and their destructor is called as a part of cleanup code. 
Please note we are yet to add entry of tmp2 in EHStack as we are yet to visit EmitAutoVarCleanups function.

Now, The main problem starts - if any of the destructor of temporaries throw, we visit EHStack, pop out each entry from EHStack and emit cleanup code (destructor) for each entry. Note that, the entry of outermost local variable is still not present in the EHStack as we are yet to visit function EmitAutoVarCleanups, as a result of this, its cleanup code (destructor) is not emitted and we land into resource leak!!

I just altered the order of function calls - first call EmitAutoVarCleanups and then EmitAutoVarInit - which ensures that entry of auto variables are added in EHStack before we process inner expressionThis resolves our original issue 'init-temp1.c', however it introduces 10 regressions all related to location of destructors of temporaries. 

This is a resource leak issue and hence i am pressing more on it (Order in which destructors should be called is not clear, but we should eliminate atleast the resource leak). I am not sure although if my approach to resolve this issue is ok. Basically, we somehow need to ensure that auto object entry gets added in EHStack (may be a call to EmitAutoVarCleanups function) before we pop out entries for temporaries.

Your proposed change causes the destructor for the auto object to be run if the initializer throws, even if the auto var constructor hasn’t been run yet.

John.

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev
Reply | Threaded
Open this post in threaded view
|

Re: [Bug] Wrong Exception Handling : Destructor not called immediately

suyog sarda


Your proposed change causes the destructor for the auto object to be run if the initializer throws, even if the auto var constructor hasn’t been run yet.


My bad, i got carried away with specifics to the problem and didn't look at whole picture. 

Ok, right now, cleanup code of temporaries is getting emitted before entry of auto variable gets added in the EHStack which is causing resource leak. 
This is because, scope of the temporaries gets over (which causes their cleanup code to emit) after initialization but before adding EHStack entry of auto variable.
If the temporary storage duration is extended till full expression, then it will be easy to use EHStack. 

For auto variable emission there are three steps involved (Correct me if i am wrong) :

1. EmitAlloca - emit code for alloca which will allocate memory
2. EmitInit - emit code for initializing the object which involves evaluating the subexpression
3. EmitCleanup - EmitCleanup code for constructed object

The problem is after allocating memory successfully in 1st step, if something goes wrong in initializing (exception in temporary destructor), the memory allocated in 1st step is not cleaned because it happens in 3rd step which is never visited.  

Is their any other way to resolve this issue? Or only extending storage duration will help in easy implementation?
   
--
With regards,
Suyog Sarda

_______________________________________________
cfe-dev mailing list
[hidden email]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev