bsnes 0.020 crashes upon "Load cartridge" [solved]

belegdol · Post by **belegdol** » Mon Jun 04, 2007 1:56 pm

I have a shiny new Fedora 7 here. Compiled the latest bzip2 tarball. The backtrace is as follows:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 46912496349520 (LWP 24514)]
0x0000003852877180 in strlen () from /lib64/libc.so.6
(gdb) bt full
#0 0x0000003852877180 in strlen () from /lib64/libc.so.6
No symbol table info available.
#1 0x0000003857845f73 in g_strdup () from /lib64/libglib-2.0.so.0
No symbol table info available.
#2 0x000000385882e778 in __cxa_pure_virtual () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#3 0x0000003858810026 in g_object_new_valist ()
from /lib64/libgobject-2.0.so.0
No symbol table info available.
#4 0x0000003858810211 in g_object_new () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#5 0x000000385b474705 in gtk_button_new_from_stock ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#6 0x000000385b4babbf in gtk_dialog_add_button ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#7 0x000000385b4de98a in __cxa_pure_virtual ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#8 0x000000385b4deb05 in gtk_file_chooser_dialog_new ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
---Type <return> to continue, or q <return> to quit---
#9 0x00000000004161b6 in libui::gtk_file_load ()
No symbol table info available.
#10 0x0000000000416269 in libui::file_load ()
No symbol table info available.
#11 0x000000000040d5f1 in event::load_rom ()
No symbol table info available.
#12 0x000000000040d740 in event::load_rom ()
No symbol table info available.
#13 0x000000000040e0c2 in MainWindow::message ()
No symbol table info available.
#14 0x000000385880af19 in g_closure_invoke () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#15 0x000000385881a788 in __cxa_pure_virtual () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#16 0x000000385881bbd4 in g_signal_emit_valist ()
from /lib64/libgobject-2.0.so.0
No symbol table info available.
#17 0x000000385881bda3 in g_signal_emit () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#18 0x000000385b63474a in gtk_widget_activate ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#19 0x000000385b53e570 in gtk_menu_shell_activate_item ()
---Type <return> to continue, or q <return> to quit---
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#20 0x000000385b53fa20 in __cxa_pure_virtual ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#21 0x000000385b53276d in __cxa_pure_virtual ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#22 0x000000385880af19 in g_closure_invoke () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#23 0x000000385881ad98 in __cxa_pure_virtual () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#24 0x000000385881b99d in g_signal_emit_valist ()
from /lib64/libgobject-2.0.so.0
No symbol table info available.
#25 0x000000385881bda3 in g_signal_emit () from /lib64/libgobject-2.0.so.0
No symbol table info available.
#26 0x000000385b630a7e in __cxa_pure_virtual ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#27 0x000000385b52bd5d in gtk_propagate_event ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
---Type <return> to continue, or q <return> to quit---
#28 0x000000385b52cd71 in gtk_main_do_event ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#29 0x0000003859c4687c in __cxa_pure_virtual ()
from /usr/lib64/libgdk-x11-2.0.so.0
No symbol table info available.
#30 0x000000385782d1f4 in g_main_context_dispatch ()
from /lib64/libglib-2.0.so.0
No symbol table info available.
#31 0x000000385783002d in __cxa_pure_virtual () from /lib64/libglib-2.0.so.0
No symbol table info available.
#32 0x000000385783055e in g_main_context_iteration ()
from /lib64/libglib-2.0.so.0
No symbol table info available.
#33 0x000000385b52cf5d in gtk_main_iteration_do ()
from /usr/lib64/libgtk-x11-2.0.so.0
No symbol table info available.
#34 0x0000000000419a3b in libui::run ()
No symbol table info available.
#35 0x00000000004139a7 in main ()
No symbol table info available.

byuu · Post by **byuu** » Mon Jun 04, 2007 4:39 pm

Shit, that bug is popping up again ... I thought it was FreeBSD-specific.

Basically, whenever I call gtk_file_chooser_dialog_new, it seems to jump deeply into a million GTK+ internal library functions and die. My 'workaround' on FreeBSD was to call the function inside another function. Bizarre, I know, but it worked ...

This is the relevent code:

Code: Select all

//FreeBSD 6.2-amd64 bug workaround for file_load() + file_save()
//gdb reveals stack corruption when calling gtk_file_chooset_dialog_new() from inside
//a member function. however, calling function with nesting sidesteps the bug ...
//gdb shows that the corruption occurs inside a GTK+ internal library function ...

