[patch] Strange stepping behaviour with ppc32 with secure PLTs

[patch] Strange stepping behaviour with ppc32 with secure PLTs

[PAUL GILLIAM]

[-- Attachment #1: Type: text/plain, Size: 2268 bytes --]

These hacks, er, I mean 'patches', fix a problem gdb has 'next'ing over
a call to a library function on a ppc32 target program when secure PLT's
are being used.  They implement the two 'GDB only solutions' of the
previous discussion on the 'gdb@' mailing list.

Here is that previous discussion:
http://sourceware.org/ml/gdb/2006-05/msg00154.html
and Daniel Jacobowitz's response:
http://sourceware.org/ml/gdb/2006-05/msg00155.html

To answer Danial's questions:

>> I would rather have a GDB only solution.
>
> Why?

I am not familiar enough with BFD to implement <foo@stub> symbols is a reasonable time.

> What do you mean by "unknown section"?

This is what I mean:
(top-gdb) p *bfd_section
$2 = {name = 0x1049c528 "*UND*", id = 1, index = 0, ...



I have attached two patches: fix1.diff and fix2.diff.

Fix1.diff implements the first 'GDB only' solution and seems to be the best of the two.

Here are the testsuite results on a system with secure PLT's:

Before:
        # of expected passes            10749
        # of unexpected failures        316
        # of expected failures          41
        # of known failures             63
        # of unresolved testcases       12
        # of untested testcases         4
        # of unsupported tests          10
After:
        # of expected passes            10969
        # of unexpected failures        107
        # of expected failures          42
        # of known failures             64
        # of untested testcases         5
        # of unsupported tests          10
        
I also obtained before and after testsuite results on an x86 system and there was no change.

Fix2.diff implements the second GDB only approach and while the testsuite results show that
it is better than no fix, it is not quite as good as fix1.diff.

Here are the testsuite results for fix2.diff:

After:
        # of expected passes            10966
        # of unexpected failures        109
        # of expected failures          42
        # of known failures             64
        # of unresolved testcases       1
        # of untested testcases         5
        # of unsupported tests          10

Again, there was not change with the x86 testsuite results.

Are either of these OK to commit?

-=# Paul #=-

[-- Attachment #2: fix1.diff --]
[-- Type: text/x-patch, Size: 961 bytes --]

2006-05-12  Paul Gilliam  <pgilliam@us.ibm.com>

	* minsyms.c (lookup_minimal_symbol_by_pc_section): Don't ignore
	minimal symbols for solib trampolines just because they're in a
	different section than the PC.


Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.45
diff -a -u -r1.45 minsyms.c
--- minsyms.c   17 Dec 2005 22:34:01 -0000      1.45
+++ minsyms.c   12 May 2006 22:17:06 -0000
@@ -486,6 +486,8 @@
                          don't fill the bfd_section member, so don't
                          throw away symbols on those platforms.  */
                       && SYMBOL_BFD_SECTION (&msymbol[hi]) != NULL
+                      /* Don't ignore symbols for solib trampolines */
+                      && MSYMBOL_TYPE (&msymbol[hi]) != mst_solib_trampoline
                       && SYMBOL_BFD_SECTION (&msymbol[hi]) != section)
                  --hi;



[-- Attachment #3: fix2.diff --]
[-- Type: text/x-patch, Size: 1140 bytes --]

2006-05-12  Paul Gilliam  <pgilliam@us.ibm.com>

	* minsyms.c (prim_record_minimal_symbol_and_info): Set a new minimal
	symbol's section to '.text', and zero out it's bfd-section, if the
	symbol is for solib-trampoline code.


Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.45
diff -a -u -r1.45 minsyms.c
--- minsyms.c	17 Dec 2005 22:34:01 -0000	1.45
+++ minsyms.c	12 May 2006 16:48:54 -0000
@@ -619,9 +619,18 @@
   SYMBOL_SET_NAMES (msymbol, (char *)name, strlen (name), objfile);
 
   SYMBOL_VALUE_ADDRESS (msymbol) = address;
-  SYMBOL_SECTION (msymbol) = section;
-  SYMBOL_BFD_SECTION (msymbol) = bfd_section;
 
+  if (ms_type != mst_solib_trampoline)
+    {
+      SYMBOL_SECTION (msymbol) = section;
+      SYMBOL_BFD_SECTION (msymbol) = bfd_section;
+    }
+  else
+    {
+      SYMBOL_SECTION (msymbol) = SECT_OFF_TEXT (objfile);
+      SYMBOL_BFD_SECTION (msymbol) = 0;
+    }
+   
   MSYMBOL_TYPE (msymbol) = ms_type;
   /* FIXME:  This info, if it remains, needs its own field.  */
   MSYMBOL_INFO (msymbol) = info;	/* FIXME! */

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Daniel Jacobowitz]

On Fri, May 12, 2006 at 02:38:55PM -0700, PAUL GILLIAM wrote:
> >> I would rather have a GDB only solution.
> >
> > Why?
> 
> I am not familiar enough with BFD to implement <foo@stub> symbols is a reasonable time.

I would like to introduce to you a magical technique for fixing PowerPC
bugs in BFD.

Hey, Alan, Paul's pointed out that there are no synthetic symbols for
the stubs in -msecure-plt mode.  How hard would that be to fix?

> > What do you mean by "unknown section"?
> 
> This is what I mean:
> (top-gdb) p *bfd_section
> $2 = {name = 0x1049c528 "*UND*", id = 1, index = 0, ...

That's not an "unknown" section.  That's the undefined section!  It
means that the symbol has a value, but is marked as undefined.  This
is typical for PLT entry stubs, but it's an implementation choice;
I don't think the gABI has much to say about undefined symbols with
addresses.

The right thing to do then is probably to create the synthetic symbols
at exactly those same addresses.

> 2006-05-12  Paul Gilliam  <pgilliam@us.ibm.com>
> 
> 	* minsyms.c (lookup_minimal_symbol_by_pc_section): Don't ignore
> 	minimal symbols for solib trampolines just because they're in a
> 	different section than the PC.

This one I'd have to think about, but if synthetic symbols were not an
option, it might be reasonable.  The second patch is definitely
incorrect.  Generally any use of SECT_OFF_TEXT for something whose
section you don't know is incorrect.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[PAUL GILLIAM]

On Fri, 2006-05-12 at 18:50 -0400, Daniel Jacobowitz wrote:
> On Fri, May 12, 2006 at 02:38:55PM -0700, PAUL GILLIAM wrote:
. . .
> > > What do you mean by "unknown section"?

Let me retroactively rephrase my answer: "I *really* meant 'undefined'
section".  8-)

>                                    The second patch is definitely
> incorrect.  Generally any use of SECT_OFF_TEXT for something whose
> section you don't know is incorrect.
> 

Not only that, but it had worse testsuite results 8-)

-=# Paul #=-

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Alan Modra]

On Fri, May 12, 2006 at 06:50:44PM -0400, Daniel Jacobowitz wrote:
> Hey, Alan, Paul's pointed out that there are no synthetic symbols for
> the stubs in -msecure-plt mode.  How hard would that be to fix?

Not too hard, but messy.  The only real difficulty is finding the stubs.
We don't have any handy symbols or relocs to identify them, so that
means code reading.

We do have --emit-stub-syms to emit symbols on stubs at link time.  I
probably should have made the linker always emit the first stub sym so
we could easily find all the stubs.

> > > What do you mean by "unknown section"?
> > 
> > This is what I mean:
> > (top-gdb) p *bfd_section
> > $2 = {name = 0x1049c528 "*UND*", id = 1, index = 0, ...
> 
> That's not an "unknown" section.  That's the undefined section!  It
> means that the symbol has a value, but is marked as undefined.  This
> is typical for PLT entry stubs, but it's an implementation choice;
> I don't think the gABI has much to say about undefined symbols with
> addresses.

PLT symbols like this, ie. undefined but with nonzero value are a clue
to the dynamic linker that it needs to treat the sym specially, to make
function pointer comparisons work between an app and its shared libs.

> The right thing to do then is probably to create the synthetic symbols
> at exactly those same addresses.

No, if I understand correctly, Paul wants to use a stub symbol as a
means of letting gdb know that it is in a plt call trampoline.  Putting
the symbol in .plt won't do that for you.  Best just teach gdb what a
plt call stub looks like.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Daniel Jacobowitz]

On Sun, May 14, 2006 at 12:01:41AM +0930, Alan Modra wrote:
> Not too hard, but messy.  The only real difficulty is finding the stubs.
> We don't have any handy symbols or relocs to identify them, so that
> means code reading.
> 
> We do have --emit-stub-syms to emit symbols on stubs at link time.  I
> probably should have made the linker always emit the first stub sym so
> we could easily find all the stubs.

Hmm.  Do they still end up in a section named .plt?  That'd probably
make it easier for both gdb and binutils.

> > The right thing to do then is probably to create the synthetic symbols
> > at exactly those same addresses.
> 
> No, if I understand correctly, Paul wants to use a stub symbol as a
> means of letting gdb know that it is in a plt call trampoline.  Putting
> the symbol in .plt won't do that for you.  Best just teach gdb what a
> plt call stub looks like.

There's two things necessary to make stepping and shared libraries play
nice.

1.  Be able to unwind from the stub, so that we can see we've stepped
into something.  We need symbols or some other similar code reading in
order to make this work.

2.  Be able to identify this debug-info-less block of code as a stub,
so that we can skip it if we're going to step into a shared library
function that has debug info.  I pointed Paul at the right function for
handling this earlier, but it's somewhat orthogonal.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Alan Modra]

On Sat, May 13, 2006 at 10:58:29AM -0400, Daniel Jacobowitz wrote:
> On Sun, May 14, 2006 at 12:01:41AM +0930, Alan Modra wrote:
> > Not too hard, but messy.  The only real difficulty is finding the stubs.
> > We don't have any handy symbols or relocs to identify them, so that
> > means code reading.
> > 
> > We do have --emit-stub-syms to emit symbols on stubs at link time.  I
> > probably should have made the linker always emit the first stub sym so
> > we could easily find all the stubs.

In thinking about this some more, I remembered why I didn't implement
synthetic stub syms for powerpc.  There is a really difficult problem
with -fPIC code, which can have multiple GOTs.  The stubs for -fPIC
-shared or -fPIC -pie code calculate the plt entry from the got pointer
used by the calling function.  That means we might need multiple stubs
for any given plt entry.  eg. printf called from f1 and f2 where f1 uses
a different GOT to f2 will require two stubs and one .plt entry.  So we
can't just start from the first stub and match one for one with plt
entries.  Worse, code reading the stub doesn't give us a clue about the
got pointer value.  So BFD can't match stubs to plt entries without
finding where the stub is falled *from*, and then analysing the function
prologue.  This is way too hard.

> Hmm.  Do they still end up in a section named .plt?  That'd probably
> make it easier for both gdb and binutils.

No.  If people use the default linker scripts, stubs are at the end of
.text.  .plt is just an array of addresses.

> > > The right thing to do then is probably to create the synthetic symbols
> > > at exactly those same addresses.
> > 
> > No, if I understand correctly, Paul wants to use a stub symbol as a
> > means of letting gdb know that it is in a plt call trampoline.  Putting
> > the symbol in .plt won't do that for you.  Best just teach gdb what a
> > plt call stub looks like.
> 
> There's two things necessary to make stepping and shared libraries play
> nice.
> 
> 1.  Be able to unwind from the stub, so that we can see we've stepped
> into something.  We need symbols or some other similar code reading in
> order to make this work.
> 
> 2.  Be able to identify this debug-info-less block of code as a stub,
> so that we can skip it if we're going to step into a shared library
> function that has debug info.  I pointed Paul at the right function for
> handling this earlier, but it's somewhat orthogonal.

I think code reading is the way to go on both of these.  Presumably gdb
knows the register state for both of these problems, so finding the
caller (from lr) isn't hard, and you reach the callee just by
single-stepping a few insns.

-shared and -pie stubs look like:

  lwz 11,(plt_entry-got_pointer)(30)
  mtctr 11
  bctr
  nop

or

  addis 11,30,(plt_entry-got_pointer)@ha
  lwz 11,(plt_entry-got_pointer)@l(11)
  mtctr 11
  bctr

depending on the size of (plt_entry-got_pointer).

Non-shared, non-pie stubs look like:

  lis 11,plt_entry@ha
  lwz 11,plt_entry@l(11)
  mtctr
  bctr

These sequences are different to those emitted by gcc for indirect
function calls.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Daniel Jacobowitz]

