> Date: Wed, 15 Nov 2017 19:45:26 -0500 > From: Todd Mortimer <[email protected]> > > Hi tech@, > > This is an updated diff that shuffles the allocation order for registers > on i386/amd64. The last one exposed a subtle bug with the way chromium > and libexecinfo interact when creating backtraces. With this diff, make > release seems fine, I didn't see any differences in regress, and > chromium builds. We are still removing about 4500 unique gadgets from > the kernel this way (about 6% of the total unique gadgets). > > The problem with the previous diff was that it put RBP before RBX, which > broke some assumptions between chromium and libexecinfo. When building > chromium, the C++ code ended up allocating RBP as a general purpose > register, which broke backtraces from the devel/libexecinfo port, which > is expecting RBP to hold a frame pointer. In the original order, RBP > points to a stack address that holds 0, so it 'worked' by terminating > the backtrace immediately, but this seems like a bit of a fluke. I am > not sure if / how we want to deal with this. Basically, libexecinfo > is using the __builtin_frame_address and __builtin_return_address > compiler builtins, which expect RBP to hold a frame pointer. Any port > that calls backtrace() from code that is using RBP as a general purpose > register will be incorrect / unstable. This seems like an uncommon > problem.
Hi Todd, Let me start by saying thatnothing what I say below should keep you from going ahead with this slightly castrated version. I would like to ask you to make some noise about this on the appropriate clang mailing lists if you didn't do so already. Be sure to mention OpenBSD ;). In case you haven't found it yet; gnu/llvm/doc/LangRef.rst doucments the llvm.returnaddress and llvm.frameaddress that back these builtins. >From their description it is obvious that what libexecinfo is doing is simply broken on many architectures. It'll work for those architectures that have a genuine frame pointer. It will probably work correctly on i386 as long as none of the code being backtraced was compiled with -fomit-frame-pointer. But on amd64 that's the default and compiling everything with -fno-omit-frame-pointer (like we do for the kernel) really isn't an option. Instead the amd64 ABI requires DWARF2 EH info to be present. The glibc backtrace implementation uses this information to provide backtraces instead of just hoping that %rbp hasn't been clobbered. Fixing this shouldn't be too difficult. The clang libunwind has _Unwind_Backtrace() which does all the heavy lifting. That of course brings us back to the discussion whether we should provide libunwind as a separate library (it's currently integrated in libc++abi). But maybe robert@ should chime in and tell us what the various options here are. Personally I'd like to see devel/libexecinfo to be removed from ports if possible. > Index: lib/Target/X86/X86RegisterInfo.td > =================================================================== > RCS file: /cvs/src/gnu/llvm/lib/Target/X86/X86RegisterInfo.td,v > retrieving revision 1.1.1.4 > diff -u -p -u -p -r1.1.1.4 X86RegisterInfo.td > --- lib/Target/X86/X86RegisterInfo.td 4 Oct 2017 20:28:02 -0000 1.1.1.4 > +++ lib/Target/X86/X86RegisterInfo.td 11 Nov 2017 14:04:23 -0000 > @@ -339,8 +339,8 @@ def GR16 : RegisterClass<"X86", [i16], 1 > R8W, R9W, R10W, R11W, R14W, R15W, R12W, R13W)>; > > def GR32 : RegisterClass<"X86", [i32], 32, > - (add EAX, ECX, EDX, ESI, EDI, EBX, EBP, ESP, > - R8D, R9D, R10D, R11D, R14D, R15D, R12D, R13D)>; > + (add EAX, ECX, EDX, ESI, EDI, > + R8D, R9D, R10D, R11D, R14D, R15D, R12D, R13D, > EBX, EBP, ESP)>; > > // GR64 - 64-bit GPRs. This oddly includes RIP, which isn't accurate, since > // RIP isn't really a register and it can't be used anywhere except in an > @@ -349,7 +349,7 @@ def GR32 : RegisterClass<"X86", [i32], 3 > // tests because of the inclusion of RIP in this register class. > def GR64 : RegisterClass<"X86", [i64], 64, > (add RAX, RCX, RDX, RSI, RDI, R8, R9, R10, R11, > - RBX, R14, R15, R12, R13, RBP, RSP, RIP)>; > + R14, R15, R12, R13, RBX, RBP, RSP, RIP)>; > > // Segment registers for use by MOV instructions (and others) that have a > // segment register as one operand. Always contain a 16-bit segment > > ----- End forwarded message ----- > >