bool file_load(Window &owner, char *filename, const char *filter, const char *path) {
  return gtk_file_load(owner, filename, filter, path);
}

bool file_save(Window &owner, char *filename, const char *filter, const char *path) {
  return gtk_file_save(owner, filename, filter, path);
}

noinline bool gtk_file_load(Window &owner, char *filename, const char *filter, const char *path) {
  strcpy(filename, "");

GtkWidget *dialog = gtk_file_chooser_dialog_new("Load File",
  GTK_WINDOW(owner.info.window), GTK_FILE_CHOOSER_ACTION_OPEN,
  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
  GTK_STOCK_OPEN, GTK_RESPONSE_ACCEPT, 0);

  if(path && strcmp(path, "")) {
    gtk_file_chooser_set_current_folder(GTK_FILE_CHOOSER(dialog), path);
  }

  if(gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
  char *fn = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
    strcpy(filename, fn);
    g_free(fn);
  }

  gtk_widget_destroy(dialog);
  return strcmp(filename, ""); //return true if filename != ""
}

noinline bool gtk_file_save(Window &owner, char *filename, const char *filter, const char *path) {
  strcpy(filename, "");

GtkWidget *dialog = gtk_file_chooser_dialog_new("Save File",
  GTK_WINDOW(owner.info.window), GTK_FILE_CHOOSER_ACTION_SAVE,
  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
  GTK_STOCK_SAVE, GTK_RESPONSE_ACCEPT, 0);

  if(path && strcmp(path, "")) {
    gtk_file_chooser_set_current_folder(GTK_FILE_CHOOSER(dialog), path);
  }
  gtk_file_chooser_set_do_overwrite_confirmation(GTK_FILE_CHOOSER(dialog), TRUE);

  if(gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
  char *fn = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
    strcpy(filename, fn);
    g_free(fn);
  }

  gtk_widget_destroy(dialog);
  return strcmp(filename, ""); //return true if filename != ""
}

Unfortunately, it's crashing with valid parameters to gtk_file_chooser_dialog_new. Therefore, it's almost certainly a bug in GTK+. I do not know how to fix it, I am sorry.

I really need the help of a more experienced Linux programmer here ...

belegdol · Post by **belegdol** » Mon Jun 04, 2007 4:54 pm

Bummer. Is it possible to load a ROM without using the menu then? I tried command line but did not succeed.

byuu · Post by **byuu** » Mon Jun 04, 2007 5:06 pm

Oops, that's quite a large oversight ... I forgot to add that.

Ok, please go to src/ui/lui/main.cpp, and add:

Code: Select all

  if(argc >= 2) {
    cartridge.load_begin(Cartridge::CART_NORMAL);
    cartridge.load(argv[1]);
    cartridge.load_end();
    snes.power();
  }

Immediately after ui_init(); and before while(_term_ == false) { inside main().

You should have this for a main function (plus the platform-independent trickery above it):

Code: Select all

//int main(int argc, char *argv[]) {
  set_config_filename();

  ui::init();
  config_file.load(config::filename);
  config_file.save(config::filename); //in case program crashes on first run, config file settings can be modified
  init_snes();
  ui_init();

  if(argc >= 2) {
    cartridge.load_begin(Cartridge::CART_NORMAL);
    cartridge.load(argv[1]);
    cartridge.load_end();
    snes.power();
  }

  while(_term_ == false) {
    while(ui::events_pending() == true) { ui::run(); }
    if(cartridge.loaded() == true) {
      snes.runtoframe();
      event::update_frame_counter();
    }
  }

  if(cartridge.loaded() == true) { cartridge.unload(); }

  config_file.save(config::filename);
  term_snes();
  ui_term();
  ui::term();
  return 0;
}

Please let me know if things work out for you. Xv is going to be the next 'issue' ... hopefully that part will work ok.

byuu · Post by **byuu** » Mon Jun 04, 2007 6:36 pm

I believe I have a fix, I need someone who is experiencing this bus to test it for me, please.

I was reading:
http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=319031

kino had the same problem. They fixed it with this:
http://err.no/patches/kino_segv_on_open_64bit.diff

Let's try the same fix on bsnes:

Please go to src/lib/libui_gtk.cpp, search for 'gtk_file_chooser_dialog_new'

You will find two instances, one in gtk_file_save, and one in gtk_file_load.

Please change the last parameter of the function call from 0 to (const gchar *) NULL, and let me know if it works.

For reference, the updated two functions are:

Code: Select all