On Mon, May 15, 2006 at 10:26:19AM +0930, Alan Modra wrote:
> In thinking about this some more, I remembered why I didn't implement
> synthetic stub syms for powerpc.  There is a really difficult problem
> with -fPIC code, which can have multiple GOTs.  The stubs for -fPIC
> -shared or -fPIC -pie code calculate the plt entry from the got pointer
> used by the calling function.  That means we might need multiple stubs
> for any given plt entry.  eg. printf called from f1 and f2 where f1 uses
> a different GOT to f2 will require two stubs and one .plt entry.  So we
> can't just start from the first stub and match one for one with plt
> entries.  Worse, code reading the stub doesn't give us a clue about the
> got pointer value.  So BFD can't match stubs to plt entries without
> finding where the stub is falled *from*, and then analysing the function
> prologue.  This is way too hard.

Can't we somehow record them in the object file, then?

If they aren't in their own section, we'd have to check for them
any time we were stopped without a symbol in .text.  And GDB doesn't
even know how to use the sizes of symbols, so it may sometimes decide
they're part of an earlier function.

> > There's two things necessary to make stepping and shared libraries play
> > nice.
> > 
> > 1.  Be able to unwind from the stub, so that we can see we've stepped
> > into something.  We need symbols or some other similar code reading in
> > order to make this work.
> > 
> > 2.  Be able to identify this debug-info-less block of code as a stub,
> > so that we can skip it if we're going to step into a shared library
> > function that has debug info.  I pointed Paul at the right function for
> > handling this earlier, but it's somewhat orthogonal.
> 
> I think code reading is the way to go on both of these.  Presumably gdb
> knows the register state for both of these problems, so finding the
> caller (from lr) isn't hard, and you reach the callee just by
> single-stepping a few insns.

