From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6916 invoked by alias); 14 Mar 2012 20:38:07 -0000 Received: (qmail 6709 invoked by uid 22791); 14 Mar 2012 20:38:05 -0000 X-SWARE-Spam-Status: No, hits=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE,SPF_HELO_PASS X-Spam-Check-By: sourceware.org Received: from moutng.kundenserver.de (HELO moutng.kundenserver.de) (212.227.126.186) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 14 Mar 2012 20:37:51 +0000 Received: from [192.168.7.23] (p4FD11477.dip0.t-ipconnect.de [79.209.20.119]) by mrelayeu.kundenserver.de (node=mreu2) with ESMTP (Nemesis) id 0MPsu6-1SCt220okc-0052K3; Wed, 14 Mar 2012 21:37:28 +0100 Message-ID: <4F610188.5010001@onevision.com> Date: Wed, 14 Mar 2012 20:38:00 -0000 From: Roland Schwingel User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:10.0.2) Gecko/20120216 Thunderbird/10.0.2 MIME-Version: 1.0 To: Joel Brobecker , gdb-patches@sourceware.org Subject: Re: [PATCH] Add dll trampoline code handling for windows 64bit Content-Type: text/plain; charset=ISO-8859-15; format=flowed Content-Transfer-Encoding: 7bit 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: 2012-03/txt/msg00513.txt.bz2 Hi Joel, Thanks for you reply and suggestions. I will prepare a new patch tomorrow when back at the office with all your suggestions. Joel Brobecker wrote on 14.03.2012 17:13:03: > In addition to Tom's answer, I have a few minor comments. The real > review will have to come from our Windows Maintainer for your patch > to be approved... > > > 2012-03-14 Roland Schwingel > > > > * amd64-windows-tdep.c: #include "frame.h" > > (amd64_windows_skip_trampoline_code): New function. > > (amd64_windows_init_abi): Add trampoline registration. > > (missing period at the end of the first line). OK There is a script called gdb_indent.sh in gdb's root dir. I applied it on amd64-windows-nat.c ahead of getting out my patch thinking that this would be the correct way. Some misunderstanding as it produced most of your indention concerns. My patch is based upon the implementation for windows 32 bit in i386-tdep.c (function i386_pe_skip_trampoline_code()). I made my patch as close as possible to the implementation there thinking that would make it easier to be accepted. If you take a look there you will see home similar my patch is. Your suggestions are newer, so I will take them into account. > > + read_memory (pc + indirect, addr, 8); > > + pos = (gdb_byte *) &destination; > > + pos[0] = addr[6]; > > + pos[1] = addr[7]; > > + pos[2] = addr[0]; > > + pos[3] = addr[1]; > > + pos[4] = addr[2]; > > + pos[5] = addr[3]; > > + pos[6] = addr[4]; > > + pos[7] = addr[5]; > > + > > + return destination; > > Yeah, Tom's suggestion is a better suggestion. I think you are going > to have endianness issues this way. You could use > read_memory_typed_address as well, but it's a little more involved, > and I don't think it's necessary here. But otherwise, to me, the > latter is the function to be used for reading addresses from inferior > memory. Before doing it "my" way I already played around with both read_memory_typed_address and read_memory_unsigned_integer but did not get the correct CORE_ADDR. The bytes were always in the wrong order believing that these are endianess issues. Will reinvestigate that when back at the office. Anyhow my approach appears to be working, too. I succesfully single stepped thru many dlls using my patch on win64 which was not possible before. > I am sorry if it feels like it's a lot of little rules. It is. But > it should be easy to learn them and it allows us to have a consistent > style for our code. Sure. No problem! Thanks for taking a look at my patch, Roland