From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.220.28]) by sourceware.org (Postfix) with ESMTPS id 626343858D28 for ; Tue, 7 Dec 2021 07:08:10 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 626343858D28 Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 3A053212C3 for ; Tue, 7 Dec 2021 07:08:09 +0000 (UTC) Received: from imap2.suse-dmz.suse.de (imap2.suse-dmz.suse.de [192.168.254.74]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-521) server-digest SHA512) (No client certificate requested) by imap2.suse-dmz.suse.de (Postfix) with ESMTPS id 1E36213487 for ; Tue, 7 Dec 2021 07:08:09 +0000 (UTC) Received: from dovecot-director2.suse.de ([192.168.254.65]) by imap2.suse-dmz.suse.de with ESMTPSA id bJYcBVkIr2FtEwAAMHmgww (envelope-from ) for ; Tue, 07 Dec 2021 07:08:09 +0000 Subject: [committed][gdb/tdep] Fix inferior plt calls in PIE for i386 From: Tom de Vries To: gdb-patches@sourceware.org References: <20211101120000.7784-1-tdevries@suse.de> Message-ID: <541c434a-d038-75e5-cac3-d991830a7851@suse.de> Date: Tue, 7 Dec 2021 08:08:08 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_NUMSUBJECT, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 07 Dec 2021 07:08:12 -0000 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 >> ... >> which calls to: >> ... >> 08048310 : >> 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
: >> 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 >> ... >> which calls to: >> ... >> 000003f0 : >> 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 >> #include >> #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 >>