[D-runtime] win32 regression in exception handling

[Don Clugston]

On 26 January 2011 09:33, Jonathan M Davis <jmdavisProg at gmx.com> wrote:
> On Wednesday 26 January 2011 00:25:34 Brad Roberts wrote:
>> On 1/24/2011 8:00 PM, Brad Roberts wrote:
>> > Now that the dust of the svn -> git migration has settled some, there's a
>> > failure that's stuck with us.  Since this druntime change, the win32 dmd
>> > test34.d test has failed:
>> >
>> > https://github.com/D-Programming-Language/druntime/commit/559df80b3d9c512
>> > 72c5804de8df27af75099b977
>> >
>> > http://d.puremagic.com/test-results/test_data.ghtml?dataid=43441
>> >
>> >   Before test 3:
>> >   object.Error at src\rt\deh.d(631): Access Violation
>> >
>> > I did a test build at the previous commit and the test passes.  So that's
>> > definitely the one that introduces the problem.  It might well be just
>> > exposing a dormant problem, but either way, it stopped working at that
>> > point.
>>
>> Ok.. I finally took some time to dive into this one.  It's failing on a
>> catch (DerivedFromError) situation.
>>
>> From test34.d:
>>
>> class B58
>> {    long x;
>>      void set(long i) { writeln("B58::set(long)"); x = i; }
>>      void set(int i)  { writeln("B58::set(int)"); x = i; }
>>      long squareIt()  { writeln("B58::squareit()"); return x * x; }
>> }
>> class D58 : B58
>> {
>>      long square;
>>      void set(long i) { writeln("D58::set(long)"); B58.set(i); square = x *
>> x; } long squareIt()  { writeln("D58::squareit()"); return square; } }
>>
>> long foo58(B58 b)
>> {   int i;
>>     try
>>     {
>>         b.set(3);
>>     }
>>     catch (HiddenFuncError o)
>>     {
>>         i = 1;
>>     }
>>     assert(i == 1);
>>     return b.squareIt();
>> }
>>
>> void test58()
>> {
>>     writeln(foo58(new D58));
>> }
>>
>> Catching Error derived classes is illegal-ish now, right?  I haven't
>> narrowed it down to exactly where the segv is coming from (the line number
>> above is the part of the eh mechanism that's constructing the error object
>> in the SEH receiving code, not the point of the segv.. unless that line of
>> code is segving somehow.
>>
>> Don, do you have the time to dig a little since you've been all over that
>> part of the code recently?
>
> As I understand it, catching Errors is perfectly legal, but because none of the
> proper cleanup is going to have been done between the point that Error was
> thrown from and the point that it was caught it, the state of things might be a
> bit messed up. The unit testing functions that I wrote which are currently under
> review catch Errors, and they'd be somewhat crippled if they couldn't.


>
> Catching Errors is not generally a good idea, but as far as I understand it,
> it's supposed to be legal.

Yes, that's correct, it's legal to catch errors.
There is an obvious disastrous problem with that commit: the inclusion
of __FILE__ and __LINE__ in the constructors for Error and Throwable
is completely wrong. They should not be in there. That assumes that
the exceptions are thrown via a throw statement, which is not correct,
and will cause serious problems. Dunno if that's the only problem,
though.
Sorry, don't have any more time right now.