[committed][gdb/tdep] Fix inferior plt calls in PIE for i386

[Tom de Vries <tdevries@suse.de>]

On 11/19/21 3:50 PM, Tom de Vries wrote:
> On 11/1/21 1:00 PM, Tom de Vries via Gdb-patches wrote:
>> Consider test-case test.c:
>> ...
>> int main (void) {
>>   void *p = malloc (10);
>>   return 0;
>> }
>> ...
>>
>> When compiled to a non-PIE exec:
>> ...
>> $ gcc -m32 test.c
>> ...
>> the call sequence looks like:
>> ...
>>  8048447:       83 ec 0c                sub    $0xc,%esp
>>  804844a:       6a 0a                   push   $0xa
>>  804844c:       e8 bf fe ff ff          call   8048310 <malloc@plt>
>> ...
>> which calls to:
>> ...
>> 08048310 <malloc@plt>:
>>  8048310:       ff 25 0c a0 04 08       jmp    *0x804a00c
>>  8048316:       68 00 00 00 00          push   $0x0
>>  804831b:       e9 e0 ff ff ff          jmp    8048300 <.plt>
>> ...
>> where the first insn at 0x8048310 initially jumps to the following address
>> 0x8048316, read from the .got.plt @ 0x804a00c:
>> ...
>>  804a000 0c9f0408 00000000 00000000 16830408  ................
>>  804a010 26830408                             &...
>> ...
>>
>> Likewise, when compiled as a PIE:
>> ...
>> $ gcc -m32 -fPIE -pie test.c
>> ...
>> we have this call sequence (with %ebx setup to point to the .got.plt):
>> ...
>> 0000055d <main>:
>>  579:   83 ec 0c                sub    $0xc,%esp
>>  57c:   6a 0a                   push   $0xa
>>  57e:   89 c3                   mov    %eax,%ebx
>>  580:   e8 6b fe ff ff          call   3f0 <malloc@plt>
>> ...
>> which calls to:
>> ...
>> 000003f0 <malloc@plt>:
>>  3f0:   ff a3 0c 00 00 00       jmp    *0xc(%ebx)
>>  3f6:   68 00 00 00 00          push   $0x0
>>  3fb:   e9 e0 ff ff ff          jmp    3e0 <.plt>
>> ...
>> where the insn at 0x3f0 initially jumps to following address 0x3f6, read from
>> the .got.plt at offset 0xc:
>> ...
>>  2000 f41e0000 00000000 00000000 f6030000  ................
>>  2010 06040000                             ....
>> ...
>>
>> When instead doing an inferior call to malloc (with nosharedlib to force
>> malloc to resolve to malloc@plt rather than the functions in ld.so or libc.so)
>> with the non-PIE exec, we have the expected:
>> ...
>> $ gdb -q -batch a.out -ex start -ex nosharedlib -ex "p /x (void *)malloc (10)"
>> Temporary breakpoint 1 at 0x8048444
>>
>> Temporary breakpoint 1, 0x08048444 in main ()
>> $1 = 0x804b160
>> ...
>>
>> But with the PIE exec, we run into:
>> ...
>> $ gdb -q -batch a.out -ex start -ex nosharedlib -ex "p /x (void *)malloc (10)"
>> Temporary breakpoint 1 at 0x56c
>>
>> Temporary breakpoint 1, 0x5655556c in main ()
>>
>> Program received signal SIGSEGV, Segmentation fault.
>> 0x565553f0 in malloc@plt ()
>> ...
>>
>> The segfault happens because:
>> - the inferior call mechanism doesn't setup %ebx
>> - %ebx instead is 0
>> - the jump to "*0xc(%ebx)" reads from memory at 0xc
>>
>> Fix this by setting up %ebx properly in i386_thiscall_push_dummy_call.
>>
>> Fixes this failure with target board unix/-m32/-pie/-fPIE reported in
>> PR28467:
>> ...
>> FAIL: gdb.base/nodebug.exp: p/c (int) array_index("abcdef",2)
>> ...
>>
>> Tested on x86_64-linux, with target board unix/-m32 and unix/-m32/-fPIE/-pie.
>>
> 
> Ping.  Any comment?
> 

Committed.

Thanks,
- Tom