I suppose it could be made to work.  It's going to be a bit nasty,
though.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Alan Modra]

On Mon, May 15, 2006 at 11:08:54AM -0400, Daniel Jacobowitz wrote:
[about powerpc stubs]
> Can't we somehow record them in the object file, then?

Would one symbol at the start of a block of stubs be sufficient?  If so,
please sugest a name to suit existing gdb code (if there is such).

On powerpc, the stubs are all in one block at the end of .text.  On
powerpc64 they are in multiple blocks throughout .text.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Daniel Jacobowitz]

On Tue, May 16, 2006 at 09:16:20AM +0930, Alan Modra wrote:
> On Mon, May 15, 2006 at 11:08:54AM -0400, Daniel Jacobowitz wrote:
> [about powerpc stubs]
> > Can't we somehow record them in the object file, then?
> 
> Would one symbol at the start of a block of stubs be sufficient?  If so,
> please sugest a name to suit existing gdb code (if there is such).
> 
> On powerpc, the stubs are all in one block at the end of .text.  On
> powerpc64 they are in multiple blocks throughout .text.

I'm not going to give you a quick answer to this one; I'd need to think
about it a little bit.

-- 
Daniel Jacobowitz
CodeSourcery

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Mark Kettenis]

> Date: Tue, 16 May 2006 09:16:20 +0930
> From: Alan Modra <amodra@bigpond.net.au>
> 
> On Mon, May 15, 2006 at 11:08:54AM -0400, Daniel Jacobowitz wrote:
> [about powerpc stubs]
> > Can't we somehow record them in the object file, then?
> 
> Would one symbol at the start of a block of stubs be sufficient?  If so,
> please sugest a name to suit existing gdb code (if there is such).
> 
> On powerpc, the stubs are all in one block at the end of .text.  On
> powerpc64 they are in multiple blocks throughout .text.

