From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 29195 invoked by alias); 19 Aug 2010 09:18:49 -0000 Received: (qmail 29183 invoked by uid 22791); 19 Aug 2010 09:18:47 -0000 X-SWARE-Spam-Status: No, hits=-2.0 required=5.0 tests=AWL,BAYES_00,TW_BJ,T_RP_MATCHES_RCVD X-Spam-Check-By: sourceware.org Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 19 Aug 2010 09:18:41 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id D128D2BAC13; Thu, 19 Aug 2010 05:18:39 -0400 (EDT) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id IIpXIGrIHknw; Thu, 19 Aug 2010 05:18:39 -0400 (EDT) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 902AE2BAB74; Thu, 19 Aug 2010 05:18:39 -0400 (EDT) Received: by joel.gnat.com (Postfix, from userid 1000) id CAE87F59B2; Thu, 19 Aug 2010 11:18:37 +0200 (CEST) From: Joel Brobecker To: gdb-patches@sourceware.org Cc: Joel Brobecker Subject: [PATCH] Fix regression in -file-list-exec-source-files command. Date: Thu, 19 Aug 2010 09:18:00 -0000 Message-Id: <1282209516-3612-1-git-send-email-brobecker@adacore.com> Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2010-08/txt/msg00333.txt.bz2 This is to fix one of the issues that was blocking for the 7.2 release :). See http://sourceware.org/ml/gdb/2010-07/msg00118.html for a description of the problem. Namely, the file and fullname fields are inverted in the output of the -file-list-exec-source-files GDB/MI command: (gdb) interpreter-exec mi -file-list-exec-source-files ^done,files=[{file="/takamaka.a/brobecke/ex/list-exec-source-files/foo.c",fullname="foo.c"},{file="/takamaka.a/brobecke/ex/list-exec-source-files/foo.c",fullname="foo.c"},{file="",fullname="init.c"},{file="",fullname="../sysdeps/x86_64/elf/start.S"},{file="",fullname="../sysdeps/x86_64/elf/start.S"}] It turns out to be a silly thinko: The map_symbol_filenames function calls the psymtab version of map_symbol_filenames routine, and this version called the callback function with filename and fullname in the wrong order (fullname/filename instead of filename/fullname). The routine description in symfile.h confirst that expected order for the FUN callback parameters: /* Call a callback for every file defined in OBJFILE. FUN is the callback. It is passed the file's name, the file's full name, and the DATA passed to this function. */ void (*map_symbol_filenames) (struct objfile *objfile, void (*fun) (const char *, const char *, void *), void *data); Fixing this error uncovered another location where the arguments were reversed: maybe_add_partial_symtab_filename. Once the first error was fixed, the debugger would crash while attempting to do completion, because it was given a NULL fullname instead of the non-NULL filename. IMO, we would had a better chance of avoiding this sort of issue if we had named the parameters in the callback function. It was less than immediate to verify the order of arguments when I found the source of the first problem (unnamed parameters, and lack of function documentation). What I suggest is to create a type for the function pointer callback, and have the parameters named and documented... But that can be taken care of through another patch. gdb/ChangeLog: * psymtab.c (map_symbol_filenames_psymtab): Call FUN with the arguments in the correct order. * symtab.c (maybe_add_partial_symtab_filename): Declare the arguments in the correct order. I've checked this in HEAD and 7.2 branch, as it seems sufficiently obvious. This was tested on x86_64-linux in HEAD. I'll see whether it's possible to create a regression test or not, next. --- gdb/ChangeLog | 7 +++++++ gdb/psymtab.c | 2 +- gdb/symtab.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index b889876..580be2b 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,10 @@ +2010-08-19 Joel Brobecker + + * psymtab.c (map_symbol_filenames_psymtab): Call FUN with + the arguments in the correct order. + * symtab.c (maybe_add_partial_symtab_filename): Declare + the arguments in the correct order. + 2010-08-19 Jan Kratochvil * varobj.c (varobj_create): Replace variable old_fi with old_id, diff --git a/gdb/psymtab.c b/gdb/psymtab.c index bc47681..228b7a8 100644 --- a/gdb/psymtab.c +++ b/gdb/psymtab.c @@ -913,7 +913,7 @@ map_symbol_filenames_psymtab (struct objfile *objfile, continue; fullname = psymtab_to_fullname (ps); - (*fun) (fullname, ps->filename, data); + (*fun) (ps->filename, fullname, data); } } diff --git a/gdb/symtab.c b/gdb/symtab.c index d43d573..baf6d94 100644 --- a/gdb/symtab.c +++ b/gdb/symtab.c @@ -4072,7 +4072,7 @@ struct add_partial_filename_data /* A callback for map_partial_symbol_filenames. */ static void -maybe_add_partial_symtab_filename (const char *fullname, const char *filename, +maybe_add_partial_symtab_filename (const char *filename, const char *fullname, void *user_data) { struct add_partial_filename_data *data = user_data; -- 1.7.1