From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 30911 invoked by alias); 17 Aug 2007 23:26:47 -0000 Received: (qmail 30874 invoked by uid 22791); 17 Aug 2007 23:26:46 -0000 X-Spam-Check-By: sourceware.org Received: from mu-out-0910.google.com (HELO mu-out-0910.google.com) (209.85.134.184) by sourceware.org (qpsmtpd/0.31) with ESMTP; Fri, 17 Aug 2007 23:26:43 +0000 Received: by mu-out-0910.google.com with SMTP id g7so702006muf for ; Fri, 17 Aug 2007 16:26:39 -0700 (PDT) Received: by 10.86.72.15 with SMTP id u15mr2481027fga.1187393199547; Fri, 17 Aug 2007 16:26:39 -0700 (PDT) Received: from ?88.210.66.131? ( [88.210.66.131]) by mx.google.com with ESMTPS id i3sm2583090nfh.2007.08.17.16.26.36 (version=TLSv1/SSLv3 cipher=RC4-MD5); Fri, 17 Aug 2007 16:26:38 -0700 (PDT) Message-ID: <46C62E62.3000300@portugalmail.pt> Date: Fri, 17 Aug 2007 23:26:00 -0000 From: Pedro Alves User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; pt-BR; rv:1.8.1.6) Gecko/20070728 Thunderbird/2.0.0.6 Mnenhy/0.7.4.0 MIME-Version: 1.0 To: gdb-patches@sourceware.org Subject: Re: Windows DLL support update. References: <46C0F600.5010607@portugalmail.pt> <20070814121008.GA18838@ednor.casa.cgf.cx> <46C39D10.3020001@portugalmail.pt> <20070816005731.GA16007@caradoc.them.org> In-Reply-To: <20070816005731.GA16007@caradoc.them.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-IsSubscribed: yes 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: 2007-08/txt/msg00345.txt.bz2 Daniel Jacobowitz wrote: > > I have one concern; one of us should fix the problem Ulrich pointed > out with my core changes: > Ooouups, I totally forgot seeing it. >> On Tue, Jun 19, 2007 at 09:05:51PM +0200, Ulrich Weigand wrote: >>> One thing I don't quite like is this: >>> >>>> + case TARGET_OBJECT_LIBRARIES: >>>> + if (core_vec->xfer_shared_libraries != NULL) >>>> + return core_vec->xfer_shared_libraries (ops, object, annex, >>>> readbuf, >>>> + writebuf, offset, len); >>> I had understood the core_fns method of providing a core file target >>> to >>> be deprecated, and in fact I just recently got rid of it for AIX in >>> favour >>> of the gdbarch_regset_from_core_section callback ... I'd prefer >>> this to >>> be a gdbarch callback (which would also support core file >>> cross-debugging). >> I hadn't even thought about it. Yes, you're right. > OK, I'll take a stab at it, although an interface suggestion would be nice. Should it be a callback with a similar interface to xfer_shared_libraries and to be called in core_xfer_partial (the callback is expected to fill a TARGET_OBJECT_LIBRARIES, similar to what you had, but implemented as a gdbarch callback), or do you think it should be somethink different? I'm giving the first option a try, and while looking at it, I've implemented gdbarch_regset_from_core_section for i386/win32/Cygwin, meaning core file cross debugging is on the way for Cygwin. > Also, we added #include's so the Makefile needs an update. > Ooouups 2. Will do. >> +/* Returns 1 if ADDR is within the cygwin1.dll text segment, returns 0 >> + otherwise. */ >> +static int >> +inside_cygwin (CORE_ADDR addr) >> +{ >> + if (cygwin_load_start == 0) >> + { >> + struct so_list *so; >> + >> + for (so = master_so_list (); so; so = so->next) > > Nothing outside of the solib implementations calls master_so_list. I > think that's best; can you use ALL_OBJFILES here instead? > It would not work with 'set auto-solib-add 0'. In that case, the so_list from the master list has an open associated bfd to look at, but ALL_OBJFILES doesn't report cygwin1.dll, because the symbols haven't been read yet. This means that in that case, the user would see the internal Cygwin exceptions as SIGSEGVs. That would be a functionality regression. Perhaps the cygwin1.dll .text range recording could also be implemented with a callback (observer?) that gets called on every solib loaded instead of iterating through all loaded libs. Let me explain why I was doing it differently from how it was done before, because we may come to the conclusion this is the wrong way to do it. The current win32-nat.c records the cygwin1.dll .text load start/end addresses when imediatelly when the dll event is reported. This range is then used to filter any Windows exception that occurs inside Cygwin, since they will be handled internally by Cygwin - the user should never see them. Same thing happens with the IsBadXxxPtr family of win32 functions. They trigger an access violation by design, so the debugger although has a chance to see it, shouldn't normally pass it to the user when they are used. It is desirable to also have this filtering funcionality on the gdbserver side. The function in the patch implements the base for the filtering pretty much decoupled from the rest of the win32-nat.c code. Instead of teaching gdbserver about these things, we could move these bits into win32-tdep.c, or i386-cygwin-tdep.c, and install a gdbarch callback to be called from/near handle_inferior_event. The callback could then decide to ignore the SIGTRAP, for instance. Alternativelly, we could extend the qSymbol mechanism, and have gdbserver know how to ignore the exceptions itself. qSymbol allows gdbserver to know the address of a symbol, but the IsBadxxPtr case needs a start-end function range, because the exception happens *inside* that function. The cygwin1.dll .text start/end could be solved if we add __cygwin_text_start__ and __cygwin_text_end__ symbols to cygwin1.dll. The advantage of this approach is that an app triggering lots of internal cygwin exceptions, or calling IsBadXxxPtr in a tight loop would perform much faster under gdbserver, as the ignore addresses could be cached, instead of passing a SIGTRAP to gdb every time, just to have it ignored. I'm not sure it justifies the extra complexity, though. The gdb-only method side has the advantage that doesn't need any protocol change, is simpler, and possibly more extensible/flexible, and can share the same user commands (set cygwin-exceptions x). Of course for all this to work gdb needs to see a copy of the same version of the dlls installed in the target. Not sure about licensing problems with OS components... Perhaps the best way would be to require dbghelp.dll (MSFTs symbol reader API) on gdbserver, and use it to get at the symbols. Then all this becomes moot. Anyways, a lot to avoid the master_so_list call. :) If desired, I'll revert to the old method of getting at the cygwin1.dll ranges on dll load, and come back to it after this patch is in. >> + if (data->module_count == 0) >> + { >> +#if 0 >> + /* TODO: What if the user supplied exec and/or >> + symbol files on the command line? */ >> + /* The first module is the .exe itself. */ >> + symbol_file_add_main (module_name, 0); >> +#endif >> + } >> + else >> + { >> + struct so_list *so = win32_make_so (module_name, base_addr); >> + solib_to_xml (so, data->obstack); >> + win32_free_so (so); >> + } >> + data->module_count++; > > Let's not add #if 0 / TODO. I think we can drop this code, how about > you? > Ahh, it worked - I was hoping somebody would comment on that. I'll remove it. :) Cheers, Pedro Alves