From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-sender-0.a4lg.com (mail-sender.a4lg.com [153.120.152.154]) by sourceware.org (Postfix) with ESMTPS id A12B63858C74 for ; Thu, 15 Sep 2022 08:25:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org A12B63858C74 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=irq.a4lg.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=irq.a4lg.com Received: from [127.0.0.1] (localhost [127.0.0.1]) by mail-sender-0.a4lg.com (Postfix) with ESMTPSA id 64519300089; Thu, 15 Sep 2022 08:25:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=irq.a4lg.com; s=2017s01; t=1663230335; bh=nG2nlkmbirLWr/dc7Y0llSw8GLALFSm4KPB18Ku/T0c=; h=Message-ID:Date:Mime-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type:Content-Transfer-Encoding; b=PIds/hHDpVah94pSQ2mSmd55F2tyiAlEq4sQWJdeBGvFuthUvu7HiylXUXkO82N99 GZizwY5t0TJPskSLMPPBfGQs9XW0d1DPmnmdXxsPBN0gzBtL2ffZlhziiCJR74TJR5 smO5+GX/RYmjRok6XO//V6q9O7tfQU0B5ylGRY3c= Message-ID: <62b9e3a7-4dcd-40e7-b30f-7cb3776a161d@irq.a4lg.com> Date: Thu, 15 Sep 2022 17:25:32 +0900 Mime-Version: 1.0 Subject: Re: [PATCH 1/1] bfd, binutils, gas: Mark unused variables To: Jan Beulich Cc: binutils@sourceware.org References: <41d8346fa386205b021320b71b3db50898c27bd7.1663215243.git.research_trasio@irq.a4lg.com> <73538f81-579a-d712-9f34-84170a437491@suse.com> Content-Language: en-US From: Tsukasa OI In-Reply-To: <73538f81-579a-d712-9f34-84170a437491@suse.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-6.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,SPF_HELO_NONE,SPF_PASS,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 List-Id: On 2022/09/15 16:29, Jan Beulich wrote: > On 15.09.2022 06:17, Tsukasa OI via Binutils wrote: >> Clang generates a warning on unused (technically, written but not read >> thereafter) variables. By the default configuration (with "-Werror"), it >> causes a build failure (unless "--disable-werror" is specified). >> >> This commit, instead of just removing those variables, adds >> ATTRIBUTE_UNUSED attribute to them, which means they are *possibly* unused >> (can be used but no warnings occur when unused). > > May I ask why you chose to do so? I can see such a variable being consumed > inside an #ifdef, but being declared unconditionally as a reason, but > (looking just at code I'm a little familiar with) ... > >> --- a/gas/config/tc-riscv.c >> +++ b/gas/config/tc-riscv.c >> @@ -2303,7 +2303,7 @@ riscv_ip (char *str, struct riscv_cl_insn *ip, expressionS *imm_expr, >> char save_c = 0; >> struct riscv_opcode *insn; >> unsigned int regno; >> - int argnum; >> + int argnum ATTRIBUTE_UNUSED; >> const struct percent_op_match *p; >> struct riscv_ip_error error; >> error.msg = "unrecognized opcode"; >> --- a/ld/pe-dll.c >> +++ b/ld/pe-dll.c >> @@ -1510,8 +1510,9 @@ generate_reloc (bfd *abfd, struct bfd_link_info *info) >> int total_relocs = 0; >> int i; >> bfd_vma sec_page = (bfd_vma) -1; >> - bfd_vma page_ptr, page_count; >> - int bi; >> + bfd_vma page_ptr; >> + bfd_vma page_count ATTRIBUTE_UNUSED; >> + int bi ATTRIBUTE_UNUSED; >> bfd *b; >> struct bfd_section *s; >> > > ... in both of these cases there's truly no reading of the values, > so I don't see why they would need maintaining. > > Jan > Possibly because I'm a coward? ... I'm kidding but I must admit that I didn't take a time to review their actual uses and chose not to break someone's job (knowing that "someone" may not exist anymore). bfd/elf32-lm32.c: It seems unused rgot_count is a part of an unimplemented feature. I want to keep this variable. Existence of this variable was the first reason for me to keep "unused" variables. bfd/mmo.c: Likewise (though that "an unimplemented feature" is never implemented for over 10 years). I want to keep this variable (but may change my mind in the future). binutils/windmc.c: For this simple function, recovering i variable is very easy. It seems it's also a debug artifact from the earliest days. Removing is an option. gas/tc-riscv.c: I first thought it's a part of an unimplemented feature and to help implementing assembler something... but it seems unused from the start (possibly an artifact on the first contribution?). Removing is an option. ld/pe-dll.c: I reviewed thoroughly and concluded that, despite that they may have a debugging use, keeping this state over 20 years seems too long. Removing is an option. bfd/elf32-nds32.c: The first time this variables is added, someone was clearly debugging their code and it may be an artifact, too. Unlike RISC-V, it's not an artifact from the first NDS port and it was added in 2018. Recovering this variable is not hard but... Removing is ... possibly an option? I think I can remove unused variables from those files: * binutils/windmc.c * gas/tc-riscv.c * ld/pe-dll.c bfd/elf32-nds32.c is a candidate of unused variable removal. For bfd/elf32-lm32.c and bfd/mmo.c, I will choose to keep unused variables for now. If those variables are still unused in like 2030, I will change my mind. I'll at least remove unused variables on three files I shown above (and possibly on bfd/elf32-nds32.c) and resubmit that patch. Thanks, Tsukasa