From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 2029 invoked by alias); 5 Mar 2008 03:28:25 -0000 Received: (qmail 2015 invoked by uid 22791); 5 Mar 2008 03:28:24 -0000 X-Spam-Check-By: sourceware.org Received: from smtp107.biz.mail.mud.yahoo.com (HELO smtp107.biz.mail.mud.yahoo.com) (68.142.200.255) by sourceware.org (qpsmtpd/0.31) with SMTP; Wed, 05 Mar 2008 03:27:54 +0000 Received: (qmail 20831 invoked from network); 5 Mar 2008 03:27:52 -0000 Received: from unknown (HELO ?192.168.1.69?) (wilson@tuliptree.org@69.110.29.99 with login) by smtp107.biz.mail.mud.yahoo.com with SMTP; 5 Mar 2008 03:27:51 -0000 X-YMail-OSG: e.EmF7QVM1kie7PorOTV0T164AwN1ikh571mG8qzMifLxc4.7iliHeACkvIHVfv2dACH6WrUHBP.BpEpp0AYtSxXz2BNemA3Ge50nENr1dVI9so7IrnW3mFMESSBSwIsfjwCTGD1d1E- X-Yahoo-Newman-Property: ymail-3 Subject: Re: [ping] Ignore thread local symbols when generating dbx debug info From: Jim Wilson To: John David Anglin Cc: gcc-patches@gcc.gnu.org, jakub@redhat.com In-Reply-To: <200802080043.m180h7Vq026518@hiauly1.hia.nrc.ca> References: <200802080043.m180h7Vq026518@hiauly1.hia.nrc.ca> Content-Type: multipart/mixed; boundary="=-zQP0gu3EZNTfeAKHknkj" Date: Wed, 05 Mar 2008 03:28:00 -0000 Message-Id: <1204687670.3227.24.camel@localhost.localdomain> Mime-Version: 1.0 X-Mailer: Evolution 2.12.3 (2.12.3-1.fc8) Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org X-SW-Source: 2008-03/txt/msg00276.txt.bz2 --=-zQP0gu3EZNTfeAKHknkj Content-Type: text/plain Content-Transfer-Encoding: 7bit Content-length: 3191 On Thu, 2008-02-07 at 19:43 -0500, John David Anglin wrote: > Second ping. > > http://gcc.gnu.org/ml/gcc-patches/2007-12/msg01166.html Belatedly replying to this, now that I'm doing FSF gcc work again... I had to spend a bit of time looking at the code. This assumes some knowledge of emutls, fortran, and hpux, all of which I am lacking. I do see the problem though. A emutls variable access requires a call to __emutls_get_address, and then we find the variable at a fixed offset from the return value. There is no way to express this in the stabs debug info. It is a bit annoying that the emutls.c file has no comments explaining what it does. It doesn't even have comments for each function as required by the coding conventions. It would be easier to understand if this stuff was documented some place. I thought it curious that this was failing for Fortran, but not apparently for C. Otherwise presumably you would have given a C testcase. I also wasn't able to reproduce this with a trivial C example. I tried debugging this a little bit. I see that the normal variable handling in expand_expr_real_1, case VAR_DECL, has an out for emutls variables. We create the necessary code to access it, and then return, before we use DECL_RTL, which calls make_decl_rtl which sets the DECL_RTL field. Since the emutls variable has no DECL_RTL, we don't try to emit debug info for it. However, with the Fortran testcase, the DECL_RTL got created in nonoverlapping_memrefs_p during the cse pass. This seems questionable to me. With emutls, a variable may or may not have a DECL_RTL set, depending on how it was optimized. This could lead to confusion later. It may also be causing subtle bugs. For instance, mudflap has hooks in make_decl_rtl, which may or may not be called with emutls, depending on how it was optimized, which means mudflap may or may not be tracking the variable. Perhaps we should be creating the DECL_RTL always in expand_expr_real_1. Perhaps make_decl_rtl should have support for emutls variables. Anyways, the dbxout.c change looks basically OK to me. It would be nice if there was a comment explaining what the patch is doing. You are right that canonical form for RTL will always put the SYMBOL_REF before the CONST_INT in a PLUS. It looks like we only have to handle the output of dbxout_expand_expr, so we don't have to handle any more complicated expressions. However, this points to another possible solution, which looks a little simpler to me. We can put this test in dbxout_expand_expr. I attached a patch that does this. Trying my patch on the testcase, I see the result is a little bit different than yours. With my patch, debug info is disabled for foo, thrc, bar1, and bar2. With your patch, debug info is disable for foo, thrc, and bar2. Which means you still have debug info for bar1. The debug info seems confused for bar1 though. Internally, gcc thinks it is addressed relative to thrc_, and then doesn't emit the address because it is a global, so it slips through your checks. Can you test my patch and see if it works? If not, then your patch is fine, preferably with a comment added like the one in my patch. Jim --=-zQP0gu3EZNTfeAKHknkj Content-Disposition: attachment; filename=patch.dbx.emutls Content-Type: text/plain; name=patch.dbx.emutls; charset=UTF-8 Content-Transfer-Encoding: 7bit Content-length: 924 2008-03-04 James E. Wilson * dbxout.c (dbxout_expand_expr, case VAR_DECL): Return NULL for emulated thread local variables. Index: dbxout.c =================================================================== --- dbxout.c (revision 132829) +++ dbxout.c (working copy) @@ -2332,6 +2332,15 @@ switch (TREE_CODE (expr)) { case VAR_DECL: + /* We can't handle emulated tls variables, because the address is an + offset to the return value of __emutls_get_address, and there is no + way to express that in stabs. Also, there are name mangling issues + here. We end up with references to undefined symbols if we don't + disable debug info for these variables. */ + if (!targetm.have_tls && DECL_THREAD_LOCAL_P (expr)) + return NULL; + /* FALLTHRU */ + case PARM_DECL: if (DECL_HAS_VALUE_EXPR_P (expr)) return dbxout_expand_expr (DECL_VALUE_EXPR (expr)); --=-zQP0gu3EZNTfeAKHknkj--