Visual D 0.50.0-beta1 released

[Rainer Schuetze]

On 28/07/2019 06:59, Bert wrote:
> On Wednesday, 24 July 2019 at 15:32:01 UTC, Rainer Schuetze wrote:
>>
>>
>> On 24/07/2019 08:33, Bert wrote:
>>> On Tuesday, 23 July 2019 at 20:57:17 UTC, Rainer Schuetze wrote
>>>> As it turned out, the problem was (again?) a false entry in the class
>>>> name cache that was added when an invalid pointer (including null) was
>>>> evaluated (and its dynamic was tried to be determined). Stupid bug, but
>>>> that happens.
>>>>
>>>> You can try the fixed version of MagoNatCC.dll from
>>>> https://ci.appveyor.com/project/rainers/mago/build/artifacts
>>>
>>> That worked! I still find it very odd that it would exhibit it with
>>> irrelevant lines of code removed. Was that changing the cached
>>> version or something? Remember I basically have several Q's in my
>>> program all virtually identical and some would work and others wouldn't.
>>>
>>> Could you post some of the code that you fixed(or I guess push it to
>>> master) when you get a chance... I'm just curious to what it looks
>>> like. While such bugs are very annoying and better coding methods
>>> should be used, I imagine that this type of scenario for interface
>>> resolution is probably well localized and this would probably be the
>>> last bug fix for it? If not maybe some logging could be implemented.
>>> Interfaces should always be resolvable so if the code fails it is a
>>> bug in Visual D and having it logged in some way might help future
>>> cases.
>>
>> The bugfix commit is
>> https://github.com/rainers/mago/commit/9801da089a4b4e9ffa8f1442f4b0aff5852d5fa9?w=1.
>>
>>
>> The problem was that evaluating the dynamic type of any uninitialized
>> or null class/interface pointer could overwrite a previously evaluated
>> entry in the vtbl->class name cache depending on what happens to be in
>> an uninitialized local variable.
>>
>> Hopefully the last bug in that area, but you never know. Adding
>> logging would help diagnosing bugs but would have to be more global.
> 
> Thanks.
> 
> I can't imagine though why adding a simple field would screw the code up.
> 
> e.g.,
> 
> class X {} // works.
> class Y { int y; } // fails.
> 

This wouldn't have failed, int needs a class/interface member, e.g.

class X
{
   Q q; // non-null
   I c; // null
   Q s; // non-null
}

> I would think that such things would have no effect on the pointer value?
> 
> Can you explain to me, for my own edification, why removing those
> non-essential lines would produce the effect?
> 

The bug basically was similar to

// global cache
string[void*] mapVtblToClassName;

void showX(X x)
{
   void* vtbl;
   string classNameq;
   bool rc = ReadProcessMemory(x.q, vtbl); // reads vtbl-ptr
   if (rc)
   {
      if (vtbl in mapVtblToClassName)
      {
         classNameq = mapVtblToClassName[vtbl];
         goto Lnext;
      }
      classNameq = getClassNameFromVtbl(vtbl); // Q
   }
   mapVtblToClassName[vtbl] = classNameq;

Lnext:
   string classNamec;
   rc = ReadProcessMemory(x.c, vtbl); // fails, vtbl unchanged
   if (rc)
   {
      // same as above...
   }
   mapVtblToClassName[vtbl] = classNamec; // overwrites entry for Q
}

Next time a Q is evaluated, the determined class name will be empty.