From owner-freebsd-hackers@FreeBSD.ORG Tue Oct 12 18:31:41 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0AC5A106566B; Tue, 12 Oct 2010 18:31:41 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-gw0-f54.google.com (mail-gw0-f54.google.com [74.125.83.54]) by mx1.freebsd.org (Postfix) with ESMTP id A149E8FC14; Tue, 12 Oct 2010 18:31:40 +0000 (UTC) Received: by gwb15 with SMTP id 15so1839528gwb.13 for ; Tue, 12 Oct 2010 11:31:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:in-reply-to :references:date:message-id:subject:from:to:cc:content-type; bh=SMg9sBY6YvQXGPMiQkxRRqLbaurKKPg+quDSUQWG0ks=; b=iZiSZlCc2lbxqhrc0N0pHkFeKZvr/8M7wJjFgbFvRTqBbkHGaOIZC3YcQD+zmgTOk6 QCZDnFyI67rVL9VAzUwKJbhfYtxk+1vE7TmsOKkZNdNnOPXaRH8FQMrVJnSC4bjez8dT r7emDQCgZ3X4u7NJhe7rsGLd4R1hrUbLh8svc= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; b=RmAXh8n+blXarJ5y4LZ/2QyiZzGJjP2TwY8Vae2X4HJEgo8xiUKqolDmIQS6uNcF+J 8/ucup22O/1a9zvyRqJ3yP2zp8wWR0oheevxvYEOkUZjJyehMpNU9AnNwmn6ZgJfHVUZ 5PVSE0CtUdrS8hKT2xNZV9GLCUP1jVotmYCfs= MIME-Version: 1.0 Received: by 10.42.148.9 with SMTP id p9mr500566icv.440.1286908298004; Tue, 12 Oct 2010 11:31:38 -0700 (PDT) Received: by 10.231.184.3 with HTTP; Tue, 12 Oct 2010 11:31:36 -0700 (PDT) In-Reply-To: <201010120830.19872.jhb@freebsd.org> References: <201010120830.19872.jhb@freebsd.org> Date: Tue, 12 Oct 2010 11:31:36 -0700 Message-ID: From: Garrett Cooper To: John Baldwin Content-Type: multipart/mixed; boundary=90e6ba212165f607a504926faffc Cc: freebsd-hackers@freebsd.org, Jilles Tjoelker Subject: Re: [PATCH] Fix /bin/sh compilation with CFLAGS += -DDEBUG > 1 X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 Oct 2010 18:31:41 -0000 --90e6ba212165f607a504926faffc Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable On Tue, Oct 12, 2010 at 5:30 AM, John Baldwin wrote: > On Tuesday, October 12, 2010 6:47:49 am Garrett Cooper wrote: >> Hi, >> =A0 =A0 It looks like the format strings are broken on 64-bit archs in >> /bin/sh's TRACE functionality (can be enabled by uncommenting -DDEBUG >> > 1 in bin/sh/Makefile). The attached patch fixes this functionality >> again so one can trace sh's calls with TRACE, which may or may be >> helpful to those debugging /bin/sh. >> =A0 =A0 Tested build and execution on amd64; tested build on i386. > > I don't think the Makefile bits are needed, you can just use > 'make DEBUG_FLAGS=3D"-g -DDEBUG=3D2"' instead. Ok... I was just trying to make it tunable, but sure :)! > Also, if you plan on using -g you should generally set DEBUG_FLAGS anyway= so > binaries are not stripped. Ok, something new to note... > The use of things like PRIoMAX is not done in FreeBSD as it is ugly. =A0Y= ou can > use things like '%t' to print ptrdiff_t types instead. =A0So for example,= for > the first hunk, I would change the type of 'startloc' to ptrdiff_t and us= e > this: > > =A0 =A0 =A0 =A0TRACE(("evalbackq: size=3D%td: \"%.*s\"\n", > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(dest - stackblock()) - startloc, > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0(int)((dest - stackblock()) - startloc), > =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0stackblock() + startloc)); > > Also, in your change here, you used %j to print a size_t. =A0That will br= eak on > i386. It didn't actually break the build, but it's fine because I didn't test runtime (and I might have broken that)... I'll go with defacto standard as per printf(3). > You should use %z to print size_t's, but even better is to just use %t > to print a ptrdiff_t (which is the type that holds the difference of two > pointers). Ok. The overall temperature of using PRI* from POSIX seems like it's undesirable; is it just POSIX cruft that FreeBSD conforms to in theory only and doesn't really use in practice, or is there an example of real practical application where it's used in the sourcebase? > The various changes in jobs.c should use '%td' as well rather than (int) > casts. Ok. Tested build and runtime on amd64 and tested build-only with i386. Thanks! -Garrett --90e6ba212165f607a504926faffc Content-Type: application/octet-stream; name="fix-bin-sh-DEBUG-functionality.diff" Content-Disposition: attachment; filename="fix-bin-sh-DEBUG-functionality.diff" Content-Transfer-Encoding: base64 X-Attachment-Id: f_gf73rqou0 SW5kZXg6IGJpbi9zaC9NYWtlZmlsZQo9PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0tLSBiaW4vc2gvTWFrZWZpbGUJKHJl dmlzaW9uIDIxMzY4MCkKKysrIGJpbi9zaC9NYWtlZmlsZQkod29ya2luZyBjb3B5KQpAQCAtMjEs NyArMjEsNyBAQAogTEZMQUdTPSAtOAkjIDgtYml0IGxleCBzY2FubmVyIGZvciBhcml0aG1ldGlj CiBDRkxBR1MrPS1EU0hFTEwgLUkuIC1JJHsuQ1VSRElSfQogIyBmb3IgZGVidWc6Ci0jIENGTEFH Uys9IC1nIC1EREVCVUc9MgorI0RFQlVHX0ZMQUdTKz0JLWcgLURERUJVRz0yCiBXQVJOUz89CTIK IFdGT1JNQVQ9MAogCkluZGV4OiBiaW4vc2gvZXhwYW5kLmMKPT09PT09PT09PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PQotLS0gYmluL3No L2V4cGFuZC5jCShyZXZpc2lvbiAyMTM2ODApCisrKyBiaW4vc2gvZXhwYW5kLmMJKHdvcmtpbmcg Y29weSkKQEAgLTQzLDE0ICs0MywxNSBAQAogI2luY2x1ZGUgPHN5cy90eXBlcy5oPgogI2luY2x1 ZGUgPHN5cy90aW1lLmg+CiAjaW5jbHVkZSA8c3lzL3N0YXQuaD4KKyNpbmNsdWRlIDxkaXJlbnQu aD4KICNpbmNsdWRlIDxlcnJuby5oPgotI2luY2x1ZGUgPGRpcmVudC5oPgotI2luY2x1ZGUgPHVu aXN0ZC5oPgorI2luY2x1ZGUgPGludHR5cGVzLmg+CisjaW5jbHVkZSA8bGltaXRzLmg+CiAjaW5j bHVkZSA8cHdkLmg+CisjaW5jbHVkZSA8c3RkaW8uaD4KICNpbmNsdWRlIDxzdGRsaWIuaD4KLSNp bmNsdWRlIDxsaW1pdHMuaD4KLSNpbmNsdWRlIDxzdGRpby5oPgogI2luY2x1ZGUgPHN0cmluZy5o PgorI2luY2x1ZGUgPHVuaXN0ZC5oPgogCiAvKgogICogUm91dGluZXMgdG8gZXhwYW5kIGFyZ3Vt ZW50cyB0byBjb21tYW5kcy4gIFdlIGhhdmUgdG8gZGVhbCB3aXRoCkBAIC00OTcsOSArNDk4LDkg QEAKIAkJZXhpdHN0YXR1cyA9IHdhaXRmb3Jqb2IoaW4uanAsIChpbnQgKilOVUxMKTsKIAlpZiAo cXVvdGVkID09IDApCiAJCXJlY29yZHJlZ2lvbihzdGFydGxvYywgZGVzdCAtIHN0YWNrYmxvY2so KSwgMCk7Ci0JVFJBQ0UoKCJldmFsYmFja3E6IHNpemU9JWQ6IFwiJS4qc1wiXG4iLAotCQkoZGVz dCAtIHN0YWNrYmxvY2soKSkgLSBzdGFydGxvYywKLQkJKGRlc3QgLSBzdGFja2Jsb2NrKCkpIC0g c3RhcnRsb2MsCisJVFJBQ0UoKCJleHBiYWNrcTogc2l6ZT0ldGQ6IFwiJS4qc1wiXG4iLAorCQko KGRlc3QgLSBzdGFja2Jsb2NrKCkpIC0gc3RhcnRsb2MpLAorCQkoaW50KSAoKGRlc3QgLSBzdGFj a2Jsb2NrKCkpIC0gc3RhcnRsb2MpLAogCQlzdGFja2Jsb2NrKCkgKyBzdGFydGxvYykpOwogCWV4 cGRlc3QgPSBkZXN0OwogCUlOVE9OOwpJbmRleDogYmluL3NoL2pvYnMuYwo9PT09PT09PT09PT09 PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09PT09Ci0t LSBiaW4vc2gvam9icy5jCShyZXZpc2lvbiAyMTM2ODApCisrKyBiaW4vc2gvam9icy5jCSh3b3Jr aW5nIGNvcHkpCkBAIC0zOCwxOCArMzgsMTggQEAKICNpbmNsdWRlIDxzeXMvY2RlZnMuaD4KIF9f RkJTRElEKCIkRnJlZUJTRCQiKTsKIAorI2luY2x1ZGUgPHN5cy9pb2N0bC5oPgorI2luY2x1ZGUg PHN5cy9wYXJhbS5oPgorI2luY2x1ZGUgPHN5cy9yZXNvdXJjZS5oPgorI2luY2x1ZGUgPHN5cy9z dGRkZWYuaD4KKyNpbmNsdWRlIDxzeXMvdGltZS5oPgorI2luY2x1ZGUgPHN5cy93YWl0Lmg+Cisj aW5jbHVkZSA8ZXJybm8uaD4KICNpbmNsdWRlIDxmY250bC5oPgorI2luY2x1ZGUgPHBhdGhzLmg+ CiAjaW5jbHVkZSA8c2lnbmFsLmg+Ci0jaW5jbHVkZSA8ZXJybm8uaD4KLSNpbmNsdWRlIDxwYXRo cy5oPgorI2luY2x1ZGUgPHN0ZGxpYi5oPgogI2luY2x1ZGUgPHVuaXN0ZC5oPgotI2luY2x1ZGUg PHN0ZGxpYi5oPgotI2luY2x1ZGUgPHN5cy9wYXJhbS5oPgotI2luY2x1ZGUgPHN5cy93YWl0Lmg+ Ci0jaW5jbHVkZSA8c3lzL3RpbWUuaD4KLSNpbmNsdWRlIDxzeXMvcmVzb3VyY2UuaD4KLSNpbmNs dWRlIDxwYXRocy5oPgotI2luY2x1ZGUgPHN5cy9pb2N0bC5oPgogCiAjaW5jbHVkZSAic2hlbGwu aCIKICNpZiBKT0JTCkBAIC02ODAsOCArNjgwLDggQEAKIAkJanAtPnBzID0gJmpwLT5wczA7CiAJ fQogCUlOVE9OOwotCVRSQUNFKCgibWFrZWpvYiglcCwgJWQpIHJldHVybnMgJSUlZFxuIiwgKHZv aWQgKilub2RlLCBucHJvY3MsCi0JICAgIGpwIC0gam9idGFiICsgMSkpOworCVRSQUNFKCgibWFr ZWpvYiglcCwgJWQpIHJldHVybnMgJSUldGRcbiIsICh2b2lkICopbm9kZSwgbnByb2NzLAorCSAg ICAocHRyZGlmZl90KSAoanAgLSBqb2J0YWIgKyAxKSkpOwogCXJldHVybiBqcDsKIH0KIApAQCAt NzY2LDcgKzc2Niw3IEBACiAJcGlkX3QgcGlkOwogCXBpZF90IHBncnA7CiAKLQlUUkFDRSgoImZv cmtzaGVsbCglJSVkLCAlcCwgJWQpIGNhbGxlZFxuIiwganAgLSBqb2J0YWIsICh2b2lkICopbiwK KwlUUkFDRSgoImZvcmtzaGVsbCglJSV0ZCwgJXAsICVkKSBjYWxsZWRcbiIsIChwdHJkaWZmX3Qp IChqcCAtIGpvYnRhYiksICh2b2lkICopbiwKIAkgICAgbW9kZSkpOwogCUlOVE9GRjsKIAlpZiAo bW9kZSA9PSBGT1JLX0JHKQpAQCAtOTAzLDcgKzkwMyw3IEBACiAJaW50IHN0OwogCiAJSU5UT0ZG OwotCVRSQUNFKCgid2FpdGZvcmpvYiglJSVkKSBjYWxsZWRcbiIsIGpwIC0gam9idGFiICsgMSkp OworCVRSQUNFKCgid2FpdGZvcmpvYiglJSV0ZCkgY2FsbGVkXG4iLCAocHRyZGlmZl90KShqcCAt IGpvYnRhYiArIDEpKSk7CiAJd2hpbGUgKGpwLT5zdGF0ZSA9PSAwKQogCQlpZiAoZG93YWl0KDEs IGpwKSA9PSAtMSkKIAkJCWRvdHJhcCgpOwpAQCAtMTAwNCw3ICsxMDA0LDcgQEAKIAkJCWlmIChz dG9wcGVkKSB7CQkvKiBzdG9wcGVkIG9yIGRvbmUgKi8KIAkJCQlpbnQgc3RhdGUgPSBkb25lPyBK T0JET05FIDogSk9CU1RPUFBFRDsKIAkJCQlpZiAoanAtPnN0YXRlICE9IHN0YXRlKSB7Ci0JCQkJ CVRSQUNFKCgiSm9iICVkOiBjaGFuZ2luZyBzdGF0ZSBmcm9tICVkIHRvICVkXG4iLCBqcCAtIGpv YnRhYiArIDEsIGpwLT5zdGF0ZSwgc3RhdGUpKTsKKwkJCQkJVFJBQ0UoKCJKb2IgJXRkOiBjaGFu Z2luZyBzdGF0ZSBmcm9tICVkIHRvICVkXG4iLCAocHRyZGlmZl90KShqcCAtIGpvYnRhYiArIDEp LCBqcC0+c3RhdGUsIHN0YXRlKSk7CiAJCQkJCWpwLT5zdGF0ZSA9IHN0YXRlOwogCQkJCQlpZiAo anAgIT0gam9iKSB7CiAJCQkJCQlpZiAoZG9uZSAmJiAhanAtPnJlbWVtYmVyZWQgJiYK --90e6ba212165f607a504926faffc--