From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by sourceware.org (Postfix) with ESMTPS id C9EAE3858421 for ; Thu, 30 Jun 2022 22:48:19 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org C9EAE3858421 Received: by mail-pj1-x102b.google.com with SMTP id x1-20020a17090abc8100b001ec7f8a51f5so4660059pjr.0 for ; Thu, 30 Jun 2022 15:48:19 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=jDsjwQSRh3hTMr63VvnbVY+Hz0ELmSx9Aq+I3FR+ML0=; b=anvaO8zsqZ9XwT/mZCfOhCiMnITpP23MNN1sztGwAIxVUy1ss/B7Cm2LXX2aGax88a TQufsTIiPmhuvOn99oOMe3+8Mgqwfl4Ht1g8TN69W7rchMh8frDjA5vh7edMfBGXAXvg A1R6sXV68yMEmvAatOWz8R7zJguje6vyC3y5d6KP4ECwXHSz9tL2yxBwto2QO+EbYJsz jmz8oJxRnGxWY3SYzS9sUC8ZAjQNCeLEO6p15y7ZTCgPl9/+645l6WKOozJSpULV6/BJ vAdmof/1uyReUY3qpwGtEiskBsE8JaFKYRypI8Y5y7PZQo3I5cvDTZyD1eW9QgGtMf3f XrQw== X-Gm-Message-State: AJIora/YwgKcHvSYLR8lQh+ec/64zAp7O8596/87bXnWz6FZSiea6uFM IIr/UOpTL2ioZF208ePGJ1KjgYyPjOzB3MqBiMIkLRX8MPo= X-Google-Smtp-Source: AGRyM1stX9MqhB6JvCASeYdZfRqwoDZqhmozRSwHyHFti14/EcCSD4Pz9pebrtDyG5woen/s8nmDNZiXPul9i2NB2Ac= X-Received: by 2002:a17:90b:895:b0:1ec:827c:ef0f with SMTP id bj21-20020a17090b089500b001ec827cef0fmr14382977pjb.10.1656629298732; Thu, 30 Jun 2022 15:48:18 -0700 (PDT) MIME-Version: 1.0 References: <0806b8f1-b463-41e8-1980-b511bdb451ff@suse.com> <20525355-6125-24f3-6b49-05b29abb127b@suse.com> In-Reply-To: <20525355-6125-24f3-6b49-05b29abb127b@suse.com> From: "H.J. Lu" Date: Thu, 30 Jun 2022 15:47:43 -0700 Message-ID: Subject: Re: [PATCH 2/3] x86: restore masking of displacement kinds To: Jan Beulich Cc: Binutils Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3019.0 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: binutils@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 30 Jun 2022 22:48:21 -0000 On Thu, Jun 30, 2022 at 5:08 AM Jan Beulich wrote: > > Commit 7d5e4556a375 rendered the check near the end of what is now > i386_finalize_displacement() entirely dead for AT&T mode, since for > operands involving a displacement .unspecified will always be set. But > the logic there is bogus anyway - Intel syntax operand size specifiers > are of no interest there either. The only thing which matters in the > "displacement only" determination is .baseindex. > > Of course when masking displacement kinds we should not at the same time > also mask off other attributes. > > Furthermore the type mask returned by lex_got() also needs to be > adjusted: The only case where we want Disp32 (rather than Disp32S) is > when dealing with 32-bit addressing mode in 64-bit code. > --- > Actually I don't understand why this masking has been dependent on > "displacement only". I would expect the masking to similarly apply to > displacements used with any kind of base/index expression. It has been there for a long time. Before I added the comment, there was if (!(i.types[this_operand] & ~Disp)) i.types[this_operand] &= types; I guess if the operand is displacement only, we don't change it. > I was considering to drop the OPERAND_TYPE_IMM32_32S_* that are being > changed, in favor of using C99 dedicated initializers. But perhaps this > would better be a separate change, dealing with the other similar > constants as well. > > --- a/gas/config/tc-i386.c > +++ b/gas/config/tc-i386.c > @@ -10380,7 +10380,7 @@ lex_got (enum bfd_reloc_code_real *rel, > OPERAND_TYPE_IMM64, true }, > { STRING_COMMA_LEN ("PLT"), { BFD_RELOC_386_PLT32, > BFD_RELOC_X86_64_PLT32 }, > - OPERAND_TYPE_IMM32_32S_DISP32, false }, > + OPERAND_TYPE_IMM32_32S_DISP32S, false }, > { STRING_COMMA_LEN ("GOTPLT"), { _dummy_first_bfd_reloc_code_real, > BFD_RELOC_X86_64_GOTPLT64 }, > OPERAND_TYPE_IMM64_DISP64, true }, > @@ -10389,28 +10389,28 @@ lex_got (enum bfd_reloc_code_real *rel, > OPERAND_TYPE_IMM64_DISP64, true }, > { STRING_COMMA_LEN ("GOTPCREL"), { _dummy_first_bfd_reloc_code_real, > BFD_RELOC_X86_64_GOTPCREL }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > { STRING_COMMA_LEN ("TLSGD"), { BFD_RELOC_386_TLS_GD, > BFD_RELOC_X86_64_TLSGD }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > { STRING_COMMA_LEN ("TLSLDM"), { BFD_RELOC_386_TLS_LDM, > _dummy_first_bfd_reloc_code_real }, > OPERAND_TYPE_NONE, true }, > { STRING_COMMA_LEN ("TLSLD"), { _dummy_first_bfd_reloc_code_real, > BFD_RELOC_X86_64_TLSLD }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > { STRING_COMMA_LEN ("GOTTPOFF"), { BFD_RELOC_386_TLS_IE_32, > BFD_RELOC_X86_64_GOTTPOFF }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > { STRING_COMMA_LEN ("TPOFF"), { BFD_RELOC_386_TLS_LE_32, > BFD_RELOC_X86_64_TPOFF32 }, > - OPERAND_TYPE_IMM32_32S_64_DISP32_64, true }, > + OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true }, > { STRING_COMMA_LEN ("NTPOFF"), { BFD_RELOC_386_TLS_LE, > _dummy_first_bfd_reloc_code_real }, > OPERAND_TYPE_NONE, true }, > { STRING_COMMA_LEN ("DTPOFF"), { BFD_RELOC_386_TLS_LDO_32, > BFD_RELOC_X86_64_DTPOFF32 }, > - OPERAND_TYPE_IMM32_32S_64_DISP32_64, true }, > + OPERAND_TYPE_IMM32_32S_64_DISP32S_64, true }, > { STRING_COMMA_LEN ("GOTNTPOFF"),{ BFD_RELOC_386_TLS_GOTIE, > _dummy_first_bfd_reloc_code_real }, > OPERAND_TYPE_NONE, true }, > @@ -10419,17 +10419,17 @@ lex_got (enum bfd_reloc_code_real *rel, > OPERAND_TYPE_NONE, true }, > { STRING_COMMA_LEN ("GOT"), { BFD_RELOC_386_GOT32, > BFD_RELOC_X86_64_GOT32 }, > - OPERAND_TYPE_IMM32_32S_64_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_64_DISP32S, true }, > { STRING_COMMA_LEN ("TLSDESC"), { BFD_RELOC_386_TLS_GOTDESC, > BFD_RELOC_X86_64_GOTPC32_TLSDESC }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > { STRING_COMMA_LEN ("TLSCALL"), { BFD_RELOC_386_TLS_DESC_CALL, > BFD_RELOC_X86_64_TLSDESC_CALL }, > - OPERAND_TYPE_IMM32_32S_DISP32, true }, > + OPERAND_TYPE_IMM32_32S_DISP32S, true }, > #else /* TE_PE */ > { STRING_COMMA_LEN ("SECREL32"), { BFD_RELOC_32_SECREL, > BFD_RELOC_32_SECREL }, > - OPERAND_TYPE_IMM32_32S_64_DISP32_64, false }, > + OPERAND_TYPE_IMM32_32S_64_DISP32S_64, false }, > #endif > }; > char *cp; > @@ -11144,7 +11144,6 @@ static int > i386_finalize_displacement (segT exp_seg ATTRIBUTE_UNUSED, expressionS *exp, > i386_operand_type types, const char *disp_start) > { > - i386_operand_type bigdisp; > int ret = 1; > > /* We do this to make sure that the section symbol is in > @@ -11210,10 +11209,10 @@ i386_finalize_displacement (segT exp_seg > i.types[this_operand].bitfield.disp8 = 1; > > /* Check if this is a displacement only operand. */ > - bigdisp = operand_type_and_not (i.types[this_operand], anydisp); > - if (operand_type_all_zero (&bigdisp)) > - i.types[this_operand] = operand_type_and (i.types[this_operand], > - types); > + if (!i.types[this_operand].bitfield.baseindex) > + i.types[this_operand] = > + operand_type_or (operand_type_and_not (i.types[this_operand], anydisp), > + operand_type_and (i.types[this_operand], types)); > > return ret; > } > --- a/opcodes/i386-gen.c > +++ b/opcodes/i386-gen.c > @@ -529,14 +529,14 @@ static initializer operand_type_init[] = > "Imm16|Imm32|Imm32S" }, > { "OPERAND_TYPE_IMM32_64", > "Imm32|Imm64" }, > - { "OPERAND_TYPE_IMM32_32S_DISP32", > - "Imm32|Imm32S|Disp32" }, > + { "OPERAND_TYPE_IMM32_32S_DISP32S", > + "Imm32|Imm32S|Disp32S" }, > { "OPERAND_TYPE_IMM64_DISP64", > "Imm64|Disp64" }, > - { "OPERAND_TYPE_IMM32_32S_64_DISP32", > - "Imm32|Imm32S|Imm64|Disp32" }, > - { "OPERAND_TYPE_IMM32_32S_64_DISP32_64", > - "Imm32|Imm32S|Imm64|Disp32|Disp64" }, > + { "OPERAND_TYPE_IMM32_32S_64_DISP32S", > + "Imm32|Imm32S|Imm64|Disp32S" }, > + { "OPERAND_TYPE_IMM32_32S_64_DISP32S_64", > + "Imm32|Imm32S|Imm64|Disp32S|Disp64" }, > { "OPERAND_TYPE_ANYIMM", > "Imm1|Imm8|Imm8S|Imm16|Imm32|Imm32S|Imm64" }, > }; > OK. Thanks. -- H.J.