namaskaaram On Thu, Jun 11, 2026, 6:09 AM Michael Eager wrote: > Hi Gopi -- > > I (or more accurately, git) fixed several whitespace issues. > Please share. I did check all the whitespace issues flagged by git-diff. > Since this is a revised patch, has it been retested? > Yes, it has been retested. dhanyavaadaaha gopi > On 6/1/26 10:05 PM, Gopi Kumar Bulusu wrote: > > namaskaaram > > > > Attached is the updated patch. All the changes suggested on PATCH v1 > > have been made and tested. > > > > I did not remove the (sal.line != 0) check as it seemed trivial. > > However, I can make that a separate patch if necessary. > > > > Our sysadm In the process of setting up a git compatible email - the > > process may take more time; until then appreciate your patience with > > the attachments. > > > > Important Note: > > > > 1) gdb appears to have an elaborate scheme to handle C++ exceptions > > and "tunnel" them through exception unaware C code. This code is with > > readline. Anytime > > an error is discovered (even a syntax error) by a module, > > gdb_exception_error() thrown is tunneled through readline using the > > TRY_SJLJ/CATCH_SJLJ macros > > implemented using setjmp/longjmp. > > > > 2) There is an issue that I came across while testing; native gdb goes > > into an infinite loop when handling errors. I analyzed it and found it > > to be an obvious glibc > > sysdeps/unix/sysv/linux/microblaze/____longjmp_chk.S issue and > > reported it to AMD. It is possible I missed out a glibc patch; At this > > time - this is not a gdb issue. > > > > 3) Plan to follow through [2] and ensure a gcc/glibc defect report is > > created if needed. > > > > dhanyavaadaaha > > gopi > > > > On Tue, Jun 2, 2026 at 5:58 AM Michael Eager wrote: > >> > >> Please post an updated patch for review. > >> > >> On 5/31/26 11:38 PM, Gopi Kumar Bulusu wrote: > >>> On Fri, May 29, 2026 at 8:26 PM Tom Tromey wrote: > >>>> > >>>>>>>>> Gopi Kumar Bulusu writes: > >>>> > >>>>> Attached is a patch to add native linux support for gdb including > >>>>> cache target support.. > >>>> > >>>> Thanks for the patch. > >>>> > >>>> I read through it and have some nits. There's nothing very serious > >>>> except this: > >>>> > >>>>> The original sources for the patch come from Xilinx/AMD Yocto git > >>>>> repository (2025.2) > >>>> > >>>> Who wrote them and are they covered by the copyright assignment? > >>>> > >>>> This is the most important consideration, the patch can't land without > >>>> this being clear. > >>> > >>> This work is being performed under a contract with Xilinx/AMD as > >>> confirmed in a different response to > >>> Simon and therefore covered by the copyright assignment. > >>> > >>> I will make the changes as described below and retest. > >>> > >>> Approved for commit ? > >>> > >>> dhanyavaadaaha > >>> gopi > >>> > >>>> > >>>>> From a3328b4cdfc58783b5f290f22255b36e26a6fb14 Mon Sep 17 00:00:00 > 2001 > >>>>> From: Gopi Kumar Bulusu > >>>>> Date: Thu, 7 May 2026 17:47:40 +0530 > >>>>> Subject: [PATCH] gdb/MicroBlaze: Add support for native linux gdb > >>>> > >>>> It's best to just git send-email rather than using an attachment. > >>>> > >>>>> bfd/ChangeLog: > >>>> > >>>>> * elf32-microblaze.c (microblaze_elf_grok_prstatus): New > function. > >>>>> (microblaze_elf_grok_psinfo): Likewise. > >>>> > >>>>> gdb/ChangeLog: > >>>> > >>>>> * Makefile.in (HFILES_NO_SRCDIR): Add microblaze-linux-tdep.h > >>>>> ALLDEPFILES: Add microblaze-linux-nat.c > >>>> > >>>> gdb doesn't use ChangeLogs any more and this text should all be > removed. > >>>> > >>>>> +#define MICROBLAZE_TARGET_HAS_GETREGS 0 > >>>>> +#define MICROBLAZE_TARGET_HAS_SETREGS 0 > >>>> > >>>> This seems weird: > >>>> > >>>>> +/* Wrapper function around ptrace. */ > >>>>> + > >>>>> +static void > >>>>> +fetch_target_gp_regs (int tid, elf_gregset_t *gregs) > >>>>> +{ > >>>>> + elf_greg_t *gregp = *gregs; > >>>>> + > >>>>> +#if MICROBLAZE_TARGET_HAS_GETREGS > >>>>> + if (ptrace (PTRACE_GETREGS, tid, 0, (long) gregp) < 0) > >>>>> + { > >>>>> + perror_with_name (_("Couldn't get registers")); > >>>>> + return; > >>>>> + } > >>>>> +#error "this configuration did not work during testing" > >>>> > >>>> Normally we don't put new dead code into gdb. > >>>> > >>>> I think this and the defines could just be replaced with a comment > >>>> explaining that PTRACE_GETREGS doesn't work on this platform. > >>>> > >>>>> +++ b/gdb/microblaze-linux-tdep.h > >>>>> @@ -0,0 +1,24 @@ > >>>>> +/* Target-dependent code for GNU/Linux on MicroBlaze. > >>>>> + > >>>>> + Copyright (C) 2021-2026 Free Software Foundation, Inc. > >>>>> + > >>>>> + This file is part of GDB. > >>>>> + > >>>>> + This program is free software; you can redistribute it and/or > modify > >>>>> + it under the terms of the GNU General Public License as > published by > >>>>> + the Free Software Foundation; either version 3 of the License, or > >>>>> + (at your option) any later version. > >>>>> + > >>>>> + This program is distributed in the hope that it will be useful, > >>>>> + but WITHOUT ANY WARRANTY; without even the implied warranty of > >>>>> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > >>>>> + GNU General Public License for more details. > >>>>> + > >>>>> + You should have received a copy of the GNU General Public License > >>>>> + along with this program. If not, see < > http://www.gnu.org/licenses/>. */ > >>>>> +#ifndef MICROBLAZE_LINUX_TDEP_H > >>>>> +#define MICROBLAZE_LINUX_TDEP_H > >>>>> + /* Target descriptions. */ > >>>>> + extern struct target_desc *tdesc_microblaze_linux; > >>>> > >>>> Blank line between the #define and the comment. > >>>> > >>>> Also this define should have a slightly different name, see > >>>> check-include-guards.py. > >>>> > >>>>> non_stack_instruction_found = 0; > >>>>> + cache->register_offsets[rd] = -imm; > >>>>> continue; > >>>> > >>>> Indentation looks wrong, probably not using tabs. > >>>> > >>>>> @@ -399,8 +400,7 @@ microblaze_skip_prologue (struct gdbarch > *gdbarch, CORE_ADDR start_pc) > >>>>> { > >>>>> sal = find_sal_for_pc (func_start, 0); > >>>> > >>>>> - if (sal.end < func_end > >>>>> - && start_pc <= sal.end) > >>>>> + if (sal.line != 0 && sal.end <= func_end && start_pc <= > sal.end) > >>>>> start_pc = sal.end; > >>>> > >>>> No objection from me but this seems somewhat unrelated. > >>>> > >>>> > >>>>> /* Call for side effects. */ > >>>>> - get_frame_func (next_frame); > >>>>> + cache->pc = get_frame_func (next_frame); > >>>> > >>>> Please remove that comment as it is now wrong. > >>>> > >>>>> + if (regnum == MICROBLAZE_SP_REGNUM) > >>>>> + regnum = 1; > >>>> > >>>> Wrong indentation. > >>>> > >>>>> + if (regnum == -1) { > >>>>> + int i; > >>>>> + > >>>>> + for (i = 0; i < MICROBLAZE_REDR_REGNUM; i++) { > >>>>> + regcache->raw_supply (i, regs + i * MICROBLAZE_REGISTER_SIZE); > >>>>> + } > >>>> > >>>> Wrong brace placement in this hunk. > >>>> > >>>>> + cb (".reg", tdep->sizeof_gregset, tdep->sizeof_gregset, > tdep->gregset, NULL, > >>>>> + cb_data); > >>>> > >>>> Normally in new code we're using 'nullptr' now. Best would be to go > >>>> through the whole patch and check for this. You don't have to fix > >>>> pre-existing code that is using NULL though. > >>>> > >>>>> gdbarch *gdbarch > >>>>> = gdbarch_alloc (&info, gdbarch_tdep_up (new > microblaze_gdbarch_tdep)); > >>>> > >>>>> + microblaze_gdbarch_tdep *tdep > >>>>> + = gdbarch_tdep (gdbarch); > >>>>> + > >>>>> + tdep->gregset = NULL; > >>>>> + tdep->sizeof_gregset = 0; > >>>>> + tdep->fpregset = NULL; > >>>>> + tdep->sizeof_fpregset = 0; > >>>> > >>>> Here it would be better to just add inline initializers to the new > >>>> fields in microblaze_gdbarch_tdep. Then they'll automatically be > >>>> initialized by the 'new'. > >>>> > >>>> Tom > >>> > >> > > -- > Michael Eager > >