Date: Tue, 12 Oct 2010 11:31:36 -0700 From: Garrett Cooper <yanegomi@gmail.com> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org, Jilles Tjoelker <jilles@stack.nl> Subject: Re: [PATCH] Fix /bin/sh compilation with CFLAGS += -DDEBUG > 1 Message-ID: <AANLkTimq55BsTuKvFq7WHXKxtGk=Ny7L3JJ9fzOyOgDT@mail.gmail.com> In-Reply-To: <201010120830.19872.jhb@freebsd.org> References: <AANLkTinHi6jaLY%2BbZdnhL=gEY3hWGrzcfFG8nO6VMc5n@mail.gmail.com> <201010120830.19872.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--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 <jhb@freebsd.org> 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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimq55BsTuKvFq7WHXKxtGk=Ny7L3JJ9fzOyOgDT>