This is the mail archive of the
mailing list for the GCC project.
Re: [PATCH] 65479 - sanitizer stack trace missing frames past #0 on powerpc64
- From: Jeff Law <law at redhat dot com>
- To: Martin Sebor <msebor at redhat dot com>, Jakub Jelinek <jakub at redhat dot com>, Yury Gribov <y dot gribov at samsung dot com>
- Cc: Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Wed, 20 May 2015 10:08:42 -0600
- Subject: Re: [PATCH] 65479 - sanitizer stack trace missing frames past #0 on powerpc64
- Authentication-results: sourceware.org; auth=none
- References: <55345AD3 dot 3000403 at redhat dot com> <5535441A dot 4030200 at redhat dot com> <55357E7C dot 9090000 at redhat dot com>
On 04/20/2015 04:32 PM, Martin Sebor wrote:
I also wonder if other targets need -fasynchronous-unwind-tables and
whether or not we should just add it unconditionally.
I initially only tested powerpc64* and x86_64. I had tried s370
but asan doesn't appear to be built there (is it not supported?)
I've now also tried aarch64 (partly because the patch also fixes
a latent bug there). Of these targets, only powerpc64* needs
the option, and only until the fast unwinding that Jakub
mentioned is implemented. I plan to work on it but I wanted to
get this simpler fix in first if only so there is a working
baseline to start from.
Sorry for the deep context switch....
asan is only supported on a few platforms and I'm pretty sure s390 isn't
one of them.
Now that I know a bit more from Jakub & Yuri's comments, I don't think
we should be turning that flag on for all the targets in the testsuite.
I was primarily trying to avoid someone else having to go through the
same analysis and reach the same conclusion for another port. But I'm
less concerned about that now.
I totally understand the desire to have a good baseline. Jakub seems to
prefer not to make this change since it's a short-lived workaround,
which I understand as well.
My inclination is to go ahead with flags changes in the testsuite.
Cleaner results are, in and of themselves, a good thing. Pulling those
lines out once the port is using the fast unwind stuff is easy enough to do.
This part should definitely hit the LLVM side first, then we can pull it
into GCC. So that process should be started.
Is libsanitizer maintained in LLVM? If so, we want to minimize
divergence, so it may be better to get this approved in LLVM then pick
it up via a merge.
I can certainly see about submitting the sanitizer bits of
the patch to LLVM. It will probably take some time and I
was hoping for cleaner powerpc test results in 5.0 (or 5.1
as it sounds like the release will be called). I don't yet
have a sense of whether it's preferable to do one or the
other or whether it makes sense to do both (i.e., commit
the fix now and then merge).
Given this hits 3 distinct pieces of code, do any of them make sense in
isolation or do they have to land together as a unit?
65479 (this bug) depends on 65749 (sanitizer stack trace
pc off by 1). The asan tests cannot very well be made to
pass without fixing the latter bug.
Let me know how you'd like to proceed with the patch.
That's part of what I was trying to figure out myself :-)
So I'll ask the question(s) in a slightly different way and see if that
guides us one direction or another.
Do the libbacktrace changes make sense independently? ie, are they the
right thing to do, even if they don't fix a visible bug? ISTM the
answer to both questions is yes. In which case, that part ought to go
forward now rather than waiting.
The testsuite changes have two components. One is the new flag the
other is some slight tweaks to the expected output. I'd hazard a guess
that the expected output changes ought to go forward independently too.
Again under the same "it's the right thing to do, even if it doesn't
fix a visible bug".
The testsuite flags change isn't as clear cut. I'd think they'd need to
visibly improve the test results before they could go in. So they may
need to wait (I'm assuming nothing actually shows visible improvement
without the libsanitizer fixes).