Did anyone look at how things are done for hppa?  There we have import
and export stubs that seem to be somewhat similar to the stubs used
here.

Also, what's necessary for producing binaries with these stubs.  Just
a new enough linker invoked with --secure-plt?  Or do I need a new
enough compiler too?

Mark

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Alan Modra]

On Tue, May 16, 2006 at 08:34:04AM +0200, Mark Kettenis wrote:
> Also, what's necessary for producing binaries with these stubs.  Just
> a new enough linker invoked with --secure-plt?  Or do I need a new
> enough compiler too?

You need a new linker and all relocatable object files involved in the
link must be compiled with -msecure-plt.  That includes headers and
static libraries.  So..  Complete new toolchain.  binutils+gcc+glibc.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[PAUL GILLIAM]

On Tue, 2006-05-16 at 16:48 +0930, Alan Modra wrote:
> On Tue, May 16, 2006 at 08:34:04AM +0200, Mark Kettenis wrote:
> > Also, what's necessary for producing binaries with these stubs.  Just
> > a new enough linker invoked with --secure-plt?  Or do I need a new
> > enough compiler too?
> 
> You need a new linker and all relocatable object files involved in the
> link must be compiled with -msecure-plt.  That includes headers and
> static libraries.  So..  Complete new toolchain.  binutils+gcc+glibc.
> 

