From patchwork Fri Mar 7 10:38:55 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nick Clifton X-Patchwork-Id: 107493 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 914F43858019 for ; Fri, 7 Mar 2025 10:39:59 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 914F43858019 Authentication-Results: sourceware.org; dkim=pass (1024-bit key, unprotected) header.d=redhat.com header.i=@redhat.com header.a=rsa-sha256 header.s=mimecast20190719 header.b=PQX5oXDk X-Original-To: binutils@sourceware.org Delivered-To: binutils@sourceware.org Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTP id 2A2E03858435 for ; Fri, 7 Mar 2025 10:39:04 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 2A2E03858435 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=redhat.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=redhat.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 2A2E03858435 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=170.10.133.124 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1741343944; cv=none; b=hYfX3E9+85S7m1ezOJzJaXzoGo/30hS0x6F20dLefk7c6ZETVRDWEFJAhvWa6CvqPurmRVlCuMVjB8ZTrHLNBEAuhMojFTIaayBLEngEhuSmhSVtWE/EKoUro/kWo+YIznW+APDq3yw0fPJCqELyEobJRNhve5LnCQqCrId4yNM= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1741343944; c=relaxed/simple; bh=4AleIyK7emD/nVuFLhFvgfbJsmISJQIa4eLcXGRsMdA=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=lT+bdJK3mASD2J+GaceF+374KOHSvc8q2DVktkFGbkjq3VngSOdHV9hNzLJJdQNvVUxD7WXO9AZbAySnVpFT90jRdLZGAvYgPCRoYEfgg9tA3zo2cSdZbE+zPX0xqDQg2SEv+mCl7++fWRh41GbSHDaL67M1BjkmmiHKUQiNSkg= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 2A2E03858435 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1741343943; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type; bh=5OS68oJZrTCBNkZp/4c+b4b5WevpuTivKYtzX8+32w4=; b=PQX5oXDkNONAenVeaa9dx64TeN5/2ZU2TGLZ4linPmtW2UC7GD3mcWs6cn4nrsO09zXT1z U80oOj5i3ihKfHSJV+UtPxah3Fyg5BdN3HRCtqetq9iuNgxaE68Fkh7Dh79xGW/BbCDbwy P64IvLL2lfmymBQvlS8titMTugnE654= Received: from mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (ec2-35-165-154-97.us-west-2.compute.amazonaws.com [35.165.154.97]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-564-CMLeAKiFNKCKVjpmHOTSew-1; Fri, 07 Mar 2025 05:39:00 -0500 X-MC-Unique: CMLeAKiFNKCKVjpmHOTSew-1 X-Mimecast-MFC-AGG-ID: CMLeAKiFNKCKVjpmHOTSew_1741343939 Received: from mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.40]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mx-prod-mc-08.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id 848B318004A9 for ; Fri, 7 Mar 2025 10:38:59 +0000 (UTC) Received: from prancer.redhat.com (unknown [10.42.28.86]) by mx-prod-int-04.mail-002.prod.us-west-2.aws.redhat.com (Postfix) with ESMTPS id BA2A11955DCE for ; Fri, 7 Mar 2025 10:38:58 +0000 (UTC) From: Nick Clifton To: binutils@sourceware.org Subject: Commit: MSP430: GAS: Correct range check of imm20 operands Date: Fri, 07 Mar 2025 10:38:55 +0000 Message-ID: <87ecz9t9a8.fsf@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.0 on 10.30.177.40 X-Mimecast-Spam-Score: 0 X-Mimecast-MFC-PROC-ID: zoCXM4ZnotaZZlERCxHV9AWZlXXoFaMP706W6kUXCNM_1741343939 X-Mimecast-Originator: redhat.com X-Spam-Status: No, score=-9.7 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_NONE, TXREP 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.30 Precedence: list List-Id: Binutils mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: binutils-bounces~patchwork=sourceware.org@sourceware.org Hi Guys, I am applying the attached patch to fix the msp430 assembler's checking of 20-bit immediate values. The code used to check for negative values less than -(0x7ffff) but this is incorrect as it does not allow -524289. Whilst correcting this, and adding a few extra lines to the MSP430 testsuite to make sure that the checks work correctly I realised that it might not be obvious why positive values up to 0xfffff are allowed for a signed immediate field. So I have added in a couple of comments explaining about the special native of hex constants in immediate fields. Cheers Nick diff --git a/gas/config/tc-msp430.c b/gas/config/tc-msp430.c index 27b1d8399a7..7ce061b1cf7 100644 --- a/gas/config/tc-msp430.c +++ b/gas/config/tc-msp430.c @@ -3584,7 +3584,13 @@ msp430_operands (struct msp430_opcode_s * opcode, char * line) if (op1.exp.X_op == O_constant) { n = op1.exp.X_add_number; - if (n > 0xfffff || n < - (0x7ffff)) + /* Strictly speaking the positive value test should be for "n > 0x7ffff" + but traditionally when specifying immediates as hex values any valid + bit pattern is allowed. Hence "suba #0xfffff, r6" is allowed, and so + the positive value test has to be "n > 0xfffff". + FIXME: We could pre-parse the expression to find out if it starts with + 0x and only then allow positive values > 0x7fffff. */ + if (n > 0xfffff || n < -0x80000) { as_bad (_("expected value of first argument of %s to fit into 20-bits"), opcode->name); diff --git a/gas/testsuite/gas/msp430/bad.l b/gas/testsuite/gas/msp430/bad.l index 7b685830678..3ea1b7b001e 100644 --- a/gas/testsuite/gas/msp430/bad.l +++ b/gas/testsuite/gas/msp430/bad.l @@ -6,13 +6,16 @@ [^:]*:10: Warning: no size modifier after period, .w assumed [^:]*:10: Warning: a NOP might be needed here, before this interrupt state change [^:]*:11: Error: instruction bis.a does not exist -[^:]*:16: Warning: a NOP might also be needed here, after the instruction that changed interrupt state -[^:]*:16: Warning: a NOP might be needed here, before an interrupt enable instruction -[^:]*:25: Warning: a NOP might be needed here, after an interrupt disable instruction -[^:]*:26: Warning: a NOP might be needed here, after an interrupt enable instruction -[^:]*:29: Warning: a NOP might be needed here, after an interrupt disable instruction -[^:]*:31: Warning: a NOP might be needed here, after an interrupt disable instruction -[^:]*:32: Warning: a NOP might be needed here, after an interrupt disable instruction -[^:]*:33: Warning: a NOP might be needed here, after an interrupt disable instruction -[^:]*:34: Warning: a NOP might be needed here, after an interrupt enable instruction +[^:]*:14: Warning: a NOP might also be needed here, after the instruction that changed interrupt state +[^:]*:14: Error: expected value of first argument of adda to fit into 20-bits +[^:]*:15: Error: expected value of first argument of adda to fit into 20-bits +[^:]*:22: Error: expected value of first argument of adda to fit into 20-bits +[^:]*:27: Warning: a NOP might be needed here, before an interrupt enable instruction +[^:]*:36: Warning: a NOP might be needed here, after an interrupt disable instruction +[^:]*:37: Warning: a NOP might be needed here, after an interrupt enable instruction +[^:]*:40: Warning: a NOP might be needed here, after an interrupt disable instruction +[^:]*:42: Warning: a NOP might be needed here, after an interrupt disable instruction +[^:]*:43: Warning: a NOP might be needed here, after an interrupt disable instruction +[^:]*:44: Warning: a NOP might be needed here, after an interrupt disable instruction +[^:]*:45: Warning: a NOP might be needed here, after an interrupt enable instruction [^:]*: Warning: a NOP might be needed after the interrupt state change at the end of the file diff --git a/gas/testsuite/gas/msp430/bad.s b/gas/testsuite/gas/msp430/bad.s index ae2db2f01cd..864903118da 100644 --- a/gas/testsuite/gas/msp430/bad.s +++ b/gas/testsuite/gas/msp430/bad.s @@ -10,6 +10,17 @@ mov. r1, r2 bis.a #8, r2 + ;; Make sure that the range checking gets #imm20 correct. + adda #-524289, r12 + adda #0x180000, r10 + ;; An immediate of #524288 will not trigger an error because + ;; positive values up to 1048575 are allowed. This is because + ;; assembler programmers often specify bit patterns as immediate + ;; values in hex knowing that they will fit, despite the fact + ;; that those same values, when viewed as integers, are out of range. + ;; eg: adda 0xfffff, r1 + adda #1048576, r11 + ;;; FIXME: Add more tests of assembler error detection here. ;; A NOP is needed *before* an EINT instruction. @@ -17,7 +28,7 @@ nop ;; And *after* a DINT instruction. dint - + ;; Changing interrupt states in two successive instructions ;; might cause an interrupt to be missed. The assembler ;; should warn about this, if the -mz command line option @@ -35,3 +46,4 @@ ;; We will also get a warning if the last instruction in the file ;; changes the interrupt state, since this file could be linked ;; with another that starts with an interrupt change. + diff --git a/gas/testsuite/gas/msp430/msp430x.d b/gas/testsuite/gas/msp430/msp430x.d index ecaef8fd6a4..fe4c6345bf3 100644 --- a/gas/testsuite/gas/msp430/msp430x.d +++ b/gas/testsuite/gas/msp430/msp430x.d @@ -230,3 +230,7 @@ Disassembly of section .text: 0+0406 <[^>]*> 84 18 44 11 rpt r4 \{ rrax.a r4 ; 0+040a <[^>]*> 44 18 45 55 rpt #5 \{ rlax.b r5 ; 0+040e <[^>]*> 05 18 46 66 rpt #6 \{ rlcx.a r6 ; +0+0412 <[^>]*> ac 08 00 00 adda #524288,r12 ;0x80000 +0+0416 <[^>]*> ab 07 ff ff adda #524287,r11 ;0x7ffff +0+041a <[^>]*> ac 08 00 00 adda #524288,r12 ;0x80000 +0+041e <[^>]*> ab 07 ff ff adda #524287,r11 ;0x7ffff diff --git a/gas/testsuite/gas/msp430/msp430x.s b/gas/testsuite/gas/msp430/msp430x.s index 8fef8827fb0..e6ae903cb12 100644 --- a/gas/testsuite/gas/msp430/msp430x.s +++ b/gas/testsuite/gas/msp430/msp430x.s @@ -281,3 +281,9 @@ foo: rpt r4 { rrax.a r4 rpt #5 { rlax.b r5 rpt #6 { rlcx.a r6 + + ;; Make sure that the range checking gets #imm20 correct. + adda #-524288, r12 + adda #524287, r11 + adda #-0x80000, r12 + adda #0x7ffff, r11 diff --git a/gas/testsuite/gas/msp430/opcode.d b/gas/testsuite/gas/msp430/opcode.d index 9212d89e69d..04d1081ad72 100644 --- a/gas/testsuite/gas/msp430/opcode.d +++ b/gas/testsuite/gas/msp430/opcode.d @@ -43,3 +43,4 @@ Disassembly of section .text: 0+062 <[^>]*> 3f 40 f0 00 mov #240, r15 ;#0x00f0 0+066 <[^>]*> 30 40 00 00 br #0x0000 ; 0+06a <[^>]*> 92 52 00 02 72 01 add &0x0200,&0x0172 ;0x0200 + diff --git a/gas/testsuite/gas/msp430/opcode.s b/gas/testsuite/gas/msp430/opcode.s index 4924a60177d..fc29b8c8b0f 100644 --- a/gas/testsuite/gas/msp430/opcode.s +++ b/gas/testsuite/gas/msp430/opcode.s @@ -55,3 +55,5 @@ main: ;; This next instruction triggered a bug which ;; was fixed by a patch to msp430-dis.c on Jan 2, 2004 add &0x200, &0x172 + +