#2362 PUI location ignored during compile

ASan will not even let me open any dialog or even menu.

Edit: turns out the problem is that system PLIB was preferred, despite CMake vars pointing to the right one.

Versions:

flightgear 11932bac4 2020-08-24 Trying to fix EnvironmentManager shutdown crash. (origin/next, origin/HEAD, next) [James Turner]
simgear 6f2b81b4 2020-08-24 Fix for the fact we find_package(AAX) all the time. (HEAD -> next, origin/next, origin/HEAD) [James Turner]
fgdata 48fb9ffde 2020-08-24 Updated German translations from Udo Kaune. (origin/next, origin/HEAD, next) [James Turner]

[James Turner]

Aha nice! I'll try to work out a fix :)

[James Turner]

This /might/be a false positive. At least, my version of PLIB, the bug seems like it's fixed. What exact PLIB versikon do you have, and where did you download/install it from?

[Anonymous]

It's ArchLinux PLib, and I can't find it anywhere in Pacman anymore, so it may have been removed, and the old files simply lingering here. From what I can see it was 1.8.5-8 from 2019-02-07 15:56 before they deleted it.

[James Turner]

Okay, can you try Plib from trunk / SVN?

[Anonymous]

Will try.

[Anonymous]

With latest PLib a89f6f8 2012-08-24 Fixing a compile error. Thanks to Alex D-HUND for reporting it. (HEAD -> master, git-svn) [wolfram_kuss]

==174447==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator delete) on 0x602000106910
    #0 0x7ffff7673009 in operator delete(void*, unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x555556576e3e in non-virtual thunk to puInput::setValue(float) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x1022e3e)
    #2 0x555556560d7c in copy_to_pui(SGPropertyNode*, puObject*) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x100cd7c)
    #3 0x55555656c6e2 in FGPUIDialog::update() (/home/mike/var/games/fg/flightgear/bin/fgfs+0x10186e2)
    #4 0x5555565acf62 in NewGUI::update(double) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x1058f62)
    #5 0x55555884c4e5 in SGSubsystemGroup::Member::update(double) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x32f84e5)
    #6 0x55555885de63 in SGSubsystemGroup::update(double) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x3309e63)
    #7 0x55555883f5d2 in SGSubsystemMgr::update(double) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x32eb5d2)
    #8 0x555557c610ba in fgMainLoop() (/home/mike/var/games/fg/flightgear/bin/fgfs+0x270d0ba)
    #9 0x555557a34f32 in fgOSMainLoop() (/home/mike/var/games/fg/flightgear/bin/fgfs+0x24e0f32)
    #10 0x555557c75887 in fgMainInit(int, char**) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x2721887)
    #11 0x555555e10e96 in main (/home/mike/var/games/fg/flightgear/bin/fgfs+0x8bce96)
    #12 0x7ffff4090151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
    #13 0x555555e4e2fd in _start (/home/mike/var/games/fg/flightgear/bin/fgfs+0x8fa2fd)

0x602000106910 is located 0 bytes inside of 6-byte region [0x602000106910,0x602000106916)
allocated by thread T10 here:
    #0 0x7ffff76720c1 in operator new[](unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x555559273abf in chop_to_width /home/mike/var/build/fg/plib/src/pui/puInput.cxx:33

Thread T10 created by T0 here:
    #0 0x7ffff76161c7 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:214
    #1 0x7ffff679ce26 in OpenThreads::Thread::start() (/home/mike/var/games/fg/OpenSceneGraph/lib/libOpenThreads.so.21+0x4e26)

SUMMARY: AddressSanitizer: alloc-dealloc-mismatch /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172 in operator delete(void*, unsigned long)
==174447==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==174447==ABORTING

[James Turner]

Okay the fix was made in 2008, so not sure what is going on here. Can you confirm that line 1474 of your pu.h looks like ' delete [] displayed_text ; displayed_text = NULL ;' ?

[Anonymous]

