From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qk1-x734.google.com (mail-qk1-x734.google.com [IPv6:2607:f8b0:4864:20::734]) by sourceware.org (Postfix) with ESMTPS id 0AB14386185D for ; Tue, 11 May 2021 02:01:38 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0AB14386185D Received: by mail-qk1-x734.google.com with SMTP id o27so17399576qkj.9 for ; Mon, 10 May 2021 19:01:38 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:to:references:from:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=+t75QLef4btB5RSdwLE769SczsbJY/Eyh9uR+YK05mQ=; b=PEinFxDCqVMFFREnRxo8D67/+AsqOQGr/2fF43gMMDyRwZbGSXeUfZD5ONb0w2GO1Z y8X44IVWKwNqAJcCPTT9eGGbfCspuQlMQaccjNY+L89Us2kXAkKE+k101aKOddJc5qeh Nm3Wx/gu4cQ8ViNalZzTPDIij6h70Gq67Z34Ni/bL086afkiXTHyr55JtqCgdPiuFUmo AaK1JT78eO3hluBihcIAz8oGWnnfl5idi3g3H6gyqd5/+vHUAKec/NWyOoVhf9P2YMFk MdiPlBR3RLv3p5oSJTqcqwYso0/G0Jk4+Fmnme1SI966o7PpSwSoX2tMl5ZAm5orjy6T PtJw== X-Gm-Message-State: AOAM532r2+Pq24Areke8qdU93jgr5o+QOTDL8m2Ps+Z+QEHfLwvBFL9T Amqsf9HVda6tQkwC9jot8eYuAGvcsu2rmg== X-Google-Smtp-Source: ABdhPJwiGW6/PFFoGBTKvwND/GG3IfsY0BvUCLmV57ShVrk628YZN9pzD2keof0jS9H90oD3bcKX7g== X-Received: by 2002:a37:404a:: with SMTP id n71mr25857018qka.330.1620698497619; Mon, 10 May 2021 19:01:37 -0700 (PDT) Received: from ?IPv6:2804:7f0:4841:40ad:215f:36bd:5b9a:ad6? ([2804:7f0:4841:40ad:215f:36bd:5b9a:ad6]) by smtp.gmail.com with ESMTPSA id w7sm12718335qtn.91.2021.05.10.19.01.36 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Mon, 10 May 2021 19:01:37 -0700 (PDT) Subject: Re: [PATCH] [sim] Fix build failure in d10v sim To: gdb-patches@sourceware.org References: <20210510191423.3627307-1-luis.machado@linaro.org> From: Luis Machado Message-ID: Date: Mon, 10 May 2021 23:01:34 -0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.6 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 May 2021 02:01:39 -0000 On 5/10/21 7:30 PM, Mike Frysinger wrote: > On 10 May 2021 16:14, Luis Machado via Gdb-patches wrote: >> While building all targets on Ubuntu 20.04/aarch64, I ran into the following >> build error: >> >> In file included from /usr/include/string.h:495, >> from ../../bfd/bfd.h:48, >> from ../../../../repos/binutils-gdb/sim/d10v/interp.c:4: >> In function 'memset', >> inlined from 'sim_create_inferior' at ../../../../repos/binutils-gdb/sim/d10v/interp.c:1146:3: >> /usr/include/aarch64-linux-gnu/bits/string_fortified.h:71:10: error: ‘__builtin_memset’ offset [33, 616] from the object at ‘State’ is out of the bounds of referenced subobject ‘regs’ with type ‘reg_t[16]’ {aka ‘short unsigned int[16]’} at offset 0 [-Werror=array-bounds] >> 71 | return __builtin___memset_chk (__dest, __ch, __len, __bos0 (__dest)); >> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> cc1: all warnings being treated as errors >> make[3]: *** [Makefile:558: interp.o] Error 1 >> >> I looked at a different sim (cr16), and it zeroes out the entire State, not >> just the registers. > > it's clearing more than just the regs. it's clearing all the members in the > state struct from regs up to mem. so regs, cregs, sp, a, slot, etc... if > you want to change it, the comment in the header probably needs adjusting. Heh... the comment in the header says something and the one before memset says another that is completely different. > >> It is unclear why we have the casts to uintptr. > > because pointer math on diff types isn't allowed, and the 3rd arg to memset > needs to be a scalar integer. so casting to uintptr_t is the correct way to > calculate this value. Right. What I mean is I don't understand why the complexity to clear some data with memset while the comment is stating we should clear everything. Then again, it looks like it was copy/pasted from somewhere else and not documented properly in the .c file. The same seems to be happening with cr16. The header says we reset things until a certain member. But the .c code says we reset everything. > >> The following patch fixes this for me. >> >> - memset (&State.regs, 0, (uintptr_t)&State.mem - (uintptr_t)&State.regs); >> + memset (&State, 0, sizeof (State)); > > i think changing the first arg to &State is sufficient and keeping the mem-regs > calculation. and maybe add a static assert that &State == &State.regs ? Ok. I'll send a v2. Thanks for the input. The comments were throwing me off. > > this code needs to be rewritten fundamentally at some point :/. Yeah. It needs some TLC. > -mike >