From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 21733 invoked by alias); 8 Jun 2013 11:06:03 -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 21714 invoked by uid 89); 8 Jun 2013 11:06:02 -0000 X-Spam-SWARE-Status: No, score=-2.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,RCVD_IN_HOSTKARMA_YE,SPF_PASS autolearn=ham version=3.3.1 X-Spam-User: qpsmtpd, 2 recipients Received: from mail-ee0-f49.google.com (HELO mail-ee0-f49.google.com) (74.125.83.49) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Sat, 08 Jun 2013 11:05:57 +0000 Received: by mail-ee0-f49.google.com with SMTP id b57so2061002eek.8 for ; Sat, 08 Jun 2013 04:05:55 -0700 (PDT) X-Received: by 10.15.41.196 with SMTP id s44mr2653782eev.138.1370689555455; Sat, 08 Jun 2013 04:05:55 -0700 (PDT) Received: from localhost ([2.26.203.247]) by mx.google.com with ESMTPSA id u7sm5432265eef.14.2013.06.08.04.05.54 for (version=TLSv1.2 cipher=RC4-SHA bits=128/128); Sat, 08 Jun 2013 04:05:54 -0700 (PDT) From: Richard Sandiford To: "Maciej W. Rozycki" Mail-Followup-To: "Maciej W. Rozycki" ,Joel Brobecker , Catherine Moore , , , rdsandiford@googlemail.com Cc: Joel Brobecker , Catherine Moore , , Subject: Re: [PATCH 1/2] MIPS: Compressed PLT/stubs support References: <87621mwt3l.fsf@talisman.default> <87ehfozyhg.fsf@talisman.default> Date: Sat, 08 Jun 2013 16:04:00 -0000 In-Reply-To: (Maciej W. Rozycki's message of "Fri, 7 Jun 2013 22:09:24 +0100") Message-ID: <8761xo981a.fsf@talisman.default> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-SW-Source: 2013-06/txt/msg00190.txt.bz2 "Maciej W. Rozycki" writes: >> Maybe something like: >> >> /* If compressed PLT entries are available, make sure that we use them >> for MIPS16 and microMIPS calls. */ >> else if ((r_type == R_MIPS16_26 || r_type == R_MICROMIPS_26_S1) >> && h != NULL >> && h->use_plt >> && h->root.plt.plist->comp_offset != MINUS_ONE) >> { >> sec = htab->splt; >> symbol = (sec->output_section->vma >> + sec->output_offset >> + htab->plt_header_size >> + htab->plt_mips_offset >> + h->root.plt.plist->comp_offset >> + 1); >> target_is_16_bit_code_p = !MICROMIPS_P (abfd); >> target_is_micromips_code_p = MICROMIPS_P (abfd); >> } >> >> at the end of the: >> >> /* If this is a reference to a 16-bit function with a stub, we need >> to redirect the relocation to the stub unless: >> >> chain of ifs. > > More or less, though I've decided to push it ahead of the chain so that > it sees the state consistent regardless of whether any PLT entry processed > has been duplicated for dual-mode support. This probably does not really > matter right now (fn_stub/need_fn_stub cases will override any symbol > value to use for the relocation and likewise the mode setting anyway, > call_stub/call_fp_stub cases will only ever have a standard MIPS PLT entry > due to the arrangement in _bfd_mips_elf_adjust_dynamic_symbol and > la25_stub cases will never have a PLT entry as these must resolve > locally), but I feel a bit uneasy about this half-cooked state. Let me > know if you disagree (and why). The changes made by the block beginning: /* If this is a reference to a 16-bit function with a stub, we need to redirect the relocation to the stub unless: are mutually-exclusive with each other and with the new PLT transformation. We should only ever perform one. And the transformation: /* If this is a MIPS16 call with a stub, that is made through the PLT or to a standard MIPS function, we need to redirect the call to the stub. Note that we specifically exclude R_MIPS16_CALL16 from this behavior; indirect calls should use an indirect stub instead. */ else if (r_type == R_MIPS16_26 && !info->relocatable && ((h != NULL && (h->call_stub != NULL || h->call_fp_stub != NULL)) || (local_p && mips_elf_tdata (input_bfd)->local_call_stubs != NULL && mips_elf_tdata (input_bfd)->local_call_stubs[r_symndx] != NULL)) && ((h != NULL && h->use_plt_entry) || !target_is_16_bit_code_p)) logically trumps the PLT one. That's why I think all four transformations should be in a single if chain, and why the PLT one should come last. You said yourself about the addition of (h != NULL && h->use_plt_entry): The current code flow is a bit subtle, this piece works because as a side effect of the PLT being standard MIPS code the call is qualified as a cross-mode jump. However this is not really the reason the call needs to be redirected for -- the redirection would have to be done regardless even if we did decide to emit the PLT entry as MIPS16 code for some reason. That is, if we (redundantly) created both standard and MIPS16 PLT entries for functions with stubs, and if the "if" statement as you had it redirected the call from a standard PLT entry to a MIPS16 PLT entry, this code must explicitly ignore that transformation. So why do it? Making a point of doing it ahead of the if-else block, only to explicitly ignore it in the if-else block, just makes the code more confusing IMO. Richard