From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id 7/5iAVW0w2PyaBcAWB0awg (envelope-from ) for ; Sun, 15 Jan 2023 03:07:49 -0500 Received: by simark.ca (Postfix, from userid 112) id EC3801E128; Sun, 15 Jan 2023 03:07:48 -0500 (EST) Authentication-Results: simark.ca; dkim=pass (1024-bit key; secure) header.d=sourceware.org header.i=@sourceware.org header.a=rsa-sha256 header.s=default header.b=ORvIGpgr; dkim-atps=neutral X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-7.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI, RDNS_DYNAMIC,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.6 Received: from sourceware.org (ip-8-43-85-97.sourceware.org [8.43.85.97]) (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 simark.ca (Postfix) with ESMTPS id 8CBCE1E112 for ; Sun, 15 Jan 2023 03:07:48 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id E82B83858C62 for ; Sun, 15 Jan 2023 08:07:47 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org E82B83858C62 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1673770067; bh=Ej+7T9K9x0PqmGmpZ/LopZN4eyKRVCACF7Ewf+DB0is=; h=Date:To:Cc:Subject:References:In-Reply-To:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=ORvIGpgrZd8kiIW1iGiBEHBGPGlRJ6/Abd3P93PSgul7vysjppGo+4HZpWSOHCTFH Si7nD36TAW/xyFeSQSTtfmT+kzyEBBjWVHDvvjgWV9XBj/g7wexb4GXQEijAdf/9dg J8s4jD7MMVAE1871iMMOnKBqr6rRrLxkgoLPPukw= Received: from smtp.gentoo.org (woodpecker.gentoo.org [140.211.166.183]) by sourceware.org (Postfix) with ESMTP id 93F603858D32 for ; Sun, 15 Jan 2023 08:07:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 93F603858D32 Received: by smtp.gentoo.org (Postfix, from userid 559) id C529F340EDB; Sun, 15 Jan 2023 08:07:21 +0000 (UTC) Date: Sun, 15 Jan 2023 03:07:20 -0500 To: Sam James Cc: Mark Wielaard , gdb-patches@sourceware.org Subject: Re: [PATCH] sim: common, microblaze, mn10300: handle signal.h defining REC_PC. Message-ID: References: <20230114232805.827713-1-mark@klomp.org> <0FE61864-9F15-4D26-B1AD-DC15FA4CF94D@gentoo.org> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="yZJ3LHjdlzQGMf7X" Content-Disposition: inline In-Reply-To: <0FE61864-9F15-4D26-B1AD-DC15FA4CF94D@gentoo.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: , From: Mike Frysinger via Gdb-patches Reply-To: Mike Frysinger Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org Sender: "Gdb-patches" --yZJ3LHjdlzQGMf7X Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On 15 Jan 2023 00:58, Sam James wrote: > > On 15 Jan 2023, at 00:22, Mike Frysinger wrote: > > On 15 Jan 2023 00:28, Mark Wielaard wrote: > >> In common sim-events.c, sim-signal.c, nrun.c and dv-sockser.c we > >> do need signal.h, but check whether REG_PC is defined (and then > >> undefine it) before including the sim headers. > >>=20 > >> It breaks the build on sparc because signal.h indirectly > >> includes /usr/include/sys/ucontext.h and defines REG_PC, > >> which is also defined in microblaze-opcm.h > >=20 > > i don't think this is correct. none of the files quoted use REG_PC, > > so undefining a random symbol in them doesn't make sense. nothing in > > sim/common/ uses REG_PC for that matter. >=20 > The original error (https://builder.sourceware.org/buildbot/#/builders/22= 9/builds/3) is: > ``` > In file included from ../../binutils-gdb/sim/mn10300/sim-main.h:41, > from ../../binutils-gdb/sim/common/dv-sockser.c:42: > ../../binutils-gdb/sim/mn10300/mn10300-sim.h:68: error: "REG_PC" redefine= d [-Werror] > 68 | #define REG_PC 9 > | > In file included from /usr/include/signal.h:316, > from ../gnulib/import/signal.h:52, > from ../../binutils-gdb/sim/common/dv-sockser.c:29: > /usr/include/sys/ucontext.h:111: note: this is the location of the previo= us definition > 111 | # define REG_PC (1) > | > ``` the arch sim-main.h shouldn't be bleeding random arch-specific headers into common headers. there's a todo in mn10300/sim-main.h about this with a breadcrumb for how to clean it up. i've cleaned up most of the arches at this point (~18), but i've got ~10 to go, and haven't gotten around to them yet. basically the cgen & igen based arches. > There's history of just ducking this in other projects, and I can't reall= y blame them: > https://patchwork.kernel.org/project/qemu-devel/patch/1490272961-1128-1-g= it-send-email-peter.maydell@linaro.org/ >=20 > Overall, we have: > ``` > $ grep -rsin "#define.*REG_PC" > sim/mn10300/mn10300_sim.h:57:#define PC (State.regs[REG_PC]) > sim/mn10300/mn10300_sim.h:74:#define REG_PC 9 > gas/config/tc-arm.c:744:#define REG_PC 15 > gprofng/libcollector/unwind.c:111:#define GET_PC(ctx) (((ucontext_t*)ctx)= ->uc_mcontext.gregs[REG_PC]) > opcodes/microblaze-opcm.h:77:#define REG_PC_MASK 0x8000 > opcodes/microblaze-opcm.h:101:#define REG_PC 32 /* PC. */ > include/opcode/cris.h:35:#define REG_PC (15) > include/opcode/cris.h:143:#define BDAP_PC_LOW (BDAP_INDIR_LOW + REG_P= C) > ``` there's more conflicts than REG_PC. it depends on the host & target, and what set of headers happened to be included. the REG_* namespace is a mess, and it's kind of a self-inflicted (i.e. GNU) problem. ptrace.h is another place where it sometimes comes up. REG_R# is a common conflict. > What do you prefer? >=20 > 1. Rename all the REG_* (ugly) > 2. #undef hack in each of the consumers where there's a #define for it? > 3. What Mark did in some misc. top-level sim place > 4. Beg every vendor to change their ucontext.h > 5. Something else? splattering #undef boilerplate around the codebase isn't going to happen. it's not maintainable. cleaning up the remaining sim-main.h headers is a known todo that probably makes the issue go away enough for the sim. somewhat ironically, i think the current state is due to portability issues with signal.h. the only reason sim is building with GNUisms enabled is to get access to the strsignal prototype in string.h. POSIX didn't adopt the function until 2008 edition. https://sourceware.org/git/?p=3Dbinutils-gdb.git;a=3Dcommit;h=3D2232061b1cc= f68bb1e46c95cab6f531831d72aa5 i bet we could drop AC_USE_SYSTEM_EXTENSIONS from the sim now since it's been in POSIX for more than a decade, and even when i landed that patch, it was for "old" systems, which makes them double old at this point. if we find a setup that still lacks strsignal support, we can add it to our local gnulib/ instead. although i think gnulib/ enables the system extensions too, so maybe dropping it from sim wouldn't help as much as i would like. -mike --yZJ3LHjdlzQGMf7X Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQIzBAABCAAdFiEEuQK1JxMl+JKsJRrUQWM7n+g39YEFAmPDtDgACgkQQWM7n+g3 9YEccBAA25/47gEFmMY2VxulNSIsrVAN4N2FEqGuJF+CXnK8LAc1VfX8iZiF4XhT xG+KyxTjuOHbVtLCUV+ECY5jDtMfg0sPILFb1kVZ9NUO9A07tCFju22f6wvk5LXv D0k1il6tgMpfwLKUcjiDTGndm6JFcgT2pWinFNFzwSOR8E9Ymofz0gdF+iiHloI9 JtJxn7iIjs7ssIyawIeYBbdhI2d1a1+bDDJQTmFlQT/ukFeW5fJJ3B1nDh+6lnKA lC1+vfWaD1DCLSRex9hmva6D2Y1QJvCkctT4AhA8pM4QNNPheQ0S1ocyInF/Wdse mytkt6xQzKzC/RJT+4O04vQrAsRQUklo7FNg9J/dRZyRBm0ILSUSWUX0/Q8RRGvM 6pAWetIfpoL8Zk0ahmINKFkramCGkwMiavVCgORZr+dQgWqmF1O33mBGPaPvzj02 yAmcLq29+zkNwqWKPxFvxgfFDbtA1VxMHpc9occsmvE/kLGm2vn3qEcr+Hau8CuH QltNrmde35V8Yz/71BuKYCfvvIUXKFKO0lnsEKWm9MFeY4tyc0CycNYIefzj8/0O nl0TbEJIwwQTPJPY8SgCwcyZnskNSrXmGWuCdaqLjrrmBbvgdw55HTrnylX6SbUG 4C1KugRL6a78MKOvR81BmuQn1qywoV03KlGmjweGb0xnOkQI4Dw= =rWvj -----END PGP SIGNATURE----- --yZJ3LHjdlzQGMf7X--