gdl icon indicating copy to clipboard operation
gdl copied to clipboard

Hash crash

Open fawltylanguage opened this issue 1 year ago • 11 comments

The following program crashes in the latest git version:

pro s::x
  self['x']=123
  print, self['x']
end

pro test
  s={s, i:0, inherits hash, j:0}
  o=obj_new('s')
  o->x
end
IDL> test
% Compiled module: TEST.
     123

GDL> test
% Compiled module: TEST.
Segmentation fault (core dumped)

fawltylanguage avatar May 05 '23 17:05 fawltylanguage

Using Version 1.0.2 Git (macOS) compiled 26/04/23, doesn't crash but returns:

InterpreterLoop: Unhandled Error.

Barney-9000 avatar May 07 '23 09:05 Barney-9000

Crash on all versions (>=1.0.1) I tested. When I compiled current Git version with -DCMAKE_CXX_FLAGS="-fsanitize=null" -DCMAKE_BUILD_TYPE=Debug I receive useful information !

GDL> test
gdl: /home/alaingdl/GDL/gdl-1.0.2git230507CMake/src/gdlarray.hpp:98: T& GDLArray<T, IsPOD>::operator[](SizeT) [with T = long long unsigned int; bool IsPOD = true; SizeT = long long unsigned int]: Assertion `ix < sz' failed.

alaingdl avatar May 07 '23 16:05 alaingdl

Hi, is it a regression or a newly-found bug?

GillesDuvert avatar May 15 '23 08:05 GillesDuvert

Probably new, I never tried this before with GDL.

fawltylanguage avatar May 15 '23 21:05 fawltylanguage

Hi, of course GDL hangs on this (this is a bug --- GDL should throw gracefully at least). However, to really support this, GDL programmers must understand what is going on here. I get the impression that this mix of object and structures is working only because of a loophole in IDL's programming and does not fit the documentation. Let me explain:

  • for me, o=obj_new('s') needs that a s__define.pro exist (or, minimally mimicked by IDL internals) . In the present case, GDL apparently does not do this minimum.
  • But, let's do the same thing with a s__define. I'm not expert but I would write this:
pro s::x
  self['x']=123
  print, self['x']
end
function s::init
  return,1
end
pro s__define
  s={s, i:0, inherits hash, j:0}
end

as a s__define.pro file .

But then I get:

IDL> o=obj_new('s')
% Compiled module: S__DEFINE.
IDL> o->x          
% HASH::SEARCH: Invalid pointer: <NullPointer>.
% Execution halted at: S::X                2 /tmp/s__define.pro
%                      $MAIN$          
IDL> 

In fact, GDL hangs just because it does not check that the pointer was invalid before using it --> correctable.

So, my naive question, why should the method 's::x' work in your example case and not in the bona fide object case above?

GillesDuvert avatar May 25 '23 13:05 GillesDuvert

Naively, again, one is tempted, as s inherits 'hash' to have s behave like a hash and thus do things like

IDL> s={s, i:0, inherits hash, j:0}                       
IDL> s['x']=123
% Type conversion error: Unable to convert given STRING to Long64.
% Detected at: $MAIN$          
% Conflicting data structures: S,<INT      (     123)>.
% Execution halted at: $MAIN$ 

that fail miserably of course since s is not a hash it is just a structure that contains the hash tags. But no more. That's why I fail to see why any hash function would work on it...

GillesDuvert avatar May 25 '23 13:05 GillesDuvert

For the programmer's record, the crash is because hash.cpp uses static addresses for finding bona fide hash tags, like: static unsigned pTableTag = structDesc::HASH->TagIndex( "TABLE_DATA"); where in the present case, 'self' is not a basic hash table struct (i:0 in the definition of s above shifts the tag number by 1) and it would be safer to have unsigned pTableTag = self->Desc()->TagIndex( "TABLE_DATA"); and the likes in every places where 'self' may not be a 'basic' hash (or list, btw) structure. In other terms, be more robust...

GillesDuvert avatar May 25 '23 13:05 GillesDuvert

Hi, of course GDL hangs on this (this is a bug --- GDL should throw gracefully at least). However, to really support this, GDL programmers must understand what is going on here. I get the impression that this mix of object and structures is working only because of a loophole in IDL's programming and does not fit the documentation. Let me explain:

* for me, `o=obj_new('s')` needs that a `s__define.pro` exist (or, minimally mimicked by IDL internals) . In the present case, GDL apparently does not do this minimum.

* But, let's do the same thing with a s__define. I'm not expert but I would write this:
pro s::x
  self['x']=123
  print, self['x']
end
function s::init
  return,1
end
pro s__define
  s={s, i:0, inherits hash, j:0}
end

as a s__define.pro file .

But then I get:

IDL> o=obj_new('s')
% Compiled module: S__DEFINE.
IDL> o->x          
% HASH::SEARCH: Invalid pointer: <NullPointer>.
% Execution halted at: S::X                2 /tmp/s__define.pro
%                      $MAIN$          
IDL> 

In fact, GDL hangs just because it does not check that the pointer was invalid before using it --> correctable.

So, my naive question, why should the method 's::x' work in your example case and not in the bona fide object case above?

Your s__define.pro is wrong. Just remove s::init and it will work in IDL.

fawltylanguage avatar May 25 '23 15:05 fawltylanguage

@fawltylanguage thanks, indeed it works without 'init'. Still at loss on how IDL takes a struct inheriting a hash, and accepts hash commands on it.

GillesDuvert avatar May 25 '23 16:05 GillesDuvert

Object inheritance and operator overloading.

Every named structure is also a potential object class. You can create object instances from an object class with obj_new(). In test.pro, s is a named structure/object class, hash is its superclass, and the o object is an instance of the class . Objects may have overloaded methods, like _overloadBracketsLeftSide and _overloadBracketsRightSide here. When IDL encounters the brackets with o, it looks up these overloaded methods. s has none, but the superclass hash has both of them, so these are called.

fawltylanguage avatar May 25 '23 16:05 fawltylanguage

Thanks your 5 lines of explanation are more clear than many pages of the IDL documentation! Yes, the crash was (is) in _overloadBracketsLeftSide indeed, so the whole "named structure"->"object" processing in GDL works well. This is, so to say, under control. I have yet to find where to convince GDL that s::x() is something to do when one writes o->x ...

GillesDuvert avatar May 26 '23 08:05 GillesDuvert