[1/2] Use -fno-asynchronous-unwind-tables if C program is compiled without debug info on x86

  .eh_frame is added in default on x86 and x86_64 so that DWARF unwinder
is always used, but prologue unwinders won't be used at all.  IOW,
prologue unwinders are not tested by regression tests.  If the test is
intended to compiled *without* debug info, we shouldn't generate any
debug info (.eh_frame for example) at all.

This patch is to disable async-unwind-tables generation if the C program
is intended to compile without debug info on x86-like target.

gdb/testsuite:

2016-04-18  Pierre Langlois  <pierre.langlois@arm.com>
	    Yao Qi  <yao.qi@linaro.org>

	PR gdb/19947
	* lib/gdb.exp (gdb_compile): Append
	"additional_flags=-fno-asynchronous-unwind-tables" if C program
	is compiled without debug info on x86.
---
 gdb/testsuite/lib/gdb.exp | 9 +++++++++
 1 file changed, 9 insertions(+)
  

I think the subject would be clearer if it mentioned this is about gdb tests.  E.g.:

Use -fno-asynchronous-unwind-tables on x86 for gdb C tests without debug info

On 04/19/2016 08:50 AM, Yao Qi wrote:
> .eh_frame is added in default on x86 and x86_64 so that DWARF unwinder
> is always used, but prologue unwinders won't be used at all.  IOW,
> prologue unwinders are not tested by regression tests.  If the test is
> intended to compiled *without* debug info, we shouldn't generate any
> debug info (.eh_frame for example) at all.
> 
> This patch is to disable async-unwind-tables generation if the C program
> is intended to compile without debug info on x86-like target.

No sure about this.  This is an ABI change on x86_64 -- the x86_64 ABI
requires eh_frame.

Should we instead add a new "nounwind" option, and a few
prologue-unwinder-specific tests?

Thanks,
Pedro Alves
  

Pedro Alves <palves@redhat.com> writes:

> No sure about this.  This is an ABI change on x86_64 -- the x86_64 ABI
> requires eh_frame.

This is because people want to unwind frames without using frame
pointer register  (registers are very limited on x86), so different
tools, like glibc's backtrace and libunwind, can use eh_frame to unwind
correctly.  Then, ABI requires eh_frame.

GDB is different, because it can still rely on the prologue unwinder if
no debug info.  On the other hand, GDB prologue unwinder should be
tested in case that no debug info is produced by compiler.  We just use
-fno-asynchronous-unwind-tables to produce binaries to exercise GDB, so
I don't worry about ABI incompliance.

>
> Should we instead add a new "nounwind" option, and a few
> prologue-unwinder-specific tests?

We don't know how prologue unwinder is used in each test case with
option "nodebug", so  I am afraid we need to add "nounwind" to every
test case where option "nodebug" is used.
  

On 04/20/2016 09:37 AM, Yao Qi wrote:
> Pedro Alves <palves@redhat.com> writes:
> 
>> No sure about this.  This is an ABI change on x86_64 -- the x86_64 ABI
>> requires eh_frame.
> 
> This is because people want to unwind frames without using frame
> pointer register  (registers are very limited on x86), so different
> tools, like glibc's backtrace and libunwind, can use eh_frame to unwind
> correctly.  Then, ABI requires eh_frame.

I believe unwind frames are also necessary for pthread cancellation
and thread_cleanup_push implemented in terms of gcc's
__attribute__ __cleanup__, in C programs.

> 
> GDB is different, because it can still rely on the prologue unwinder if
> no debug info. 

Which is heuristic and can (and does) thus fail sometimes.

> On the other hand, GDB prologue unwinder should be
> tested in case that no debug info is produced by compiler.

Though that's not testing what the compiler will normally output
with just "-On" and no -g.  That's what in part what I was trying
to convey -- the ABI change means we're not testing what
people normally use.

> We just use
> -fno-asynchronous-unwind-tables to produce binaries to exercise GDB, so
> I don't worry about ABI incompliance.

Right.

> 
>>
>> Should we instead add a new "nounwind" option, and a few
>> prologue-unwinder-specific tests?
> 
> We don't know how prologue unwinder is used in each test case with
> option "nodebug", so  I am afraid we need to add "nounwind" to every
> test case where option "nodebug" is used.

I think most, if not all, "nodebug" tests will be about no symbol info,
and aren't really about the unwinder at all.

I was suggesting new tests specific for the prologue unwinder,
not changing the existing tests.  E.g., just something simple that runs
to a function and does "backtrace" making sure the expected frames are
visible.

If we want to cover more cases, maybe do a test that single-steps through
some body of code, and issues backtrace at every single-step,
prologues/epilogues included, making sure "main" is always visible
in the backtrace.

We could actually have that test try "backtrace" with dwarf unwinding,
and then the same but with the prologue unwinder.

Whether forcing use of the prologue unwinder through compilation flags
is the best way, not sure.  A "maint set force-prologue-unwinder on" command
would make it easier to make sure we test what we intend to test on all
architectures, independent of program language, and independent
of architecture.  Plus, a test unit testing the prologue unwinder could
then conveniently make use of debug info.

(If such a test steps through dynamic symbol resolution / plts, then it's
likely that it can only really work with dwarf unwinding, though I think
it'd be a nice test to have and would likely expose bugs in glibc's
cfi markers in assembly code.)

Thanks,
Pedro Alves
  

Pedro Alves <palves@redhat.com> writes:

> I think most, if not all, "nodebug" tests will be about no symbol info,
> and aren't really about the unwinder at all.
>

It is quite common that gdb tests find some bugs of some thing which
isn't these tests about.  The test was written for feature A, but it may
find bugs in feature B or C.

