From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id ksrxBnwWiWX81ikAWB0awg (envelope-from ) for ; Mon, 25 Dec 2023 00:43:24 -0500 Authentication-Results: simark.ca; dkim=pass (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.a=rsa-sha256 header.s=20230601 header.b=HM3iQ/h/; dkim-atps=neutral Received: by simark.ca (Postfix, from userid 112) id 0A7CC1E0C3; Mon, 25 Dec 2023 00:43:24 -0500 (EST) Received: from server2.sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id C72951E091 for ; Mon, 25 Dec 2023 00:43:21 -0500 (EST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 3810D3858C20 for ; Mon, 25 Dec 2023 05:43:21 +0000 (GMT) Received: from mail-pg1-x52b.google.com (mail-pg1-x52b.google.com [IPv6:2607:f8b0:4864:20::52b]) by sourceware.org (Postfix) with ESMTPS id 8A26E3858C98 for ; Mon, 25 Dec 2023 05:42:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 8A26E3858C98 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 8A26E3858C98 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52b ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703482974; cv=none; b=YXSAUrrJFm0dihf4zzkqXX7lUAHVaMsFG/w7ZDAdN1HMtaepk0H5PaXdAEQMHXptytKF26WpdwXEg+CRLmImGOdmMihDg+ML3JobFGS0i+elREqhq0ZNKHaXsKFj2QnU2b7D0pfGj6jtG44pQVJEZLoyOgGQ5YtnrOCjRP0aRHU= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1703482974; c=relaxed/simple; bh=lXwhQOKL0fbBDnKRWkjIXoDaUH8EB45CnWll4ipdtSo=; h=DKIM-Signature:MIME-Version:From:Date:Message-ID:Subject:To; b=CKXVP2eF+g1ezZuAt8uKsWTCO46X/ZbD2Pc5TLrTwN9JVL9vzQDJvn7+gqaLF+3gdsYaZGolMpi2wzL1f5ANIvbwYS4yRuKJ+kDu4zgS4ZF4WjJyFWu8/YAQdA+NN2vZ9bIQe654Cq+kwNZruV6Lcx+Vu5G1fhu4JjMkkRf7JmE= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52b.google.com with SMTP id 41be03b00d2f7-5cdfed46372so1377167a12.3 for ; Sun, 24 Dec 2023 21:42:48 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1703482967; x=1704087767; darn=sourceware.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=1XrDpQ8KmhDOZrnOWpa5iRLbfhGJCr/XzXHzqP2D80o=; b=HM3iQ/h/Yi7aaz+D0K2NXNsRCZe/cgSL+ez0OpfjLrSXuLAcN3zpf0jpdUCgnHLz0J 8U4eObPBi/+nZecZEhAr5n59XiRWfKKOUBAClk1DqZhBSyxLPH5zAkxv199swEARhOph hJtH6srgJis8cfJWdCqAfdBSBy0t9nS19xKKDulxbF6WewU8o+6dkTyJdSTL5lQtZ1J5 NLau7Nz/A+oTXstWjkv0alpPivuUUzfjOngARQLzUPOX2GRv2z6TGRCOrUkb4XSbVnQF 5I2ecKBlH5ez3oyZP5GLa4JQR9mQfE2+nd393Qtxkxn8hB2SGcO1q52V3jRZA7m7BEbf /6cA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1703482967; x=1704087767; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=1XrDpQ8KmhDOZrnOWpa5iRLbfhGJCr/XzXHzqP2D80o=; b=AriS6rEYlaGEIYjrZHD77X21+TIvwD73PXVbFL6I/L4fiGu306zqNxJPy8W0VU4u4r ZkFOL3NscbulKX0PQJ4lgk4zLgKqhXIwtn2vkVJ6IiJ1JdmiJ0q5XLSfTRb+50U0dnLg nDD15Wl52qg4O+t3GpO9Bxyay2KtL6+hjKnOm0aksXAsOk3Q/ro/TOzpXiKgKAYQA88X wQQm/LErsfeE3KZSYyZYO+c9u0vUZTDEkJUSHv0TZTviISGElB6yJrQyXT1HhM/KhRWz UxC9gagYfxMoOR+YdRhJMdYpFRe4Fpn2Uab5sR6txotBCIga+4QY+Y5KHff9A1sow0IV h2ng== X-Gm-Message-State: AOJu0Yz0c/XTJbXS0b8xBsdNHH0kvpbk30dp1AFtdAsBLo6YzZ9w8IST QOYC5DD4hz51CZReIJ3lx+E21hfZs/G5sRdvoi1rM41U X-Google-Smtp-Source: AGHT+IGSt3ovgQULr5C/1NJj3EzMdQMt6+Ek6tQKKCAn3q/mMXUbm6a/B7fQscsxe53rKDrKziWa3PTFOgwi1vvhPos= X-Received: by 2002:a05:6a20:8888:b0:187:db8b:e3e8 with SMTP id d8-20020a056a20888800b00187db8be3e8mr5720221pzf.39.1703482967280; Sun, 24 Dec 2023 21:42:47 -0800 (PST) MIME-Version: 1.0 References: <20231211114252.451602-2-zeck654321@gmail.com> <87jzpkg7kh.fsf@redhat.com> <87il4v4iia.fsf@redhat.com> In-Reply-To: <87il4v4iia.fsf@redhat.com> From: Zeck S Date: Sun, 24 Dec 2023 23:42:34 -0600 Message-ID: Subject: Re: [RFC][PATCH?] fixed some segfaults and bugs in mdebug support To: Andrew Burgess Cc: gdb-patches@sourceware.org Content-Type: multipart/alternative; boundary="000000000000254fda060d4f09e2" X-Spam-Status: No, score=-9.8 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_ENVFROM_END_DIGIT, FREEMAIL_FROM, GIT_PATCH_0, HTML_MESSAGE, 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: gdb-patches@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gdb-patches-bounces+public-inbox=simark.ca@sourceware.org --000000000000254fda060d4f09e2 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Wanted to say, still working on this, would like to get some tests in. Occupied a bit because of the time of year. Hoping to get something together in January. Also, really really appreciate that you maintainers are willing to keep this code around at all, and have up to this point. I know it's very niche. There are N64 (and PS2) reverse engineering projects, that either use this part of GDB fairly directly, by gdb stubs in the emulator, or referenced its code when trying to parse symbols either mistakenly shipped with old games, or produced from old compilers when trying to make matching binaries. I'm not particularly involved in the scene, beyond observing, but I gather some other people have patched things in this code privately, or are using old GDB versions that worked. Started working on this project because I was doing some N64 reversing. Seemed like a waste to not just go the extra mile to get the fixes shared, if possible. For an example of its use, the Ares N64 emulator just got gdb stub support this November, and it's possible to use an ELF file from a build of the Super Mario 64 decompilation project to debug the original ROM, essentially perfectly, with views of threads and everything else, after patching GDB. When I started on this, I'd just finished writing a very basic stub for the Mupen64plus emulator (turned out to be redundant, I'd somehow missed some developer forks that had one). On Mon, Dec 18, 2023 at 10:50=E2=80=AFAM Andrew Burgess wrote: > Zeck S writes: > > > I definitely would like to add some tests, or tweak some existing tests= ! > > Just was not sure how normal that was, or if it would almost be viewed > > "cheating" or something. > > I don't think it would be considered cheating. I'd just make sure to > place a comment at the head of the file explaining that the test is > deliberately simple in order to compile with XXX compiler and that > changes should be made with care. This will pretty much guarantee > nobody else will dare change the test's source code :) > > As Tom said, the mdebug stuff is not used much, so if you posted an > updated patch without any tests, I'd be happy to approve it. It's your > choice. > > Thanks, > Andrew > > > > I really wanted to get the test suite running, which is why I made the > > qemu-user board. > > And it has been very useful, but legitimately some of the tests just > can't > > pass (or sometimes even build) with this nasty old compiler and symbol > > format combination. > > If I can make a few new tests I can work around all of this with relati= ve > > ease. > > > > I'll address everything else as well! > > > > I'm tempted to make some kind of container for making running tests wit= h > > this setup easier. > > It's all a bit ridiculous, wrapping an old compiler and a modern linker > in > > a bash script, then the compiler runs in an emulator, the built > executable > > runs in another emulator... > > If I do that I'll make it public somewhere. > > > > Thanks for the feedback. I know this is a weird old format, but it is > > actually becoming slightly more relevant again with the N64 game > > decompilation projects. > > > > On Mon, Dec 11, 2023 at 8:03=E2=80=AFAM Andrew Burgess > wrote: > > > >> Zeck S writes: > >> > >> > I'm not quite sure what to do. > >> > I have done some testing using the qemu-mips board I made. > >> > There are a lot of limitations I'm running into. > >> > I can just run as much of the testsuite as I can and see what happen= s. > >> > > >> > > >> > Here are the main problems: > >> > 1. IDO only supports ansi c. This is a problem for some headers in > glibc > >> which use c99. > >> > 2. Test code sometimes has GCC features in them like asm() > >> > 3. Test expectations sometimes assume features from nicer formats li= ke > >> DWARF. > >> > > >> > I knew I probably couldn't get standard library stuff working. > >> > Even if I hack past the C99 issues (variadic macros), > >> > there's also problems > >> > with compiler specific files like stddef.h. > >> > > >> > I started testing by just running gdb.base/info-types-c.exp alone, > >> > since I knew I fixed bugs in that area, and the test didn't use > stdio.h. > >> > Found that the .c file used asm() which IDO doesn't support. > >> > Commented that code out, ran the test. Failed. > >> > Fixed a segfault that had been introduced since I started this > project. > >> > Still failed. > >> > Fixed some typedef handling to get closer to the expected output. > >> > However, the expected output includes line numbers for types. > >> > mdebug format doesn't include line information for types. > >> > There are also differences in output stemming from > >> > mdebug assuming the debugger knows size information etc about "basic= " > >> > types, unlike dwarf, which includes information about them. > >> > > >> > So at this point, I realize, I've modified the .c file for the test > >> > which already seems not great, and I realize due to limitations of > >> mdebug, > >> > I can't ever make the test pass. > >> > Not sure how to demonstrate this as progress, but uh, it feels like > >> > progress? > >> > >> So, I know nothing about targets that make use of ECOFF / mdebug forma= t > >> stuff, and I don't really have time to try setting up a test > >> environment. > >> > >> I guess my expectation for at least some of these fixes would be that > >> you could write a small sample program that does compile with your > >> compiler, and then a .exp file which triggers the right GDB commands t= o > >> expose the bug. > >> > >> From what you've said, it doesn't sound like the existing test framewo= rk > >> actually has problems invoking the compiler and running the test, it's > >> just that the test has too many DWARF/ISO-C assumptions within. > >> > >> Personally, I'm a huge fan of writing tests, I think it is worth the > >> effort in the long run, so I think it would be worth your time to try > >> and write some new tests for these issues. > >> > >> That said, given the nature of these fixes and the difficulties you're > >> experiencing, if you don't feel that's possible, I'm not going to oppo= se > >> merging these fixes without tests... > >> > >> ... but I do have a couple of style issues that would need fixing firs= t, > >> see inline below: > >> > >> > > >> > > >> > For the curious, this is the bash script I wrapped IDO with. > >> > stderr is directed to dev null, because qemu-irix puts some non fata= l > >> > errors there, which confuses the test runner. > >> > Put the script in a directory in PATH. > >> > Passing an executable path as CC_FOR_TARGET does not work. > >> > It must be the name of a file in PATH. > >> > > >> > #!/bin/sh > >> > > >> > echo "$@" >> theargs > >> > > >> > for arg do > >> > if [ "$arg" =3D "-static" ] > >> > then > >> > link=3D1 > >> > fi > >> > done > >> > > >> > if [ "$link" =3D 1 ] > >> > then > >> > mips-unknown-linux-gnu-gcc "$@" > >> > else > >> > for arg do > >> > shift > >> > [ "$arg" =3D "-fdiagnostics-color=3Dnever" ] && continue > >> > [ "$arg" =3D "-g" ] && continue > >> > set -- "$@" "$arg" > >> > done > >> > > >> > /path/to/qemu-irix -L /path/to/ido/ido7.1_compiler > >> /path/to/ido/ido7.1_compiler/usr/bin/cc -Xcpluscomm -nostdlib -nostdin= c > >> -mips2 -g2 "$@" 2>/dev/null > >> > fi > >> > > >> > > >> > --- > >> > gdb/mdebugread.c | 89 > ++++++++++++++++++++++++++++-------------------- > >> > 1 file changed, 52 insertions(+), 37 deletions(-) > >> > > >> > diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c > >> > index cd6638224e7..191932e91cd 100644 > >> > --- a/gdb/mdebugread.c > >> > +++ b/gdb/mdebugread.c > >> > @@ -237,7 +237,7 @@ static struct type *new_type (char *); > >> > enum block_type { FUNCTION_BLOCK, NON_FUNCTION_BLOCK }; > >> > > >> > static struct block *new_block (struct objfile *objfile, > >> > - enum block_type, enum language); > >> > + enum block_type, enum language, bool > >> isGlobal); > >> > >> GDB style is for snake_case rather than camelCase, so: > >> s/isGlobal/is_global/ throughout. > >> > >> > > >> > static struct compunit_symtab *new_symtab (const char *, int, struc= t > >> objfile *); > >> > > >> > @@ -246,7 +246,7 @@ static struct linetable *new_linetable (int); > >> > static struct blockvector *new_bvect (int); > >> > > >> > static struct type *parse_type (int, union aux_ext *, unsigned int, > int > >> *, > >> > - int, const char *); > >> > + int, const char *, bool); > >> > > >> > static struct symbol *mylookup_symbol (const char *, const struct > block > >> *, > >> > domain_enum, enum address_class= ); > >> > @@ -572,7 +572,7 @@ add_data_symbol (SYMR *sh, union aux_ext *ax, in= t > >> bigend, > >> > || sh->sc =3D=3D scNil || sh->index =3D=3D indexNil) > >> > s->set_type (builtin_type (objfile)->nodebug_data_symbol); > >> > else > >> > - s->set_type (parse_type (cur_fd, ax, sh->index, 0, bigend, > name)); > >> > + s->set_type (parse_type (cur_fd, ax, sh->index, 0, bigend, name= , > >> false)); > >> > /* Value of a data symbol is its memory address. */ > >> > } > >> > > >> > @@ -705,7 +705,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char > >> *ext_sh, int bigend, > >> > break; > >> > } > >> > s->set_value_longest (svalue); > >> > - s->set_type (parse_type (cur_fd, ax, sh->index, 0, bigend, > name)); > >> > + s->set_type (parse_type (cur_fd, ax, sh->index, 0, bigend, > name, > >> false)); > >> > add_symbol (s, top_stack->cur_st, top_stack->cur_block); > >> > break; > >> > > >> > @@ -761,7 +761,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char > >> *ext_sh, int bigend, > >> > t =3D builtin_type (objfile)->builtin_int; > >> > else > >> > { > >> > - t =3D parse_type (cur_fd, ax, sh->index + 1, 0, bigend, name= ); > >> > + t =3D parse_type (cur_fd, ax, sh->index + 1, 0, bigend, name= , > >> false); > >> > if (strcmp (name, "malloc") =3D=3D 0 > >> > && t->code () =3D=3D TYPE_CODE_VOID) > >> > { > >> > @@ -805,7 +805,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, char > >> *ext_sh, int bigend, > >> > s->type ()->set_is_prototyped (true); > >> > > >> > /* Create and enter a new lexical context. */ > >> > - b =3D new_block (objfile, FUNCTION_BLOCK, s->language ()); > >> > + b =3D new_block (objfile, FUNCTION_BLOCK, s->language (), fal= se); > >> > s->set_value_block (b); > >> > b->set_function (s); > >> > b->set_start (sh->value); > >> > @@ -1135,7 +1135,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, cha= r > >> *ext_sh, int bigend, > >> > } > >> > > >> > top_stack->blocktype =3D stBlock; > >> > - b =3D new_block (objfile, NON_FUNCTION_BLOCK, psymtab_languag= e); > >> > + b =3D new_block (objfile, NON_FUNCTION_BLOCK, psymtab_languag= e, > >> false); > >> > b->set_start (sh->value + top_stack->procadr); > >> > b->set_superblock (top_stack->cur_block); > >> > top_stack->cur_block =3D b; > >> > @@ -1247,7 +1247,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, cha= r > >> *ext_sh, int bigend, > >> > f->set_loc_bitpos (sh->value); > >> > bitsize =3D 0; > >> > f->set_type (parse_type (cur_fd, ax, sh->index, &bitsize, > bigend, > >> > - name)); > >> > + name, false)); > >> > f->set_bitsize (bitsize); > >> > } > >> > break; > >> > @@ -1269,7 +1269,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax, cha= r > >> *ext_sh, int bigend, > >> > pend =3D is_pending_symbol (cur_fdr, ext_sh); > >> > if (pend =3D=3D NULL) > >> > { > >> > - t =3D parse_type (cur_fd, ax, sh->index, NULL, bigend, name)= ; > >> > + t =3D parse_type (cur_fd, ax, sh->index, NULL, bigend, name, > true); > >> > add_pending (cur_fdr, ext_sh, t); > >> > } > >> > else > >> > @@ -1382,7 +1382,7 @@ basic_type (int bt, struct objfile *objfile) > >> > if (map_bt[bt]) > >> > return map_bt[bt]; > >> > > >> > - type_allocator alloc (objfile, get_current_subfile ()->language); > >> > + type_allocator alloc (objfile, psymtab_language); > >> > >> With fixes like this, it can (I think) be useful to mention the commit > >> that is being fixed, in this case 76fc0f62138e. Wouldn't be much, I'd > >> just say: > >> > >> Commit 76fc0f62138e added calls to 'get_current_subfile ()->language= ', > >> but at the time this was known to be a bit of a guess, and untested. > >> We should actually have used 'psymtab_language' because .... > >> > >> Then, if in the future, someone looks at your patch and wonders why wa= s > >> this change made? There's a super quick way that can find the history > >> of this code, and understand the original change, and your update. > >> > >> > > >> > switch (bt) > >> > { > >> > @@ -1514,7 +1514,7 @@ basic_type (int bt, struct objfile *objfile) > >> > > >> > static struct type * > >> > parse_type (int fd, union aux_ext *ax, unsigned int aux_index, int > *bs, > >> > - int bigend, const char *sym_name) > >> > + int bigend, const char *sym_name, bool isStTypedef) > >> > >> Switch to snake_case for isStTypedef. > >> > >> > { > >> > TIR t[1]; > >> > struct type *tp =3D 0; > >> > @@ -1571,7 +1571,7 @@ parse_type (int fd, union aux_ext *ax, unsigne= d > >> int aux_index, int *bs, > >> > } > >> > } > >> > > >> > - type_allocator alloc (mdebugread_objfile, get_current_subfile > >> ()->language); > >> > + type_allocator alloc (mdebugread_objfile, psymtab_language); > >> > > >> > /* Move on to next aux. */ > >> > ax++; > >> > @@ -1628,7 +1628,7 @@ parse_type (int fd, union aux_ext *ax, unsigne= d > >> int aux_index, int *bs, > >> > xref_fh =3D get_rfd (fd, rf); > >> > xref_fd =3D xref_fh - debug_info->fdr; > >> > tp =3D parse_type (xref_fd, debug_info->external_aux + > >> xref_fh->iauxBase, > >> > - rn->index, NULL, xref_fh->fBigendian, sym_name); > >> > + rn->index, NULL, xref_fh->fBigendian, sym_name, > false); > >> > } > >> > > >> > /* All these types really point to some (common) MIPS type > >> > @@ -1785,6 +1785,13 @@ parse_type (int fd, union aux_ext *ax, unsign= ed > >> int aux_index, int *bs, > >> > if (t->continued) > >> > complaint (_("illegal TIR continued for %s"), sym_name); > >> > > >> > + if (isStTypedef) > >> > + { > >> > + struct type *wrap =3D alloc.new_type (TYPE_CODE_TYPEDEF, 0, > >> sym_name); > >> > + wrap->set_target_type(tp); > >> > + tp =3D wrap; > >> > + } > >> > >> GNU style indents the '{' and '}', this should look like: > >> > >> if (isStTypedef) > >> { > >> struct type *wrap =3D alloc.new_type (TYPE_CODE_TYPEDEF, 0, > sym_name); > >> wrap->set_target_type(tp); > >> tp =3D wrap; > >> } > >> > >> > + > >> > return tp; > >> > } > >> > > >> > @@ -1839,7 +1846,7 @@ upgrade_type (int fd, struct type **tpp, int t= q, > >> union aux_ext *ax, int bigend, > >> > > >> > indx =3D parse_type (fh - debug_info->fdr, > >> > debug_info->external_aux + fh->iauxBase, > >> > - id, NULL, bigend, sym_name); > >> > + id, NULL, bigend, sym_name, false); > >> > > >> > /* The bounds type should be an integer type, but might be > >> anything > >> > else due to corrupt aux entries. */ > >> > @@ -2154,8 +2161,7 @@ parse_external (EXTR *es, int bigend, const > >> section_offsets §ion_offsets, > >> > with that and do not need to reorder our linetables. */ > >> > > >> > static void > >> > -parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines, > >> > - CORE_ADDR lowest_pdr_addr) > >> > +parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int maxlines) > >> > >> As above, this seems to be reverting parts of 1acc9dca423f78e4, it wou= ld > >> be great to more of an explanation for why that commit was wrong. > >> > >> Remember, someone thought that commit was correct once. You're lookin= g > >> at their code and thinking they got it wrong. What's to stop someone > >> looking at your code and thinking _you_ got it wrong? > >> > >> Hopefully, what stops that is that you add more details in the commit > >> message, along with a link to the previous commit. > >> > >> > { > >> > unsigned char *base; > >> > int j, k; > >> > @@ -2169,7 +2175,6 @@ parse_lines (FDR *fh, PDR *pr, struct linetabl= e > >> *lt, int maxlines, > >> > for (j =3D 0; j < fh->cpd; j++, pr++) > >> > { > >> > CORE_ADDR l; > >> > - CORE_ADDR adr; > >> > unsigned char *halt; > >> > > >> > /* No code for this one. */ > >> > @@ -2186,9 +2191,7 @@ parse_lines (FDR *fh, PDR *pr, struct linetabl= e > >> *lt, int maxlines, > >> > halt =3D base + fh->cbLine; > >> > base +=3D pr->cbLineOffset; > >> > > >> > - adr =3D pr->adr - lowest_pdr_addr; > >> > - > >> > - l =3D adr >> 2; /* in words */ > >> > + l =3D pr->adr >> 2; /* in words */ > >> > for (lineno =3D pr->lnLow; base < halt;) > >> > { > >> > count =3D *base & 0x0f; > >> > @@ -4053,6 +4056,14 @@ mdebug_expand_psymtab (legacy_psymtab *pst, > >> struct objfile *objfile) > >> > { > >> > (*swap_pdr_in) (cur_bfd, pdr_ptr, pdr_in); > >> > > >> > + char *sym_ptr =3D (char *)(debug_info->external_sym) + > >> > + (fh->isymBase + pdr_in->isym) * external_sym_size; > >> > >> The layout seems wrong here. There should for sure be a single space > >> after '(char *)', and the '+' should be at the start of the new line > >> rather than the end of the previous line. > >> > >> Additionally, we usually wrap expressions like this within '( ... )' t= o > >> force editors to align things better. Turns out that this pattern of > >> code is repeated lots in this file. Search for: '* external_sym_size' > >> and you'll find lots of example for how to align this code. I'd go > >> with: > >> > >> char *sym_ptr =3D ((char *) debug_info->external_sym > >> + (fh->isymBase + pdr_in->isym) > >> * external_sym_size); > >> > >> maybe? > >> > >> > + > >> > + SYMR sh; > >> > + (*swap_sym_in) (cur_bfd, sym_ptr, &sh); > >> > + > >> > + pdr_in->adr =3D sh.value; > >> > + > >> > /* Determine lowest PDR address, the PDRs are not always > >> > sorted. */ > >> > if (pdr_in =3D=3D pr_block.data ()) > >> > @@ -4154,16 +4165,16 @@ mdebug_expand_psymtab (legacy_psymtab *pst, > >> struct objfile *objfile) > >> > { > >> > (*swap_pdr_in) (cur_bfd, pdr_ptr, pdr_in); > >> > > >> > - /* Determine lowest PDR address, the PDRs are not > always > >> > - sorted. */ > >> > - if (pdr_in =3D=3D pr_block.data ()) > >> > - lowest_pdr_addr =3D pdr_in->adr; > >> > - else if (pdr_in->adr < lowest_pdr_addr) > >> > - lowest_pdr_addr =3D pdr_in->adr; > >> > + sym_ptr =3D (char *)(debug_info->external_sym) + > >> > + (fh->isymBase + pdr_in->isym) * > external_sym_size; > >> > >> Similar layout issues to above. > >> > >> > + > >> > + SYMR sh; > >> > + (*swap_sym_in) (cur_bfd, sym_ptr, &sh); > >> > + > >> > + pdr_in->adr =3D sh.value; > >> > >> These lines are all indented using only spaces. GDB (unfortunately) > >> uses a hybrid tab and space approach for indentation. If you 'git dif= f' > >> and look at this file, git _should_ highlight the whitespace issues fo= r > >> you -- we have a .gitattributes file in the binutils-gdb root that ask= s > >> for whitespace issues to be highlighted. > >> > >> > } > >> > > >> > - parse_lines (fh, pr_block.data (), lines, maxlines, > >> > - lowest_pdr_addr); > >> > + parse_lines (fh, pr_block.data (), lines, maxlines); > >> > if (lines->nitems < fh->cline) > >> > lines =3D shrink_linetable (lines); > >> > > >> > @@ -4291,7 +4302,7 @@ cross_ref (int fd, union aux_ext *ax, struct > type > >> **tpp, > >> > rf =3D rn->rfd; > >> > } > >> > > >> > - type_allocator alloc (mdebugread_objfile, get_current_subfile > >> ()->language); > >> > + type_allocator alloc (mdebugread_objfile, psymtab_language); > >> > > >> > /* mips cc uses a rf of -1 for opaque struct definitions. > >> > Set TYPE_STUB for these types so that check_typedef will > >> > @@ -4412,7 +4423,8 @@ cross_ref (int fd, union aux_ext *ax, struct > type > >> **tpp, > >> > sh.index, > >> > NULL, > >> > fh->fBigendian, > >> > - debug_info->ss + fh->issBase + sh.iss= ); > >> > + debug_info->ss + fh->issBase + sh.iss= , > >> > + sh.st =3D=3D stTypedef); > >> > add_pending (fh, esh, *tpp); > >> > break; > >> > > >> > @@ -4438,7 +4450,8 @@ cross_ref (int fd, union aux_ext *ax, struct > type > >> **tpp, > >> > sh.index, > >> > NULL, > >> > fh->fBigendian, > >> > - debug_info->ss + fh->issBase + sh.iss); > >> > + debug_info->ss + fh->issBase + sh.iss, > >> > + true); > >> > } > >> > else > >> > { > >> > @@ -4542,6 +4555,7 @@ add_line (struct linetable *lt, int lineno, > >> CORE_ADDR adr, int last) > >> > return lineno; > >> > > >> > lt->item[lt->nitems].line =3D lineno; > >> > + lt->item[lt->nitems].is_stmt =3D 1; > >> > lt->item[lt->nitems++].set_unrelocated_pc (unrelocated_addr (adr = << > >> 2)); > >> > return lineno; > >> > } > >> > @@ -4634,9 +4648,10 @@ new_symtab (const char *name, int maxlines, > >> struct objfile *objfile) > >> > > >> > /* All symtabs must have at least two blocks. */ > >> > bv =3D new_bvect (2); > >> > - bv->set_block (GLOBAL_BLOCK, new_block (objfile, > NON_FUNCTION_BLOCK, > >> lang)); > >> > - bv->set_block (STATIC_BLOCK, new_block (objfile, > NON_FUNCTION_BLOCK, > >> lang)); > >> > + bv->set_block (GLOBAL_BLOCK, new_block (objfile, > NON_FUNCTION_BLOCK, > >> lang, true)); > >> > + bv->set_block (STATIC_BLOCK, new_block (objfile, > NON_FUNCTION_BLOCK, > >> lang, false)); > >> > bv->static_block ()->set_superblock (bv->global_block ()); > >> > + bv->global_block ()->set_compunit_symtab(cust); > >> > cust->set_blockvector (bv); > >> > > >> > cust->set_debugformat ("ECOFF"); > >> > @@ -4723,9 +4738,10 @@ new_bvect (int nblocks) > >> > > >> > static struct block * > >> > new_block (struct objfile *objfile, enum block_type type, > >> > - enum language language) > >> > + enum language language, bool isGlobal) > >> > { > >> > - struct block *retval =3D new (&objfile->objfile_obstack) block; > >> > + struct block *retval =3D isGlobal ? new (&objfile->objfile_obstac= k) > >> global_block > >> > + : new (&objfile->objfile_obstack) block; > >> > >> By the time you've s/isGlobal/is_global/ I think you should format thi= s > >> as: > >> > >> struct block *retval =3D (is_global > >> ? new (&objfile->objfile_obstack) global_blo= ck > >> : new (&objfile->objfile_obstack) block); > >> > >> > >> Thanks, > >> Andrew > >> > >> > >> > > >> > if (type =3D=3D FUNCTION_BLOCK) > >> > retval->set_multidict (mdict_create_linear_expandable > (language)); > >> > @@ -4754,8 +4770,7 @@ new_type (char *name) > >> > { > >> > struct type *t; > >> > > >> > - t =3D type_allocator (mdebugread_objfile, > >> > - get_current_subfile ()->language).new_type (); > >> > + t =3D type_allocator (mdebugread_objfile, psymtab_language).new_t= ype > (); > >> > t->set_name (name); > >> > INIT_CPLUS_SPECIFIC (t); > >> > return t; > >> > -- > >> > 2.41.0 > >> > >> > > --000000000000254fda060d4f09e2 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable
Wanted to say, still working on this, wou= ld like to get some tests in. Occupied a bit because of the time of year. H= oping to get something together in January.

Also,= really=C2=A0really appreciate that you maintainers are willing to=C2=A0kee= p this code around at all,=C2=A0and have up to this point. I know it's = very niche.
There are N64 (and PS2) reverse engineering projects, that e= ither use this part of GDB fairly directly, by gdb stubs in the emulator,or referenced its code when trying to parse symbols either mistakenly shi= pped with old games, or produced from old compilers when trying to make mat= ching binaries.
I'm not particularly involved in the scene, beyond o= bserving, but I gather some other people have patched things in this code p= rivately, or are using old GDB versions that worked.
Started work= ing on this project because I was doing some N64 reversing. Seemed like a w= aste to not just go the extra mile to get the fixes shared, if possible.

For an example of its use, the Ares N64 emulator jus= t got gdb stub support this November, and it's possible to use an ELF f= ile from a build of the Super Mario 64 decompilation project to debug the o= riginal ROM, essentially perfectly, with views of threads and everything el= se,=C2=A0after patching GDB. When I started on this, I'd just finished = writing a very basic stub for the Mupen64plus emulator (turned out to be re= dundant, I'd somehow missed some developer forks that had one).


On Mon, Dec 18, 2023 at 10:50=E2=80=AFAM Andrew Burgess <aburgess@redhat.com> wrote:
Zeck S <zeck654321@gmail.com> w= rites:

> I definitely would like to add some tests, or tweak some existing test= s!
> Just was not sure how normal that was, or if it would almost be viewed=
> "cheating" or something.

I don't think it would be considered cheating.=C2=A0 I'd just make = sure to
place a comment at the head of the file explaining that the test is
deliberately simple in order to compile with XXX compiler and that
changes should be made with care.=C2=A0 This will pretty much guarantee
nobody else will dare change the test's source code :)

As Tom said, the mdebug stuff is not used much, so if you posted an
updated patch without any tests, I'd be happy to approve it.=C2=A0 It&#= 39;s your
choice.

Thanks,
Andrew


> I really wanted to get the test suite running, which is why I made the=
> qemu-user board.
> And it has been very useful, but legitimately some of the tests just c= an't
> pass (or sometimes even build) with this nasty old compiler and symbol=
> format combination.
> If I can make a few new tests I can work around all of this with relat= ive
> ease.
>
> I'll address everything else as well!
>
> I'm tempted to make some kind of container for making running test= s with
> this setup easier.
> It's all a bit ridiculous, wrapping an old compiler and a modern l= inker in
> a bash script, then the compiler runs in an emulator, the built execut= able
> runs in another emulator...
> If I do that I'll make it public somewhere.
>
> Thanks for the feedback. I know this is a weird old format, but it is<= br> > actually becoming slightly more relevant again with the N64 game
> decompilation projects.
>
> On Mon, Dec 11, 2023 at 8:03=E2=80=AFAM Andrew Burgess <aburgess@redhat.com> w= rote:
>
>> Zeck S <zeck654321@gmail.com> writes:
>>
>> > I'm not quite sure what to do.
>> > I have done some testing using the qemu-mips board I made. >> > There are a lot of limitations I'm running into.
>> > I can just run as much of the testsuite as I can and see what= happens.
>> >
>> >
>> > Here are the main problems:
>> > 1. IDO only supports ansi c. This is a problem for some heade= rs in glibc
>> which use c99.
>> > 2. Test code sometimes has GCC features in them like asm() >> > 3. Test expectations sometimes assume features from nicer for= mats like
>> DWARF.
>> >
>> > I knew I probably couldn't get standard library stuff wor= king.
>> > Even if I hack past the C99 issues (variadic macros),
>> > there's also problems
>> > with compiler specific files like stddef.h.
>> >
>> > I started testing by just running gdb.base/info-types-c.exp a= lone,
>> > since I knew I fixed bugs in that area, and the test didn'= ;t use stdio.h.
>> > Found that the .c file used asm() which IDO doesn't suppo= rt.
>> > Commented that code out, ran the test. Failed.
>> > Fixed a segfault that had been introduced since I started thi= s project.
>> > Still failed.
>> > Fixed some typedef handling to get closer to the expected out= put.
>> > However, the expected output includes line numbers for types.=
>> > mdebug format doesn't include line information for types.=
>> > There are also differences in output stemming from
>> > mdebug assuming the debugger knows size information etc about= "basic"
>> > types, unlike dwarf, which includes information about them. >> >
>> > So at this point, I realize, I've modified the .c file fo= r the test
>> > which already seems not great, and I realize due to limitatio= ns of
>> mdebug,
>> > I can't ever make the test pass.
>> > Not sure how to demonstrate this as progress, but uh, it feel= s like
>> > progress?
>>
>> So, I know nothing about targets that make use of ECOFF / mdebug f= ormat
>> stuff, and I don't really have time to try setting up a test >> environment.
>>
>> I guess my expectation for at least some of these fixes would be t= hat
>> you could write a small sample program that does compile with your=
>> compiler, and then a .exp file which triggers the right GDB comman= ds to
>> expose the bug.
>>
>> From what you've said, it doesn't sound like the existing = test framework
>> actually has problems invoking the compiler and running the test, = it's
>> just that the test has too many DWARF/ISO-C assumptions within. >>
>> Personally, I'm a huge fan of writing tests, I think it is wor= th the
>> effort in the long run, so I think it would be worth your time to = try
>> and write some new tests for these issues.
>>
>> That said, given the nature of these fixes and the difficulties yo= u're
>> experiencing, if you don't feel that's possible, I'm n= ot going to oppose
>> merging these fixes without tests...
>>
>> ... but I do have a couple of style issues that would need fixing = first,
>> see inline below:
>>
>> >
>> >
>> > For the curious, this is the bash script I wrapped IDO with.<= br> >> > stderr is directed to dev null, because qemu-irix puts some n= on fatal
>> > errors there, which confuses the test runner.
>> > Put the script in a directory in PATH.
>> > Passing an executable path as CC_FOR_TARGET does not work. >> > It must be the name of a file in PATH.
>> >
>> > #!/bin/sh
>> >
>> > echo "$@" >> theargs
>> >
>> > for arg do
>> >=C2=A0 =C2=A0if [ "$arg" =3D "-static" ] >> >=C2=A0 =C2=A0then
>> >=C2=A0 =C2=A0 =C2=A0link=3D1
>> >=C2=A0 =C2=A0fi
>> > done
>> >
>> > if [ "$link" =3D 1 ]
>> > then
>> >=C2=A0 =C2=A0mips-unknown-linux-gnu-gcc "$@"
>> > else
>> > for arg do
>> >=C2=A0 =C2=A0shift
>> >=C2=A0 =C2=A0[ "$arg" =3D "-fdiagnostics-color= =3Dnever" ] && continue
>> >=C2=A0 =C2=A0[ "$arg" =3D "-g" ] &&= ; continue
>> >=C2=A0 =C2=A0set -- "$@" "$arg"
>> > done
>> >
>> > /path/to/qemu-irix -L /path/to/ido/ido7.1_compiler
>> /path/to/ido/ido7.1_compiler/usr/bin/cc -Xcpluscomm -nostdlib -nos= tdinc
>> -mips2 -g2 "$@" 2>/dev/null
>> > fi
>> >
>> >
>> > ---
>> >=C2=A0 gdb/mdebugread.c | 89 ++++++++++++++++++++++++++++-----= ---------------
>> >=C2=A0 1 file changed, 52 insertions(+), 37 deletions(-)
>> >
>> > diff --git a/gdb/mdebugread.c b/gdb/mdebugread.c
>> > index cd6638224e7..191932e91cd 100644
>> > --- a/gdb/mdebugread.c
>> > +++ b/gdb/mdebugread.c
>> > @@ -237,7 +237,7 @@ static struct type *new_type (char *); >> >=C2=A0 enum block_type { FUNCTION_BLOCK, NON_FUNCTION_BLOCK };=
>> >
>> >=C2=A0 static struct block *new_block (struct objfile *objfile= ,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0enum block_type, enum language= );
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0enum block_type, enum language= , bool
>> isGlobal);
>>
>> GDB style is for snake_case rather than camelCase, so:
>> s/isGlobal/is_global/ throughout.
>>
>> >
>> >=C2=A0 static struct compunit_symtab *new_symtab (const char *= , int, struct
>> objfile *);
>> >
>> > @@ -246,7 +246,7 @@ static struct linetable *new_linetable (i= nt);
>> >=C2=A0 static struct blockvector *new_bvect (int);
>> >
>> >=C2=A0 static struct type *parse_type (int, union aux_ext *, u= nsigned int, int
>> *,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int, const char *);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int, const char *, bool);
>> >
>> >=C2=A0 static struct symbol *mylookup_symbol (const char *, co= nst struct block
>> *,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 doma= in_enum, enum address_class);
>> > @@ -572,7 +572,7 @@ add_data_symbol (SYMR *sh, union aux_ext = *ax, int
>> bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 || sh->sc =3D=3D scNil || sh-&g= t;index =3D=3D indexNil)
>> >=C2=A0 =C2=A0 =C2=A0 s->set_type (builtin_type (objfile)-&g= t;nodebug_data_symbol);
>> >=C2=A0 =C2=A0 else
>> > -=C2=A0 =C2=A0 s->set_type (parse_type (cur_fd, ax, sh->= ;index, 0, bigend, name));
>> > +=C2=A0 =C2=A0 s->set_type (parse_type (cur_fd, ax, sh->= ;index, 0, bigend, name,
>> false));
>> >=C2=A0 =C2=A0 /* Value of a data symbol is its memory address.= =C2=A0 */
>> >=C2=A0 }
>> >
>> > @@ -705,7 +705,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax= , char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->set_value_longest (svalue);<= br> >> > -=C2=A0 =C2=A0 =C2=A0 s->set_type (parse_type (cur_fd, ax,= sh->index, 0, bigend, name));
>> > +=C2=A0 =C2=A0 =C2=A0 s->set_type (parse_type (cur_fd, ax,= sh->index, 0, bigend, name,
>> false));
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 add_symbol (s, top_stack->cur_s= t, top_stack->cur_block);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> >
>> > @@ -761,7 +761,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax= , char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0t =3D builtin_type (objfile)->bu= iltin_int;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 else
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0t =3D parse_type (cur_fd, ax, sh-= >index + 1, 0, bigend, name);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0t =3D parse_type (cur_fd, ax, sh-= >index + 1, 0, bigend, name,
>> false);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (strcmp (name, "mall= oc") =3D=3D 0
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0&& t-&= gt;code () =3D=3D TYPE_CODE_VOID)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> > @@ -805,7 +805,7 @@ parse_symbol (SYMR *sh, union aux_ext *ax= , char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0s->type ()->set_is_prototyped= (true);
>> >
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* Create and enter a new lexical = context.=C2=A0 */
>> > -=C2=A0 =C2=A0 =C2=A0 b =3D new_block (objfile, FUNCTION_BLOC= K, s->language ());
>> > +=C2=A0 =C2=A0 =C2=A0 b =3D new_block (objfile, FUNCTION_BLOC= K, s->language (), false);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 s->set_value_block (b);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 b->set_function (s);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 b->set_start (sh->value); >> > @@ -1135,7 +1135,7 @@ parse_symbol (SYMR *sh, union aux_ext *= ax, char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 top_stack->blocktype =3D stBloc= k;
>> > -=C2=A0 =C2=A0 =C2=A0 b =3D new_block (objfile, NON_FUNCTION_= BLOCK, psymtab_language);
>> > +=C2=A0 =C2=A0 =C2=A0 b =3D new_block (objfile, NON_FUNCTION_= BLOCK, psymtab_language,
>> false);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 b->set_start (sh->value + to= p_stack->procadr);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 b->set_superblock (top_stack-&g= t;cur_block);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 top_stack->cur_block =3D b;
>> > @@ -1247,7 +1247,7 @@ parse_symbol (SYMR *sh, union aux_ext *= ax, char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0f->set_loc_bitpos (sh->value)= ;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0bitsize =3D 0;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0f->set_type (parse_type (cur_fd,= ax, sh->index, &bitsize, bigend,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 name));
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 name, false));
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0f->set_bitsize (bitsize);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 }
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 break;
>> > @@ -1269,7 +1269,7 @@ parse_symbol (SYMR *sh, union aux_ext *= ax, char
>> *ext_sh, int bigend,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 pend =3D is_pending_symbol (cur_fd= r, ext_sh);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 if (pend =3D=3D NULL)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0t =3D parse_type (cur_fd, ax, sh-= >index, NULL, bigend, name);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0t =3D parse_type (cur_fd, ax, sh-= >index, NULL, bigend, name, true);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0add_pending (cur_fdr, ext_sh= , t);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 else
>> > @@ -1382,7 +1382,7 @@ basic_type (int bt, struct objfile *obj= file)
>> >=C2=A0 =C2=A0 if (map_bt[bt])
>> >=C2=A0 =C2=A0 =C2=A0 return map_bt[bt];
>> >
>> > -=C2=A0 type_allocator alloc (objfile, get_current_subfile ()= ->language);
>> > +=C2=A0 type_allocator alloc (objfile, psymtab_language);
>>
>> With fixes like this, it can (I think) be useful to mention the co= mmit
>> that is being fixed, in this case 76fc0f62138e.=C2=A0 Wouldn't= be much, I'd
>> just say:
>>
>>=C2=A0 =C2=A0Commit 76fc0f62138e added calls to 'get_current_su= bfile ()->language',
>>=C2=A0 =C2=A0but at the time this was known to be a bit of a guess,= and untested.
>>=C2=A0 =C2=A0We should actually have used 'psymtab_language'= ; because ....
>>
>> Then, if in the future, someone looks at your patch and wonders wh= y was
>> this change made?=C2=A0 There's a super quick way that can fin= d the history
>> of this code, and understand the original change, and your update.=
>>
>> >
>> >=C2=A0 =C2=A0 switch (bt)
>> >=C2=A0 =C2=A0 =C2=A0 {
>> > @@ -1514,7 +1514,7 @@ basic_type (int bt, struct objfile *obj= file)
>> >
>> >=C2=A0 static struct type *
>> >=C2=A0 parse_type (int fd, union aux_ext *ax, unsigned int aux= _index, int *bs,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int bigend, const char *sy= m_name)
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0int bigend, const char *sy= m_name, bool isStTypedef)
>>
>> Switch to snake_case for isStTypedef.
>>
>> >=C2=A0 {
>> >=C2=A0 =C2=A0 TIR t[1];
>> >=C2=A0 =C2=A0 struct type *tp =3D 0;
>> > @@ -1571,7 +1571,7 @@ parse_type (int fd, union aux_ext *ax, = unsigned
>> int aux_index, int *bs,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >=C2=A0 =C2=A0 =C2=A0 }
>> >
>> > -=C2=A0 type_allocator alloc (mdebugread_objfile, get_current= _subfile
>> ()->language);
>> > +=C2=A0 type_allocator alloc (mdebugread_objfile, psymtab_lan= guage);
>> >
>> >=C2=A0 =C2=A0 /* Move on to next aux.=C2=A0 */
>> >=C2=A0 =C2=A0 ax++;
>> > @@ -1628,7 +1628,7 @@ parse_type (int fd, union aux_ext *ax, = unsigned
>> int aux_index, int *bs,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 xref_fh =3D get_rfd (fd, rf);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 xref_fd =3D xref_fh - debug_info-&= gt;fdr;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 tp =3D parse_type (xref_fd, debug_= info->external_aux +
>> xref_fh->iauxBase,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0rn->index, NULL, xref_fh->fBigendian, sym_name);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0rn->index, NULL, xref_fh->fBigendian, sym_name, false);
>> >=C2=A0 =C2=A0 =C2=A0 }
>> >
>> >=C2=A0 =C2=A0 /* All these types really point to some (common)= MIPS type
>> > @@ -1785,6 +1785,13 @@ parse_type (int fd, union aux_ext *ax,= unsigned
>> int aux_index, int *bs,
>> >=C2=A0 =C2=A0 if (t->continued)
>> >=C2=A0 =C2=A0 =C2=A0 complaint (_("illegal TIR continued = for %s"), sym_name);
>> >
>> > +=C2=A0 if (isStTypedef)
>> > +=C2=A0 {
>> > +=C2=A0 =C2=A0 =C2=A0struct type *wrap =3D alloc.new_type (TY= PE_CODE_TYPEDEF, 0,
>> sym_name);
>> > +=C2=A0 =C2=A0 =C2=A0wrap->set_target_type(tp);
>> > +=C2=A0 =C2=A0 =C2=A0tp =3D wrap;
>> > +=C2=A0 }
>>
>> GNU style indents the '{' and '}', this should loo= k like:
>>
>>=C2=A0 =C2=A0if (isStTypedef)
>>=C2=A0 =C2=A0 =C2=A0{
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0struct type *wrap =3D alloc.new_type (TY= PE_CODE_TYPEDEF, 0, sym_name);
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0wrap->set_target_type(tp);
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0tp =3D wrap;
>>=C2=A0 =C2=A0 =C2=A0}
>>
>> > +
>> >=C2=A0 =C2=A0 return tp;
>> >=C2=A0 }
>> >
>> > @@ -1839,7 +1846,7 @@ upgrade_type (int fd, struct type **tpp= , int tq,
>> union aux_ext *ax, int bigend,
>> >
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 indx =3D parse_type (fh - debug_in= fo->fdr,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 debug_info->external_aux + fh->iauxBase,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 id, NULL, bigend, sym_name);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 id, NULL, bigend, sym_name, false);
>> >
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* The bounds type should be an in= teger type, but might be
>> anything
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 else due to corrupt aux entries.= =C2=A0 */
>> > @@ -2154,8 +2161,7 @@ parse_external (EXTR *es, int bigend, c= onst
>> section_offsets &section_offsets,
>> >=C2=A0 =C2=A0 =C2=A0with that and do not need to reorder our l= inetables.=C2=A0 */
>> >
>> >=C2=A0 static void
>> > -parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int max= lines,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 CORE_ADDR lowest_pdr_addr= )
>> > +parse_lines (FDR *fh, PDR *pr, struct linetable *lt, int max= lines)
>>
>> As above, this seems to be reverting parts of 1acc9dca423f78e4, it= would
>> be great to more of an explanation for why that commit was wrong.<= br> >>
>> Remember, someone thought that commit was correct once.=C2=A0 You&= #39;re looking
>> at their code and thinking they got it wrong.=C2=A0 What's to = stop someone
>> looking at your code and thinking _you_ got it wrong?
>>
>> Hopefully, what stops that is that you add more details in the com= mit
>> message, along with a link to the previous commit.
>>
>> >=C2=A0 {
>> >=C2=A0 =C2=A0 unsigned char *base;
>> >=C2=A0 =C2=A0 int j, k;
>> > @@ -2169,7 +2175,6 @@ parse_lines (FDR *fh, PDR *pr, struct l= inetable
>> *lt, int maxlines,
>> >=C2=A0 =C2=A0 for (j =3D 0; j < fh->cpd; j++, pr++)
>> >=C2=A0 =C2=A0 =C2=A0 {
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 CORE_ADDR l;
>> > -=C2=A0 =C2=A0 =C2=A0 CORE_ADDR adr;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 unsigned char *halt;
>> >
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 /* No code for this one.=C2=A0 */<= br> >> > @@ -2186,9 +2191,7 @@ parse_lines (FDR *fh, PDR *pr, struct l= inetable
>> *lt, int maxlines,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0halt =3D base + fh->cbLine;
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 base +=3D pr->cbLineOffset;
>> >
>> > -=C2=A0 =C2=A0 =C2=A0 adr =3D pr->adr - lowest_pdr_addr; >> > -
>> > -=C2=A0 =C2=A0 =C2=A0 l =3D adr >> 2;=C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 /* in words */
>> > +=C2=A0 =C2=A0 =C2=A0 l =3D pr->adr >> 2;=C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 /* in words */
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 for (lineno =3D pr->lnLow; base= < halt;)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0count =3D *base & 0x0f;<= br> >> > @@ -4053,6 +4056,14 @@ mdebug_expand_psymtab (legacy_psymtab = *pst,
>> struct objfile *objfile)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(*swap_pdr_in)= (cur_bfd, pdr_ptr, pdr_in);
>> >
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char *sym_ptr =3D (= char *)(debug_info->external_sym) +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(fh->isym= Base + pdr_in->isym) * external_sym_size;
>>
>> The layout seems wrong here.=C2=A0 There should for sure be a sing= le space
>> after '(char *)', and the '+' should be at the sta= rt of the new line
>> rather than the end of the previous line.
>>
>> Additionally, we usually wrap expressions like this within '( = ... )' to
>> force editors to align things better.=C2=A0 Turns out that this pa= ttern of
>> code is repeated lots in this file.=C2=A0 Search for: '* exter= nal_sym_size'
>> and you'll find lots of example for how to align this code.=C2= =A0 I'd go
>> with:
>>
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0char *sym_pt= r =3D ((char *) debug_info->external_sym
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 + (fh->isymBase + pdr_in-&= gt;isym)
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 * external_sym_size);
>>
>> maybe?
>>
>> > +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0SYMR sh;
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0(*swap_sym_in) (cur= _bfd, sym_ptr, &sh);
>> > +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0pdr_in->adr =3D = sh.value;
>> > +
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* Determine l= owest PDR address, the PDRs are not always
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sorted= .=C2=A0 */
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (pdr_in =3D= =3D pr_block.data ())
>> > @@ -4154,16 +4165,16 @@ mdebug_expand_psymtab (legacy_psymtab= *pst,
>> struct objfile *objfile)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= (*swap_pdr_in) (cur_bfd, pdr_ptr, pdr_in);
>> >
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0/* De= termine lowest PDR address, the PDRs are not always
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 sorted.=C2=A0 */
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (p= dr_in =3D=3D pr_block.data ())
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0lowest_pdr_addr =3D pdr_in->adr;
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0else = if (pdr_in->adr < lowest_pdr_addr)
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0lowest_pdr_addr =3D pdr_in->adr;
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 sym_ptr =3D (char *)(debug_info->external_sym) +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 (fh->isymBase + pdr_in->isym) * external_sym_size;
>>
>> Similar layout issues to above.
>>
>> > +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 SYMR sh;
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 (*swap_sym_in) (cur_bfd, sym_ptr, &sh);
>> > +
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 pdr_in->adr =3D sh.value;
>>
>> These lines are all indented using only spaces.=C2=A0 GDB (unfortu= nately)
>> uses a hybrid tab and space approach for indentation.=C2=A0 If you= 'git diff'
>> and look at this file, git _should_ highlight the whitespace issue= s for
>> you -- we have a .gitattributes file in the binutils-gdb root that= asks
>> for whitespace issues to be highlighted.
>>
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0parse_lines (fh, pr= _block.data (), lines, maxlines,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 lowest_pdr_addr);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0parse_lines (fh, pr= _block.data (), lines, maxlines);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0if (lines->= nitems < fh->cline)
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0lines = =3D shrink_linetable (lines);
>> >
>> > @@ -4291,7 +4302,7 @@ cross_ref (int fd, union aux_ext *ax, s= truct type
>> **tpp,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 rf =3D rn->rfd;
>> >=C2=A0 =C2=A0 =C2=A0 }
>> >
>> > -=C2=A0 type_allocator alloc (mdebugread_objfile, get_current= _subfile
>> ()->language);
>> > +=C2=A0 type_allocator alloc (mdebugread_objfile, psymtab_lan= guage);
>> >
>> >=C2=A0 =C2=A0 /* mips cc uses a rf of -1 for opaque struct def= initions.
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0Set TYPE_STUB for these types so th= at check_typedef will
>> > @@ -4412,7 +4423,8 @@ cross_ref (int fd, union aux_ext *ax, s= truct type
>> **tpp,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sh.index,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fh->fBigendian,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 debug_info->ss + fh->is= sBase + sh.iss);
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 debug_info->ss + fh->is= sBase + sh.iss,
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sh.st =3D=3D stTypedef);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0add_pending (f= h, esh, *tpp);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0break;
>> >
>> > @@ -4438,7 +4450,8 @@ cross_ref (int fd, union aux_ext *ax, s= truct type
>> **tpp,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 sh.index,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 NULL,
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0= =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 fh->fBigendian,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 debug_info->ss + fh->issBase + sh.iss= );
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 debug_info->ss + fh->issBase + sh.iss= ,
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 true);
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0}
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 else
>> >=C2=A0 =C2=A0 =C2=A0 =C2=A0{
>> > @@ -4542,6 +4555,7 @@ add_line (struct linetable *lt, int lin= eno,
>> CORE_ADDR adr, int last)
>> >=C2=A0 =C2=A0 =C2=A0 return lineno;
>> >
>> >=C2=A0 =C2=A0 lt->item[lt->nitems].line =3D lineno;
>> > +=C2=A0 lt->item[lt->nitems].is_stmt =3D 1;
>> >=C2=A0 =C2=A0 lt->item[lt->nitems++].set_unrelocated_pc = (unrelocated_addr (adr <<
>> 2));
>> >=C2=A0 =C2=A0 return lineno;
>> >=C2=A0 }
>> > @@ -4634,9 +4648,10 @@ new_symtab (const char *name, int maxl= ines,
>> struct objfile *objfile)
>> >
>> >=C2=A0 =C2=A0 /* All symtabs must have at least two blocks.=C2= =A0 */
>> >=C2=A0 =C2=A0 bv =3D new_bvect (2);
>> > -=C2=A0 bv->set_block (GLOBAL_BLOCK, new_block (objfile, N= ON_FUNCTION_BLOCK,
>> lang));
>> > -=C2=A0 bv->set_block (STATIC_BLOCK, new_block (objfile, N= ON_FUNCTION_BLOCK,
>> lang));
>> > +=C2=A0 bv->set_block (GLOBAL_BLOCK, new_block (objfile, N= ON_FUNCTION_BLOCK,
>> lang, true));
>> > +=C2=A0 bv->set_block (STATIC_BLOCK, new_block (objfile, N= ON_FUNCTION_BLOCK,
>> lang, false));
>> >=C2=A0 =C2=A0 bv->static_block ()->set_superblock (bv-&g= t;global_block ());
>> > +=C2=A0 bv->global_block ()->set_compunit_symtab(cust);=
>> >=C2=A0 =C2=A0 cust->set_blockvector (bv);
>> >
>> >=C2=A0 =C2=A0 cust->set_debugformat ("ECOFF"); >> > @@ -4723,9 +4738,10 @@ new_bvect (int nblocks)
>> >
>> >=C2=A0 static struct block *
>> >=C2=A0 new_block (struct objfile *objfile, enum block_type typ= e,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 enum language language)
>> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 enum language language, bool isG= lobal)
>> >=C2=A0 {
>> > -=C2=A0 struct block *retval =3D new (&objfile->objfil= e_obstack) block;
>> > +=C2=A0 struct block *retval =3D isGlobal ? new (&objfile= ->objfile_obstack)
>> global_block
>> > +=C2=A0 =C2=A0 : new (&objfile->objfile_obstack) block= ;
>>
>> By the time you've s/isGlobal/is_global/ I think you should fo= rmat this
>> as:
>>
>>=C2=A0 =C2=A0struct block *retval =3D (is_global
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0? new (&objfile->objfile_obstack) glo= bal_block
>>=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0: new (&objfile->objfile_obstack) blo= ck);
>>
>>
>> Thanks,
>> Andrew
>>
>>
>> >
>> >=C2=A0 =C2=A0 if (type =3D=3D FUNCTION_BLOCK)
>> >=C2=A0 =C2=A0 =C2=A0 retval->set_multidict (mdict_create_li= near_expandable (language));
>> > @@ -4754,8 +4770,7 @@ new_type (char *name)
>> >=C2=A0 {
>> >=C2=A0 =C2=A0 struct type *t;
>> >
>> > -=C2=A0 t =3D type_allocator (mdebugread_objfile,
>> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2= =A0 =C2=A0get_current_subfile ()->language).new_type ();
>> > +=C2=A0 t =3D type_allocator (mdebugread_objfile, psymtab_lan= guage).new_type ();
>> >=C2=A0 =C2=A0 t->set_name (name);
>> >=C2=A0 =C2=A0 INIT_CPLUS_SPECIFIC (t);
>> >=C2=A0 =C2=A0 return t;
>> > --
>> > 2.41.0
>>
>>

--000000000000254fda060d4f09e2--