noinline bool gtk_file_load(Window &owner, char *filename, const char *filter, const char *path) {
  strcpy(filename, "");

GtkWidget *dialog = gtk_file_chooser_dialog_new("Load File",
  GTK_WINDOW(owner.info.window), GTK_FILE_CHOOSER_ACTION_OPEN,
  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
  GTK_STOCK_OPEN, GTK_RESPONSE_ACCEPT, (const gchar*)NULL); //<- last param modified

  if(path && strcmp(path, "")) {
    gtk_file_chooser_set_current_folder(GTK_FILE_CHOOSER(dialog), path);
  }

  if(gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
  char *fn = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
    strcpy(filename, fn);
    g_free(fn);
  }

  gtk_widget_destroy(dialog);
  return strcmp(filename, ""); //return true if filename != ""
}

Code: Select all

noinline bool gtk_file_save(Window &owner, char *filename, const char *filter, const char *path) {
  strcpy(filename, "");

GtkWidget *dialog = gtk_file_chooser_dialog_new("Save File",
  GTK_WINDOW(owner.info.window), GTK_FILE_CHOOSER_ACTION_SAVE,
  GTK_STOCK_CANCEL, GTK_RESPONSE_CANCEL,
  GTK_STOCK_SAVE, GTK_RESPONSE_ACCEPT, (const gchar*)NULL); //<- last param modified

  if(path && strcmp(path, "")) {
    gtk_file_chooser_set_current_folder(GTK_FILE_CHOOSER(dialog), path);
  }
  gtk_file_chooser_set_do_overwrite_confirmation(GTK_FILE_CHOOSER(dialog), TRUE);

  if(gtk_dialog_run(GTK_DIALOG(dialog)) == GTK_RESPONSE_ACCEPT) {
  char *fn = gtk_file_chooser_get_filename(GTK_FILE_CHOOSER(dialog));
    strcpy(filename, fn);
    g_free(fn);
  }

  gtk_widget_destroy(dialog);
  return strcmp(filename, ""); //return true if filename != ""
}

If this fix works, I'll remove the old (poor) workaround and post a tarbz2 update shortly.

Turambar · Post by **Turambar** » Mon Jun 04, 2007 7:32 pm

I confirm that every patch you have commited in this thread work great. Gtk open dialog won't crash anymore whether path ends with a slash or not. Also providing code for command line usage was nice, thanks.

[vEX] · Post by **[vEX]** » Mon Jun 04, 2007 7:50 pm

I can confirm as well, even the problem I had that I posted in the other thread (with bsnes not responding at all) is gone, works fine now.

byuu · Post by **byuu** » Mon Jun 04, 2007 8:00 pm