> I was suggesting new tests specific for the prologue unwinder,
> not changing the existing tests.  E.g., just something simple that runs
> to a function and does "backtrace" making sure the expected frames are
> visible.
>
> If we want to cover more cases, maybe do a test that single-steps through
> some body of code, and issues backtrace at every single-step,
> prologues/epilogues included, making sure "main" is always visible
> in the backtrace.
>
> We could actually have that test try "backtrace" with dwarf unwinding,
> and then the same but with the prologue unwinder.
>

That must be useful.  If debug infor is produced, it can be a good test
to compiler.

> Whether forcing use of the prologue unwinder through compilation flags
> is the best way, not sure.  A "maint set force-prologue-unwinder on" command
> would make it easier to make sure we test what we intend to test on all
> architectures, independent of program language, and independent
> of architecture.  Plus, a test unit testing the prologue unwinder could
> then conveniently make use of debug info.

We had this idea too when we wrote aarch64 tracepoint support.  We
wanted to know which unwinder is chosen by GDB for a given frame.

>
> (If such a test steps through dynamic symbol resolution / plts, then it's
> likely that it can only really work with dwarf unwinding, though I think
> it'd be a nice test to have and would likely expose bugs in glibc's
> cfi markers in assembly code.)

I want to focus on handling unavailable memory in frame unwinder, so I
cannot do all of them above.  What I can do are:

 1. Name some unwinders, such as dwarf unwinders, prologue unwinders,
 sigtramp unwinders, stub unwinders, etc.  Other unwinders are nameless.
 2. Add a new maint command "maint set/show preferred-unwinder".  This
 command tell GDB to prefer a unwinder during frame unwinding.  If the
 unwinder can't be applied to the frame (sniffer fails), it is not chosen.
 3. Prefer different unwinders (dwarf vs. prologue) in the different
 runs of gdb.trace tests, like

 foreach_with_prefix unwinder { "dwarf" "prologue" } {
   gdb_test "maint set preferred-unwinder $unwinder"
   gdb_test "tfind start"
   .....
   gdb_test "tfind stop"
 }

 so that the PR 19947 can be reproduced.  I'll stop here.

 What I don't plan to do in the short term are:

 - Write a case, let GDB single-step through its body, including
   prologue, epilogue and its callees, and check "bt" can show the
   frames correctly with different unwinders.
 - Single-step through dynamic symbol resolution / plt, prefer "dwarf"
   unwinder, and check "bt" can show the frames correctly.

Is it OK to you?
  

On 04/22/2016 03:23 PM, Yao Qi wrote:

> I want to focus on handling unavailable memory in frame unwinder, so I
> cannot do all of them above.  What I can do are:
> 
>  1. Name some unwinders, such as dwarf unwinders, prologue unwinders,
>  sigtramp unwinders, stub unwinders, etc.  Other unwinders are nameless.
>  2. Add a new maint command "maint set/show preferred-unwinder".  This
>  command tell GDB to prefer a unwinder during frame unwinding.  If the
>  unwinder can't be applied to the frame (sniffer fails), it is not chosen.

Instead of a name, maybe a frame-unwinder-class enum, based on 
https://sourceware.org/bugzilla/show_bug.cgi?id=19288#c13

~~~~~~~~~~~~~~~
- The "Accurate unwinders"

  These would be the DWARF / x64 SEH based ones.

- JIT unwinders 

  Python/Guile unwind API unwinders, and also the C JIT-reader
  API unwinder, in jit.c.

- Fallback prologue unwinders
~~~~~~~~~~~~~~~

Then it could be "required" instead of "preferred", which would
catch problems with the unwinder's sniffer as well.

>  3. Prefer different unwinders (dwarf vs. prologue) in the different
>  runs of gdb.trace tests, like
> 
>  foreach_with_prefix unwinder { "dwarf" "prologue" } {
>    gdb_test "maint set preferred-unwinder $unwinder"
>    gdb_test "tfind start"
>    .....
>    gdb_test "tfind stop"
>  }
> 
>  so that the PR 19947 can be reproduced.  I'll stop here.
> 
>  What I don't plan to do in the short term are:
> 
>  - Write a case, let GDB single-step through its body, including
>    prologue, epilogue and its callees, and check "bt" can show the
>    frames correctly with different unwinders.
>  - Single-step through dynamic symbol resolution / plt, prefer "dwarf"
>    unwinder, and check "bt" can show the frames correctly.
> 
> Is it OK to you?
> 

Yes, that sounds good to me.

Thanks,
Pedro Alves
  

Pedro Alves <palves@redhat.com> writes:

> Instead of a name, maybe a frame-unwinder-class enum, based on 
> https://sourceware.org/bugzilla/show_bug.cgi?id=19288#c13
>
> ~~~~~~~~~~~~~~~
> - The "Accurate unwinders"
>
>   These would be the DWARF / x64 SEH based ones.
>
> - JIT unwinders 
>
>   Python/Guile unwind API unwinders, and also the C JIT-reader
>   API unwinder, in jit.c.
>
> - Fallback prologue unwinders
> ~~~~~~~~~~~~~~~
>
> Then it could be "required" instead of "preferred", which would
> catch problems with the unwinder's sniffer as well.

I am fine on this, but I don't think this is helpful to PR 19288.

What we need in PR 19288 is a dynamic list of unwinders (based on
priority), so that user can change the orders of unwinders in the list
by changing their priorities.  IIUC, in PR 19288, each unwinder works
well if the desired unwinder in the chain is selected.  The problem is
the desired unwinder can't be selected in the chain.

If we want to handle these two problems together, we need command line
interface to set and change the priorities of unwinders or classes of
unwinders.  This would be useful to PR 19288.  We can set priority to -1
or 0 to disable the unwinder completely.

However, my concern on this feature is that we may expose too much to
the user, and usually, it is tricky and sensitive on changing the order of
unwinders.