public inbox for gdb-patches@sourceware.org
 help / color / mirror / Atom feed
From: "Ulrich Weigand" <uweigand@de.ibm.com>
To: prudo@linux.vnet.ibm.com (Philipp Rudo)
Cc: gdb-patches@sourceware.org, arnez@linux.vnet.ibm.com (Andreas Arnez)
Subject: Re: [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism
Date: Tue, 05 Dec 2017 17:44:00 -0000	[thread overview]
Message-ID: <20171205174421.2D8A3D80327@oc3748833570.ibm.com> (raw)
In-Reply-To: <20171205180640.6677e975@ThinkPad> from "Philipp Rudo" at Dec 05, 2017 06:06:40 PM

Philipp Rudo wrote:

> > All of that *except* the sigtramp unwinder is OS-independent.  In fact,
> > if move the prolog-based sniffer to common code, you'll see that you no
> > longer need to export various internal routines from s390-tdep.c to
> > s390-linux-tdep.c.
> > 
> > This may require swapping the order of the stub and the sigtramp unwinder,
> > but that should be harmless.  In the end you should have this sequence:
> > 
> > - first, in common code, announce all the DWARF-based unwinders
> > - then, in Linux ABI code, announce the sigtramp unwinder
> > - finally, back in common code, announce all the fallback unwinders
> 
> That's not true.  At least for the kernel the unwinders (except the DWARF-based)
> failed and I had to write my own one.  Especially the fact that the kernel has 
> multiple stack locations and no distinct 'End of Stack' caused problems.
> 
> However if I go with your approach and add my unwinder in the kernel ABI code
> as default, i.e. that it always catches the call, it should work.  But that
> won't be nice code.  That's why I moved them to the Linux ABI.

That's surprising, I would have thought the prolog parser generic enough
to handle kernel code as well (and it really has to be anyway, since you
install the s390_skip_prologue callback in generic code ...).

I agree that there may be some special cases.  About the issues you mention:

- By "multiple stack locations" I assume you refer to the interrupt stack
  vs. the regular per-task kernel stacks?  I agree that we need kernel-
  specific code to switch between the two.  But that should simply be
  the equivalent to the user-space signal stack handling, so you'll install
  a sniffer in the kernel-specific code that detects the frame where you
  need to *switch* stacks.  All the normal frames should be handled fine
  by the standard prolog-parser code.

- End-of-stack detection is always a bit hit-and-miss with the prolog
  parser, and uses heuristics anyway.  If you need to tweak those a
  bit so they work well for kernel code, I'd hope (without having seen
  the code) that it should be possible to simply update the generic
  code to handle both.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU/Linux compilers and toolchain
  Ulrich.Weigand@de.ibm.com

  reply	other threads:[~2017-12-05 17:44 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-12-05 12:29 [PATCH v2 00/11] Split up s390-linux-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 10/11] s390: Add comments to uncommented functions in s390-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 09/11] s390: Clean up s390-linux-tdep.c Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 08/11] s390: Split up s390-linux-tdep.c into two files Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch at init Philipp Rudo
2017-12-05 16:16   ` Yao Qi
2017-12-06  9:56     ` Philipp Rudo
2017-12-06 10:28       ` [PATCH v2 01/11] s390: Remove duplicate checks for cached gdbarch@init Ulrich Weigand
2017-12-07  9:18         ` Philipp Rudo
2017-12-07  9:59           ` Yao Qi
2017-12-05 12:29 ` [PATCH v2 06/11] s390: if -> gdb_assert for tdesc_has_registers check Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 05/11] s390: Move tdesc validation to separate function Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 02/11] s390: Allocate gdbarch & tdep at start of gdbarch init Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 07/11] s390: Hook s390 into OSABI mechanism Philipp Rudo
2017-12-05 13:21   ` Ulrich Weigand
2017-12-05 17:06     ` Philipp Rudo
2017-12-05 17:44       ` Ulrich Weigand [this message]
2017-12-06 11:39         ` Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 04/11] s390: gdbarch_tdep add field tdesc Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 03/11] s390: gdbarch_tdep.have_* int -> bool Philipp Rudo
2017-12-05 12:29 ` [PATCH v2 11/11] s390: Add comments to uncommented functions in s390-linux-tdep.c Philipp Rudo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171205174421.2D8A3D80327@oc3748833570.ibm.com \
    --to=uweigand@de.ibm.com \
    --cc=arnez@linux.vnet.ibm.com \
    --cc=gdb-patches@sourceware.org \
    --cc=prudo@linux.vnet.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).