Project

General

Profile

Actions

Bug #12903

open

x86 libc string functions should set a frame pointer

Added by Jason King over 3 years ago. Updated over 2 years ago.

Status:
New
Priority:
Normal
Assignee:
Category:
lib - userland libraries
Start date:
Due date:
% Done:

0%

Estimated time:
Difficulty:
Medium
Tags:
Gerrit CR:
External Bug:

Description

Currently, if one has code that crashes in one of the libc string functions (e.g. strlen), the resulting stack trace from pstack(1) or in mdb(1) always misses a frame. That is, for the following (contrived) code:

static void
foo(void)
{
    strlen(NULL);
}

static void
bar(void)
{
    foo();
}

int
main(void)
{
    bar();
    return 0;
}

The resulting core file will show main and foo, but not bar in the stack trace. This is because the string functions in libc are written in assembly, but do not manipulate the frame pointer %ebp/%rbp on function entry/exit (unlike the C functions). The fix is to utilize the frame pointer (push %[er]bp; mov %[er]sp, %[er]bp at the start of a function, and pop %ebp/leave at the end of a function) as the compiled C code does.

Note that the kernel uses C versions of these functions, so is not effected by this.

Actions #1

Updated by Electric Monk over 3 years ago

  • Gerrit CR set to 758
Actions #2

Updated by Jason King over 3 years ago

  • Gerrit CR deleted (758)

For testing, several approaches were used:

First a full pkgsrc build was run on a platform booted with the libc changes. No unexpected errors were found (the build failures matched those from a build without the changes).

I then tried to build a PI that doesn't loopback mount libc_hwcap1.so.1 (this is the only variant that contains alternate implementations of the functions -- memcpy(), memmove(), memset() from the base libc.so), unfortunately there seemed to be some issue with that (and possibly worthy of a bug) that prevented the system from booting properly. So instead, I copied memset.s and memcpy.s from usr/src/lib/libc/i386/gen to usr/src/lib/libc/i386_hwcap1/gen and ran dis on the resulting library to make sure the non-hwcap1 versions were now in place. Another full pkgsrc build was run on this platform with identical results.

Actions #3

Updated by Joshua M. Clulow over 3 years ago

  • Gerrit CR set to 758
Actions #4

Updated by Jason King about 3 years ago

As an orthogonal way to verify the changes (though more indirectly) since many of them may return in multiple spots, grep -B 3 ret and grep -A 3 ENTRY were run on all the modified 64-bit .s files to verify %rbp is being pushed upon function entry and that a leave instruction precedes every ret instruction.

Unfortunately, there's no similarly simple way to verify the 32-bit code.

Actions #5

Updated by Jason King over 2 years ago

Additionally, to verify the libc_hwcap3 overlay (not sure what the correct term for it is) had no impact, this change was built on an AMD machine, the resulting bits were booted, and then built again. No errors were encountered during either build, and a wsdiff showed no differences between the two builds.

Actions

Also available in: Atom PDF