Hold the presses!  Could the fact that the libc being used does not have
a '.symtab' section be the cause of the problem?  For some reason, the
library I was using has a '.dynsym' section, but no '.symtab' section.
I suspect that someone was trying to save some disc space and caused
several problems.

The routine 'bfd_get_synthetic_symtab' takes both static and dynamic
symbol tables as input.  So it's no wonder it's not doing the right
thing if one is missing.

-=# Paul #=-

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Alan Modra]

On Fri, May 19, 2006 at 09:24:14AM -0700, PAUL GILLIAM wrote:
> Hold the presses!  Could the fact that the libc being used does not have
> a '.symtab' section be the cause of the problem?  For some reason, the
> library I was using has a '.dynsym' section, but no '.symtab' section.
> I suspect that someone was trying to save some disc space and caused
> several problems.
> 
> The routine 'bfd_get_synthetic_symtab' takes both static and dynamic
> symbol tables as input.  So it's no wonder it's not doing the right
> thing if one is missing.

No.  ppc32 uses the generic _bfd_elf_get_synthetic_symtab, which only
uses the dyn syms.  The only target to use both sym tables is ppc64,
where synthetic syms are handy for disassembly of relocatable object
files (which don't have a dynamic sym table).

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[PAUL GILLIAM]

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]

It's been more than a month, so I thought I'd try again.

I also thought I'd try the new patch tracker:

:ADDPATCH PowerPC-32:

This patch fixes a problem gdb has 'next'ing over a call to a library
function on a ppc32 target program when secure PLT's are being used.  It
implements a 'GDB only solution' of the previous discussion on the
'gdb@' mailing list.

Here is that previous discussion:
http://sourceware.org/ml/gdb/2006-05/msg00154.html
and Daniel Jacobowitz's response:
http://sourceware.org/ml/gdb/2006-05/msg00155.html

Subsequent discussion on gdb-patches begins here:
http://sourceware.org/ml/gdb-patches/2006-05/msg00270.html
And peter out here:
http://sourceware.org/ml/gdb-patches/2006-05/msg00420.html

I have attached a update of the patch, now diff'ed against current
sources.

OK to commit?

-=# Paul #=-





[-- Attachment #2: keep-solib-tramp-sym.diff --]
[-- Type: text/x-patch, Size: 867 bytes --]

2006-06-23  Paul Gilliam  <pgilliam@us.ibm.com>

        * minsyms.c (lookup_minimal_symbol_by_pc_section): Don't ignore
        minimal symbols for solib trampolines just because they're in a
        different section than the PC.

Index: minsyms.c
===================================================================
RCS file: /cvs/src/src/gdb/minsyms.c,v
retrieving revision 1.45
diff -a -u -r1.45 minsyms.c
--- minsyms.c	17 Dec 2005 22:34:01 -0000	1.45
+++ minsyms.c	23 Jun 2006 20:59:34 -0000
@@ -486,6 +486,8 @@
 			  don't fill the bfd_section member, so don't
 			  throw away symbols on those platforms.  */
 		       && SYMBOL_BFD_SECTION (&msymbol[hi]) != NULL
+		       /* Don't ignoe symbols for solib tranpolines. */
+		       && MSYMBOL_TYPE (&msymbol[hi]) != mst_solib_trampoline
 		       && SYMBOL_BFD_SECTION (&msymbol[hi]) != section)
 		  --hi;
 

Re: [patch] Strange stepping behaviour with ppc32 with secure PLTs

[Daniel Jacobowitz]

On Fri, Jun 23, 2006 at 01:01:24PM -0700, PAUL GILLIAM wrote:
> It's been more than a month, so I thought I'd try again.

I remain convinced that this is not the correct solution.  The
discussion you're replying to talks about a better way to identify such
stubs using code reading.  If the symbols is "defined" in the undefined
section, we don't have reliable information about what it really means.

-- 
Daniel Jacobowitz
CodeSourcery