>> Bug: https://sourceware.org/bugzilla/show_bug.cgi?id=28467
>> ---
>>  gdb/i386-tdep.c | 43 +++++++++++++++++++++++++++++++++++++++++++
>>  gdb/maint.c     |  2 +-
>>  gdb/maint.h     |  3 +++
>>  3 files changed, 47 insertions(+), 1 deletion(-)
>>
>> diff --git a/gdb/i386-tdep.c b/gdb/i386-tdep.c
>> index f65a07492d2..7df9420eba2 100644
>> --- a/gdb/i386-tdep.c
>> +++ b/gdb/i386-tdep.c
>> @@ -67,6 +67,8 @@
>>  #include <algorithm>
>>  #include <unordered_set>
>>  #include "producer.h"
>> +#include "infcall.h"
>> +#include "maint.h"
>>  
>>  /* Register names.  */
>>  
>> @@ -2776,6 +2778,47 @@ i386_thiscall_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
>>      regcache->cooked_write (I386_ECX_REGNUM,
>>  			    value_contents_all (args[0]).data ());
>>  
>> +  /* If the PLT is position-independent, the SYSTEM V ABI requires %ebx to be
>> +     set to the address of the GOT when doing a call to a PLT address.
>> +     Note that we do not try to determine whether the PLT is
>> +     position-independent, we just set the register regardless.  */
>> +  CORE_ADDR func_addr = find_function_addr (function, nullptr, nullptr);
>> +  if (in_plt_section (func_addr))
>> +    {
>> +      struct objfile *objf = nullptr;
>> +      asection *asect = nullptr;
>> +      obj_section *osect = nullptr;
>> +
>> +      /* Get object file containing func_addr.  */
>> +      obj_section *func_section = find_pc_section (func_addr);
>> +      if (func_section != nullptr)
>> +	objf = func_section->objfile;
>> +
>> +      if (objf != nullptr)
>> +	{
>> +	  /* Get corresponding .got.plt or .got section.  */
>> +	  asect = bfd_get_section_by_name (objf->obfd, ".got.plt");
>> +	  if (asect == nullptr)
>> +	    asect = bfd_get_section_by_name (objf->obfd, ".got");
>> +	}
>> +
>> +      if (asect != nullptr)
>> +	/* Translate asection to obj_section.  */
>> +	osect = maint_obj_section_from_bfd_section (objf->obfd, asect, objf);
>> +
>> +      if (osect != nullptr)
>> +	{
>> +	  /* Store the section address in %ebx.  */
>> +	  store_unsigned_integer (buf, 4, byte_order, osect->addr ());
>> +	  regcache->cooked_write (I386_EBX_REGNUM, buf);
>> +	}
>> +      else
>> +	{
>> +	  /* If we would only do this for a position-independent PLT, it would
>> +	     make sense to issue a warning here.  */
>> +	}
>> +    }
>> +
>>    /* MarkK wrote: This "+ 8" is all over the place:
>>       (i386_frame_this_id, i386_sigtramp_frame_this_id,
>>       i386_dummy_id).  It's there, since all frame unwinders for
>> diff --git a/gdb/maint.c b/gdb/maint.c
>> index bcc71aab579..75d3e49991b 100644
>> --- a/gdb/maint.c
>> +++ b/gdb/maint.c
>> @@ -329,7 +329,7 @@ print_objfile_section_info (bfd *abfd, struct obj_section *asect,
>>     from ABFD.  It might be that no such wrapper exists (for example debug
>>     sections don't have such wrappers) in which case nullptr is returned.  */
>>  
>> -static obj_section *
>> +obj_section *
>>  maint_obj_section_from_bfd_section (bfd *abfd,
>>  				    asection *asection,
>>  				    objfile *ofile)
>> diff --git a/gdb/maint.h b/gdb/maint.h
>> index d3c0122a321..81b3beb703d 100644
>> --- a/gdb/maint.h
>> +++ b/gdb/maint.h
>> @@ -63,4 +63,7 @@ class scoped_command_stats
>>    int m_start_nr_blocks;
>>  };
>>  
>> +extern obj_section *maint_obj_section_from_bfd_section (bfd *abfd,
>> +							asection *asection,
>> +							objfile *ofile);
>>  #endif /* MAINT_H */
>>
>> base-commit: 94c9216c03ab1af16b1bdd11a10a66c13e6458d8
>>