From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 11498 invoked by alias); 23 Nov 2014 18:36:52 -0000 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 Received: (qmail 11487 invoked by uid 89); 23 Nov 2014 18:36:51 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-4.0 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Sun, 23 Nov 2014 18:36:50 +0000 Received: from int-mx11.intmail.prod.int.phx2.redhat.com (int-mx11.intmail.prod.int.phx2.redhat.com [10.5.11.24]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id sANIamaP005132 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=FAIL) for ; Sun, 23 Nov 2014 13:36:48 -0500 Received: from host2.jankratochvil.net (ovpn-116-31.ams2.redhat.com [10.36.116.31]) by int-mx11.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id sANIajri021658 (version=TLSv1/SSLv3 cipher=AES128-GCM-SHA256 bits=128 verify=NO); Sun, 23 Nov 2014 13:36:47 -0500 Date: Sun, 23 Nov 2014 18:36:00 -0000 From: Jan Kratochvil To: Pedro Alves Cc: gdb-patches@sourceware.org Subject: Re: [PATCH v3 14/14] the "compile" command Message-ID: <20141123183644.GA31165@host2.jankratochvil.net> References: <20141101214552.13230.45564.stgit@host1.jankratochvil.net> <20141101214733.13230.49968.stgit@host1.jankratochvil.net> <54625B26.8000309@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <54625B26.8000309@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-IsSubscribed: yes X-SW-Source: 2014-11/txt/msg00573.txt.bz2 On Tue, 11 Nov 2014 19:53:26 +0100, Pedro Alves wrote: > > +/* See compile-internal.h. */ > > + > > +void > > +gcc_convert_symbol (void *datum, > > + struct gcc_c_context *gcc_context, > > + enum gcc_c_oracle_request request, > > + const char *identifier) > > ... > > > + > > + /* We can't allow exceptions to escape out of this callback. Safest > > + is to simply emit a gcc error. */ > > + TRY_CATCH (e, RETURN_MASK_ERROR) > > + { > > + struct symbol *sym; > > Shouldn't this catch ctrl-c too then? Likewise the other hooks. Yes, changed. > > + for (i = 0; i < 4; ++i) > > It'd be good to give this magic constant a name, or at > a least a comment. Added there: // Iterate all log2 sizes in bytes supported by c_get_mode_for_size. Added to c_get_mode_for_size: default: internal_error (__FILE__, __LINE__, _("Invalid GCC mode size %d."), size); > > + { > > + const char *mode = c_get_mode_for_size (1 << i); > > + > > + gdb_assert (mode != NULL); > > + fprintf_unfiltered (buf, > > + "typedef int" > > + " __attribute__ ((__mode__(__%s__)))" > > + " __gdb_int_%s;\n", > > + mode, mode); > > + } > > > > > + > > +/* A cleanup function to remove a directory and all its contents. */ > > + > > +static void > > +do_rmdir (void *arg) > > +{ > > + char *zap = concat ("rm -rf ", arg, (char *) NULL); > > + > > + system (zap); > > +} > > This is quite scary... Could we please add an assert here > that tempdir_name starts with "/tmp/gdbobj-", just in case something > goes really wrong here? Added: /* Initial filename for temporary files. */ #define TMP_PREFIX "/tmp/gdbobj-" + #define TEMPLATE TMP_PREFIX "XXXXXX" + gdb_assert (strncmp (dir, TMP_PREFIX, strlen (TMP_PREFIX)) == 0); > > + /* Override flags possibly coming from DW_AT_producer. */ > > + compile_args = xstrdup ("-O0 -gdwarf-4" > > + /* We use -fPIC to ensure that we can reference properly. Otherwise > > + on x86-64 a string constant's address might be truncated when gdb > > + loads the object; another approach would be -mcmodel=large, but > > + -fPIC seems more portable across back ends. */ > > This comment ends up being a bit unexpected/odd, given that ... > > > + " -fPIC" > > + /* We don't want warnings. */ > > + " -w" > > ... patch #6 has: > > > +char * > > +default_gcc_target_options (struct gdbarch *gdbarch) > > +{ > > + return xstrprintf ("-m%d%s", gdbarch_ptr_bit (gdbarch), > > + gdbarch_ptr_bit (gdbarch) == 64 ? " -mcmodel=large" : ""); > > +} > > + > > IOW, is the comment stale? You are right the comment is stale although the code is not stale. Up to date reason for these options I have written in: https://sourceware.org/gdb/wiki/GCCCompileAndExecute#Relocating_the_object_file Therefore put there: /* We use -fPIC Otherwise GDB would need to reserve space large enough for any object file in the inferior in advance to get the final address when to link the object file to and additionally the default system linker script would need to be modified so that one can specify there the absolute target address. */ " -fPIC" And also: /* -mcmodel=large is used so that no GOT (Global Offset Table) is needed to be created in inferior memory by GDB (normally it is set by ld.so). */ char * default_gcc_target_options (struct gdbarch *gdbarch) { return xstrprintf ("-m%d%s", gdbarch_ptr_bit (gdbarch), gdbarch_ptr_bit (gdbarch) == 64 ? " -mcmodel=large" : ""); } Thanks, Jan