Yes:

  virtual void setValue ( float f )
  {
    puValue::setValue ( f ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }

[James Turner]

Okay, it's weird then: the ASan error is saying 'you used delete when you should have used delete[]' .. but we /are/. using delete[], if I'm reading everything correctly.

[James Turner]

Can you repeat with a debug build and see if chanegs the backrtrace information? Becuase the function is in a header, I wonder if something weird is going on

[Anonymous]

It looks the same with

CMAKE_C_FLAGS:STRING=-march=native -O3 -pipe -g
CMAKE_CXX_FLAGS:STRING=-march=native -O3 -pipe -g --std=c++11

==268275==ERROR: AddressSanitizer: alloc-dealloc-mismatch (operator new [] vs operator delete) on 0x602002d99870
    #0 0x7ffff7673009 in operator delete(void*, unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172
    #1 0x555556559f5e in non-virtual thunk to puInput::setValue(float) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x1005f5e)
    #2 0x555556543e9c in copy_to_pui /home/mike/src/git/flightgear/flightgear/src/GUI/FGPUIDialog.cxx:618
    #3 0x55555654f802 in FGPUIDialog::update() /home/mike/src/git/flightgear/flightgear/src/GUI/FGPUIDialog.cxx:779
    #4 0x555556590082 in NewGUI::update(double) /home/mike/src/git/flightgear/flightgear/src/GUI/new_gui.cxx:198
    #5 0x55555884c2b5 in SGSubsystemGroup::Member::update(double) /home/mike/src/git/flightgear/simgear/simgear/structure/subsystem_mgr.cxx:819
    #6 0x55555885dc33 in SGSubsystemGroup::update(double) /home/mike/src/git/flightgear/simgear/simgear/structure/subsystem_mgr.cxx:403
    #7 0x55555883f3a2 in SGSubsystemMgr::update(double) /home/mike/src/git/flightgear/simgear/simgear/structure/subsystem_mgr.cxx:961
    #8 0x555557c511ea in fgMainLoop /home/mike/src/git/flightgear/flightgear/src/Main/main.cxx:161
    #9 0x555557a21242 in fgOSMainLoop() /home/mike/src/git/flightgear/flightgear/src/Viewer/fg_os_osgviewer.cxx:309
    #10 0x555557c659b7 in fgMainInit(int, char**) /home/mike/src/git/flightgear/flightgear/src/Main/main.cxx:751
    #11 0x555555e10d86 in main /home/mike/src/git/flightgear/flightgear/src/Main/bootstrap.cxx:313
    #12 0x7ffff4090151 in __libc_start_main (/usr/lib/libc.so.6+0x28151)
    #13 0x555555e4e2fd in _start (/home/mike/var/games/fg/flightgear/bin/fgfs+0x8fa2fd)

0x602002d99870 is located 0 bytes inside of 6-byte region [0x602002d99870,0x602002d99876)
allocated by thread T10 here:
    #0 0x7ffff76720c1 in operator new[](unsigned long) /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x55555927388f in chop_to_width /home/mike/var/build/fg/plib/src/pui/puInput.cxx:33

Thread T10 created by T0 here:
    #0 0x7ffff76161c7 in __interceptor_pthread_create /build/gcc/src/gcc/libsanitizer/asan/asan_interceptors.cpp:214
    #1 0x7ffff679ce26 in OpenThreads::Thread::start() /home/mike/src/git/flightgear/OpenSceneGraph/src/OpenThreads/pthreads/PThread.cpp:698

SUMMARY: AddressSanitizer: alloc-dealloc-mismatch /build/gcc/src/gcc/libsanitizer/asan/asan_new_delete.cpp:172 in operator delete(void*, unsigned long)
==268275==HINT: if you don't care about these errors you may set ASAN_OPTIONS=alloc_dealloc_mismatch=0
==268275==ABORTING

The usual ones were:

CMAKE_C_FLAGS:STRING=-march=native -O3 -pipe
CMAKE_CXX_FLAGS:STRING=-march=native -O3 -pipe --std=c++11

Should I try checking with -O0, -O1 or -O2?

[Anonymous]

But it's saying new [] is mismatched with delete not with delete []? I don't know what that even means but I assume they both should have the [] (is that some kind of pointer thing)?

[James Turner]

Yes it's a weird pointer thing, but the important point is if you use new[], you need to use delete[] as well. And as far as I can see from the code, we are. The error is saying 'you used new[], but used delete /without/ []'. But you checked and the header you showed me has the version with []. So I'm confused.

Can you try -g with -O0 ? That might make it clearer.

[James Turner]

I'd especially like to know a file+line number location for ' #1 0x555556559f5e in non-virtual thunk to puInput::setValue(float) (/home/mike/var/games/fg/flightgear/bin/fgfs+0x1005f5e)
' - since right now we can't tell, and I'd like to be 100% sure it's using the pu.h we think it is.

[Edward d'Auvergne]

I see a major difference between my distro supplied pu.h (version 1.8.5) and the HEAD r2173 version, which is very, very likely to be the source of this issue. Here is the trunk version of the puInput class:

class puInput : public puInputBase, public puObject
{
  UL_TYPE_DATA

  int display_starting_point ;

  char *getDisplayedText ( void )
  {
    return ( displayed_text == NULL ? getStringValue () : displayed_text ) ;
  }

public:
  void draw     ( int dx, int dy ) ;
  void doHit    ( int button, int updown, int x, int y ) ;
  int  checkKey ( int key, int updown ) ;

  void invokeDownCallback ( void )
  {
    rejectInput () ;
    normalizeCursors () ;
    if ( down_cb != NULL ) (*down_cb)(this) ;
  }

  puInput ( int minx, int miny, int maxx, int maxy ) :
             puInputBase (), puObject ( minx, miny, maxx, maxy )
  {
    type |= PUCLASS_INPUT ;

    display_starting_point = 0 ;

    setColourScheme ( colour [ PUCOL_EDITFIELD ][0],
                      colour [ PUCOL_EDITFIELD ][1],
                      colour [ PUCOL_EDITFIELD ][2],
                      colour [ PUCOL_EDITFIELD ][3] ) ;
    setColour ( PUCOL_MISC, 0.1f, 0.1f, 1.0f ) ; /* Colour of 'I' bar cursor */

    widget = this ;
  }

  virtual void setValue ( puValue *pv )
  {
    puValue::setValue ( pv ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( int   i )
  {
    puValue::setValue ( i ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( float f )
  {
    puValue::setValue ( f ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( const char *s )
  {
    puValue::setValue ( s ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( bool b )
  {
    puValue::setValue ( b ) ;
    delete [] displayed_text ;  displayed_text = NULL ;
  }
} ;

And then the 1.8.5 version:

class puInput : public puInputBase, public puObject
{
  UL_TYPE_DATA

  int display_starting_point ;

  char *getDisplayedText ( void )
  {
    return ( displayed_text == NULL ? getStringValue () : displayed_text ) ;
  }

public:
  void draw     ( int dx, int dy ) ;
  void doHit    ( int button, int updown, int x, int y ) ;
  int  checkKey ( int key, int updown ) ;

  void invokeDownCallback ( void )
  {
    rejectInput () ;
    normalizeCursors () ;
    if ( down_cb != NULL ) (*down_cb)(this) ;
  }

  puInput ( int minx, int miny, int maxx, int maxy ) :
             puInputBase (), puObject ( minx, miny, maxx, maxy )
  {
    type |= PUCLASS_INPUT ;

    display_starting_point = 0 ;

    setColourScheme ( colour [ PUCOL_EDITFIELD ][0],
                      colour [ PUCOL_EDITFIELD ][1],
                      colour [ PUCOL_EDITFIELD ][2],
                      colour [ PUCOL_EDITFIELD ][3] ) ;
    setColour ( PUCOL_MISC, 0.1f, 0.1f, 1.0f ) ; /* Colour of 'I' bar cursor */

    widget = this ;
  }

  virtual void setValue ( puValue *pv )
  {
    puValue::setValue ( pv ) ;
    delete displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( int   i )
  {
    puValue::setValue ( i ) ;
    delete displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( float f )
  {
    puValue::setValue ( f ) ;
    delete displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( const char *s )
  {
    puValue::setValue ( s ) ;
    delete displayed_text ;  displayed_text = NULL ;
  }

  virtual void setValue ( bool b )
  {
    puValue::setValue ( b ) ;
    delete displayed_text ;  displayed_text = NULL ;
  }
} ;

Note the delete displayed_text; having been fixed as delete [] displayed_text;. The SVN commit that fixed this is:

------------------------------------------------------------------------
r2136 | fayjf | 2008-12-18 05:19:29 +0100 (Thu, 18 Dec 2008) | 1 line

Fixing the delete display_text statement in puInputBase (e-mail from Melchior Franz, Wed 12/17/2008 6:12 PM)

Index: src/pui/pu.h
===================================================================
--- src/pui/pu.h        (revision 2135)
+++ src/pui/pu.h        (revision 2136)
@@ -1445,31 +1445,31 @@
   virtual void setValue ( puValue *pv )
   {
     puValue::setValue ( pv ) ;

-    delete displayed_text ;  displayed_text = NULL ;
+    delete [] displayed_text ;  displayed_text = NULL ;
   }

   virtual void setValue ( int   i )
   {
     puValue::setValue ( i ) ;

-    delete displayed_text ;  displayed_text = NULL ;
+    delete [] displayed_text ;  displayed_text = NULL ;
   }

   virtual void setValue ( float f )
   {
     puValue::setValue ( f ) ;

-    delete displayed_text ;  displayed_text = NULL ;
+    delete [] displayed_text ;  displayed_text = NULL ;
   }

   virtual void setValue ( const char *s )
   {
     puValue::setValue ( s ) ;

-    delete displayed_text ;  displayed_text = NULL ;
+    delete [] displayed_text ;  displayed_text = NULL ;
   }

   virtual void setValue ( bool b )
   {
     puValue::setValue ( b ) ;

-    delete displayed_text ;  displayed_text = NULL ;
+    delete [] displayed_text ;  displayed_text = NULL ;
   }
 } ;


------------------------------------------------------------------------

For reference, Melchior Franz was a FlightGear developer. And the 1.8.5 release that distros use was committed at:

------------------------------------------------------------------------
r2131 | sjbaker | 2008-03-11 03:23:50 +0100 (Tue, 11 Mar 2008) | 3 lines

Changed version numbers for release 1.8.5


------------------------------------------------------------------------

So clearly a FlightGear ASan build cannot use the system PLIB.

[Edward d'Auvergne]

And the message from Melchior Franz to the PLIB developers:

Re: [Plib-devel] [PATCH] delete -> delete []
From: John F. Fay johnffay@cy... - 2008-12-18 04:28:30

Arrgggh ... sorry about that.

I've put it into SVN.

• John

-----Original Message-----
From: Melchior FRANZ
Sent: Wednesday, December 17, 2008 6:12 PM
To: plib-devel@...
Subject: [Plib-devel] [PATCH] delete -> delete []

The puInput class wrongly uses "delete" on displayed_text,
which was allocated with "new char[]" (puInput.cxx:33).
It's mostly a cosmetic problem but upsets debuggers like
valgrind.

m.
<< File: pu.diff; charset = iso 8859-15 >> << File: ATT00045.txt >> << File: ATT00046.txt >>

[Anonymous]

I will try getting it to crash with -O0 -pipe -g

[Anonymous]

Strange, but O2 and O0 look the same.

[Anonymous]

Sorry about the confusion, I know what's going on now. It was still including the old plib from the OS that was left over from the package that had disappeared.

I moved /usr/include/plib away to see, and it stopped building, because it was still taking those despite the PLIB_INCLUDE_DIR:PATH=/home/mike/var/games/fg/plib/include in my CMakeCache.txt after having used PLIBDIR="${PLIB}" with cmake.

[Anonymous]

After moving system PLIB includes and libraries away and adding the

   -DCMAKE_C_FLAGS="-I${PLIB}/include" \
   -DCMAKE_CXX_FLAGS="-I${PLIB}/include" \

it compiled, and the issue is gone. So the problem was CMake not obeying the set PLib location.

$ grep plib CMakeCache.txt 
CMAKE_CXX_FLAGS:STRING=-march=native -O3 -pipe -g --std=c++11
CMAKE_C_FLAGS:STRING=-march=native -O3 -pipe -g
PLIB_INCLUDE_DIR:PATH=/home/mike/var/games/fg/plib/include
PLIB_PUAUX_LIBRARY:FILEPATH=optimized;/home/mike/var/games/fg/plib/lib/libplibpuaux.a;debug;/home/mike/var/games/fg/plib/lib/libplibpuaux.a
PLIB_PUAUX_LIBRARY_RELEASE:FILEPATH=/home/mike/var/games/fg/plib/lib/libplibpuaux.a
PLIB_PU_LIBRARY:FILEPATH=optimized;/home/mike/var/games/fg/plib/lib/libplibpu.a;debug;/home/mike/var/games/fg/plib/lib/libplibpu.a
PLIB_PU_LIBRARY_RELEASE:FILEPATH=/home/mike/var/games/fg/plib/lib/libplibpu.a
PLIB_SG_LIBRARY:FILEPATH=optimized;/home/mike/var/games/fg/plib/lib/libplibsg.a;debug;/home/mike/var/games/fg/plib/lib/libplibsg.a
PLIB_SG_LIBRARY_RELEASE:FILEPATH=/home/mike/var/games/fg/plib/lib/libplibsg.a
PLIB_UL_LIBRARY:FILEPATH=optimized;/home/mike/var/games/fg/plib/lib/libplibul.a;debug;/home/mike/var/games/fg/plib/lib/libplibul.a
PLIB_UL_LIBRARY_RELEASE:FILEPATH=/home/mike/var/games/fg/plib/lib/libplibul.a
FIND_PACKAGE_MESSAGE_DETAILS_PLIB:INTERNAL=[optimized;/home/mike/var/games/fg/plib/lib/libplibpuaux.a;debug;/home/mike/var/games/fg/plib/lib/libplibpuaux.a;optimized;/home/mike/var/games/fg/plib/lib/libplibpu.a;debug;/home/mike/var/games/fg/plib/lib/libplibpu.a;optimized;/home/mike/var/games/fg/plib/lib/libplibsg.a;debug;/home/mike/var/games/fg/plib/lib/libplibsg.a;optimized;/home/mike/var/games/fg/plib/lib/libplibul.a;debug;/home/mike/var/games/fg/plib/lib/libplibul.a][/home/mike/var/games/fg/plib/include][v()]

[Anonymous]

Edited bug description due to changed nature of the bug.

[Erik Hofman]

I've now pushed an update to use our own version of pu.h which should fix this issue.