Excellent! Thank you both for testing. Glad it's working, hopefully at full speed. If not, I suggest disabling audio, set system.audio to "none" in the advanced panel. If you get <60fps, the ALSA buffer underruns do something odd and cut the speed of bsnes in half. You'll then want to enable XV_SYNC_TO_VBLANK to get 60fps video, but even that doesn't work on nVidia's drivers, because the devs disabled it to work around a bug in Beryl (wasn't that nice of them?)

I'll post the updated source again tonight with these changes for everyone else.

I think I might know why GTK+ crashes with the last parameter as 0. If it's using a va_list or vararg setup, then pushing 0 would only push a 32-bit variable. That would work fine on 32-bit platforms, but on a 64-bit platform, only the lower half of the address would be clear, so it's possible (and apparently likely) that the upper half would not be zero, hence it would fail a pointer == null test, and then start performing a strlen on that value and crash. The (const gchar*) cast is just ensuring it pushes a 64-bit null value on 64-bit systems.

They really should document this on their website, that's an easy bug to miss, as without the varargs, null is implicitly cast to 64-bits.

funkyass · Post by **funkyass** » Mon Jun 04, 2007 8:44 pm

What does GCC assume for a plain integer constant?

byuu · Post by **byuu** » Mon Jun 04, 2007 9:08 pm

int, 32-bits on both 32-bit and 64-bit architectures.
C++ standard actually declares NULL as 0, too. Not very safe, but usually not a problem as comparison will promote int(0) to int64(0) to test against a pointer.

blargg · Post by **blargg** » Tue Jun 05, 2007 4:17 pm

C++ standard actually declares NULL as 0, too. Not very safe,

#define NULL ((void*) 0)

doesn't work in C++, because the implicit void*->T* type hole from C was closed. Even in C where NULL is can be defined as shown above, the representation of a null int* or Foo* might not be the same, thus you can still run into problems (and the representation of a null pointer should never be assumed to be all 0 bits). The correct way to pass a null pointer to a vararg function is to cast it to the *exact* pointer type the vararg function will decode it as, i.e. (int*) 0 if it will be decoding it as va_arg( args, int* ).

Varargs are not suitable for casual use.

byuu · Post by **byuu** » Tue Jun 05, 2007 5:22 pm

Varargs are not suitable for casual use.

I agree, I'm intimately familiar with varargs, understanding how they work on the asm level, so I know about their limitations. They definitely should not be used, as they are, in C++ apps.

There are safe ways to do varargs, but unless compilers add new function call ABIs, it doesn't really matter. They can't be used as they are now.

Sadly, C++ has some major issues with small-time memory allocation and chaining. It would be nice to safely replace sprintf functions with things such as:
my_textbox.set_text("number = %d" + x);

... but with the above and operator overloading, you run the risk of lots of costly new[] / delete[] calls.

But as it stands, varargs are the only means I have of simplistic string printing. C++ stringstreams have the most nauseating syntax I have ever encountered, even if they do eliminate the new[] / delete[] problem. They just look absolutely unnatural.

EDIT: hmm, actually ... this syntax isn't too bad:

my_textbox.set_text(string() << "number = " << num << ", or " << string("%0.2d", num) << ".");

Yeah, it'll probably call malloc+free twice, but what can you do? The syntax for std::ostringstream would be something like:

my_textbox.set_text( (std::ostringstream() << "number = " << num << ", or " << std::ios_base::setf(std::ios_base::ios::dec) << std::ios_base::flags::width(2) << num << ".").str().c_str() );

... which would probably error out anyway.

creaothceann · Post by **creaothceann** » Tue Jun 05, 2007 9:57 pm

byuu wrote:[...] costly new[] / delete[] calls

[...] it'll probably call malloc+free twice, but what can you do?

It's string printing.

Honestly...

byuu · Post by **byuu** » Tue Jun 05, 2007 11:30 pm

It's string printing. Honestly...

... and? malloc and free can be very costly. Once the program can no longer find free memory in heap, it has to reorder memory to gain that free space. This process can be several orders of magnitude more expensive than the cost of building a string. If you're designing a real-time app, you don't want to risk that kind of unresponsiveness.

creaothceann · Post by **creaothceann** » Tue Jun 05, 2007 11:51 pm

Yes, but... how often are you printing text, and how often will the heap memory in these cases run out?
My point being, I can't imagine* bsnes having that problem. Just use "<<" and be done with it.

*would love to see profiler data though, if available

blargg · Post by **blargg** » Wed Jun 06, 2007 12:05 am

I'm with creaothceann on this one. Unless you have actually profiled your app and found that the allocator is taking a lot of time, don't unnecessarily avoid it. Even if you find allocation taking a lot of time, you can often use a specialized allocator and speed things up. If you're bending over backwards just to avoid allocations, you're spending time that could be better spent elsewhere optimizing things that really require lots of work. Sure, the allocator has to ask the system for more memory occasionally, but it won't be doing that constantly (or if it does, it's broken).

byuu · Post by **byuu** » Wed Jun 06, 2007 12:36 am

Well, again, I was worried about use in string-intensive apps like cross assemblers, but yeah, alright. I'll implement << string concatenation, then, and drop the vararg crap. Even if it is slower (and that's a maybe), it's cleaner and automatically applies to all functions ... I don't have to write va_list parsers for each vararg function.

But I'm not using std::ostringstream, I can't stand the syntax of that thing.

So ... how much flack will I take for declaring operator const char*(); in my string class? :/

Mockup example, mainly to show usage, not implementation (it's not overflow safe like the real libstring):

Code: Select all

struct string {
char *t;
  string& operator<<(const char *data) {
    strcat(t, data);
    return *this;
  }

  string& operator<<(const int data) {
  char n[256];
    sprintf(n, "%d", data);
    strcat(t, n);
    return *this;
  }

  operator const char*() { return t; }

  string() { printf("allocating string\n"); t = new char[4096]; }
  ~string() { delete[] t; printf("freeing string\n"); }

  string(const char *fmt, const int data) {
    printf("allocating string (int)\n");
    t = new char[4096];
    sprintf(t, fmt, data);
  }
};

void echo(const char *str) { printf("%s", str); }

int main() {
  echo(string() << "age = " << string("%0.4x", 18) << "\n");

  printf("done\n");
  getch();
  return 0;
}