From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 35362 invoked by alias); 30 Jun 2015 15:56:50 -0000 Mailing-List: contact systemtap-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Post: List-Help: , Sender: systemtap-owner@sourceware.org Received: (qmail 35315 invoked by uid 48); 30 Jun 2015 15:56:47 -0000 From: "dsmith at redhat dot com" To: systemtap@sourceware.org Subject: [Bug tapsets/18597] long_arg() doesn't correctly handle negative values in 32-on-64 environment Date: Tue, 30 Jun 2015 15:56:00 -0000 X-Bugzilla-Reason: AssignedTo X-Bugzilla-Type: changed X-Bugzilla-Watch-Reason: None X-Bugzilla-Product: systemtap X-Bugzilla-Component: tapsets X-Bugzilla-Version: unspecified X-Bugzilla-Keywords: X-Bugzilla-Severity: normal X-Bugzilla-Who: dsmith at redhat dot com X-Bugzilla-Status: NEW X-Bugzilla-Resolution: X-Bugzilla-Priority: P2 X-Bugzilla-Assigned-To: systemtap at sourceware dot org X-Bugzilla-Target-Milestone: --- X-Bugzilla-Flags: X-Bugzilla-Changed-Fields: Message-ID: In-Reply-To: References: Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Bugzilla-URL: http://sourceware.org/bugzilla/ Auto-Submitted: auto-generated MIME-Version: 1.0 X-SW-Source: 2015-q2/txt/msg00222.txt.bz2 https://sourceware.org/bugzilla/show_bug.cgi?id=18597 --- Comment #5 from David Smith --- (In reply to David Smith from comment #4) > If this code wasn't needed before, why is it needed now? I've figured this out. This code is needed because stp_arg() (which long_arg() calls) had a bug. The bug was that in some circumstances, longlong_arg() would return a 64-bit value (from 1 register) when called on a 32-bit process. longlong_arg()/stp_arg() was designed to do the following: If we're in a 32-bit compat process, then you can't put a 64-bit value in 1 register. Instead, you have to break the 64-bit value up and put it in 2 registers. This is why longlong_arg() had code in it like this: - if (probing_32bit_app()) { - lowbits = _stp_arg(argnum, 0, 1) - highbits = _stp_arg(argnum+1, 0, 1) - return ((highbits << 32) | lowbits) However, this code doesn't really work correctly on a 64-bit OS. Let me give an example. Here's pread() and the x86_64 compat function for pread(). The 'pos' argument (or the combination of 'poslo' and 'poshi' arguments) is a 64-bit value. ==== ssize_t sys_pread64(unsigned int fd, char __user *buf, size_t count, loff_t pos) { //... } asmlinkage long sys32_pread(unsigned int fd, char __user *ubuf, u32 count, u32 poslo, u32 poshi) { return sys_pread64(fd, ubuf, count, ((loff_t)AA(poshi) << 32) | AA(poslo)); } ==== If we're probing a compat function, we'd never call longlong_arg(), we'd do the same thing that the wrapper function is doing - call ulong_arg() on arguments 4 and 5 and slam the values together. Since probing_32bit_app() never returned true, the supposed 32-bit code in longlong_arg() was never getting called. Your patch has to add lots of "compat" probes because as a side effect of fixing the negative value problem it "broke" longlong_arg() returning a 64-bit value from 1 register even when in a 32-bit process (which shouldn't have worked in the first place). I've been struggling with the right way to fix this. I went down the same path you tried, and ended up about in the same place. Then I decided to try a different path. What if we decided that if you call longlong_arg() on a 64-bit OS on a 32-bit process you really *want* a 64-bit value from 1 register? In some ways this makes sense and it matches our old behavior. My theory here is that you know what you are doing and if you call longlong_arg() on a 32-bit process you must be in the true 64-bit function at this point. I'm testing this now and it works well on everything but ppc64. I'm unsure of what is going on there. -- You are receiving this mail because: You are the assignee for the bug.