69424 2011-10-05 07:39:59 -0700 REGRESSION (r96595): WTFReportBacktrace listed as the top frame in all assertion backtraces 2011-10-05 08:51:12 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 1 aroben gavinp darin gavinp ggaren oldest_to_newest 478344 0 aroben 2011-10-05 07:39:59 -0700 To reproduce: 1. Insert an ASSERT_NOT_REACHED() in a commonly-called function 2. Run the program that calls the function The backtrace you get has WTFReportBacktrace listed as the top frame. 478345 1 aroben 2011-10-05 07:40:29 -0700 This started happening in <http://trac.webkit.org/changeset/96595>. Looks like we need to skip two frames instead of one now, since the stack trace includes both WTFGetBacktrace and WTFReportBacktrace. 478354 2 gavinp 2011-10-05 08:00:34 -0700 Better make it an argument, defaulting to one. Otherwise callers (who don't exist yet, but I'll add one soon) to WTFGetBacktrace will miss the possibly most important frame. 478363 3 109795 gavinp 2011-10-05 08:14:34 -0700 Created attachment 109795 Patch 478365 4 109795 gavinp 2011-10-05 08:15:49 -0700 Comment on attachment 109795 Patch With apologies, here's a fix that should make WTFReportBacktrace() go back to working, and leave WTFGetBacktrace at maximum usefulness. 478374 5 109795 aroben 2011-10-05 08:27:26 -0700 Comment on attachment 109795 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109795&action=review > Source/JavaScriptCore/wtf/Assertions.cpp:172 > *size = backtrace(stack, *size); We need to support skipFrames here, too. (If we can't implement skipFrames in WTFGetBacktrace on all ports, perhaps WTFReportBacktrace should just do the skipping on its own.) > Source/JavaScriptCore/wtf/Assertions.cpp:202 > + WTFGetBacktrace(samples, &frames, 2); > > for (int i = 1; i < frames; ++i) { This doesn't seem right. We tell WTFGetBacktrace to skip two frames, but then we skip a third frame when starting the loop. 478379 6 109795 gavinp 2011-10-05 08:34:20 -0700 Comment on attachment 109795 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109795&action=review >> Source/JavaScriptCore/wtf/Assertions.cpp:172 >> *size = backtrace(stack, *size); > > We need to support skipFrames here, too. (If we can't implement skipFrames in WTFGetBacktrace on all ports, perhaps WTFReportBacktrace should just do the skipping on its own.) You're right. >> Source/JavaScriptCore/wtf/Assertions.cpp:202 >> for (int i = 1; i < frames; ++i) { > > This doesn't seem right. We tell WTFGetBacktrace to skip two frames, but then we skip a third frame when starting the loop. It isn't. That's what I get for writing the fix too quickly; I'm testing another now. 478388 7 109803 gavinp 2011-10-05 08:42:01 -0700 Created attachment 109803 Patch 478390 8 109803 gavinp 2011-10-05 08:42:44 -0700 Comment on attachment 109803 Patch Now here is a better CL, the logic is kept in WTFReportBacktrace, so callers to WTFGetBacktrace should just expect to get WTFGetBacktrace + themselves on the stack. 478393 9 109803 aroben 2011-10-05 08:45:26 -0700 Comment on attachment 109803 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=109803&action=review > Source/JavaScriptCore/wtf/Assertions.cpp:199 > + static const int maxFrames = 31; > + static const int skipFrames = 2; > + void* samples[maxFrames + skipFrames]; > + int frames = maxFrames + skipFrames; I don't think the name maxFrames is so great anymore. We request more than maxFrames, after all. I'd suggest calling the two constants framesToShow and framesToSkip. Other than that this all looks good. 478398 10 gavinp 2011-10-05 08:51:12 -0700 Committed r96708: <http://trac.webkit.org/changeset/96708> 109795 2011-10-05 08:14:34 -0700 2011-10-05 08:41:56 -0700 Patch bug-69424-20111005111432.patch text/plain 3005 gavinp U3VidmVyc2lvbiBSZXZpc2lvbjogOTY2MDgKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGY2 NDk3OTEzOWFjYzkzNDUyYzNlZWVhZThhZTEzYmExZjVkNzg2ZDMuLmY1MTRkMTFiMTkxNjUzYzUz OTVhMGM4ODIwZTAzOTZlZDViNWEwZDYgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDE1IEBACisyMDExLTEwLTA1ICBHYXZpbiBQZXRlcnMgIDxnYXZpbnBAY2hyb21pdW0ub3Jn PgorCisgICAgICAgIFJFR1JFU1NJT04gKHI5NjU5NSk6IFdURlJlcG9ydEJhY2t0cmFjZSBsaXN0 ZWQgYXMgdGhlIHRvcCBmcmFtZSBpbiBhbGwgYXNzZXJ0aW9uIGJhY2t0cmFjZXMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY5NDI0CisKKyAgICAgICAg UGFyYW1ldGVyaXplIHRoZSBudW1iZXIgb2YgZnJhbWVzIHRvIHNraXAsIHNvIHRoYXQgV1RGUmVw b3J0QmFja3RyYWNlIGNhbiBub3QgcmVwb3J0IGl0c2VsZi4KKworICAgICAgICBSZXZpZXdlZCBi eSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIHd0Zi9Bc3NlcnRpb25zLmNwcDoKKyAgICAg ICAgKiB3dGYvQXNzZXJ0aW9ucy5oOgorCiAyMDExLTEwLTA0ICBDc2FiYSBPc3p0cm9nb27DoWMg IDxvc3N5QHdlYmtpdC5vcmc+CiAKICAgICAgICAgTWluR1cgd2FybmluZyBmaXggYWZ0ZXIgcjk2 Mjg2LgpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0Zi9Bc3NlcnRpb25zLmNw cCBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvQXNzZXJ0aW9ucy5jcHAKaW5kZXggY2M0Mjc2 MWFhZTBlOTIzZDc3NGM3NmNlNTQ1YTFhYWY4MzQxMTc5MC4uMTdiNmVkNTUwNmJhMzk0ZTllYzY0 NDFkNWVmYjY2NGIwYjczMjYzYSAxMDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL3d0 Zi9Bc3NlcnRpb25zLmNwcAorKysgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0Fzc2VydGlv bnMuY3BwCkBAIC0xNjYsNyArMTY2LDcgQEAgdm9pZCBXVEZSZXBvcnRBcmd1bWVudEFzc2VydGlv bkZhaWx1cmUoY29uc3QgY2hhciogZmlsZSwgaW50IGxpbmUsIGNvbnN0IGNoYXIqIGYKICAgICBw cmludENhbGxTaXRlKGZpbGUsIGxpbmUsIGZ1bmN0aW9uKTsKIH0KIAotdm9pZCBXVEZHZXRCYWNr dHJhY2Uodm9pZCoqIHN0YWNrLCBpbnQqIHNpemUpCit2b2lkIFdURkdldEJhY2t0cmFjZSh2b2lk Kiogc3RhY2ssIGludCogc2l6ZSwgaW50IHNraXBGcmFtZXMpCiB7CiAjaWYgT1MoREFSV0lOKSB8 fCBPUyhMSU5VWCkKICAgICAqc2l6ZSA9IGJhY2t0cmFjZShzdGFjaywgKnNpemUpOwpAQCAtMTgz LDcgKzE4Myw3IEBAIHZvaWQgV1RGR2V0QmFja3RyYWNlKHZvaWQqKiBzdGFjaywgaW50KiBzaXpl KQogICAgIFJ0bENhcHR1cmVTdGFja0JhY2tUcmFjZUZ1bmMgY2FwdHVyZVN0YWNrQmFja1RyYWNl RnVuYyA9IHJlaW50ZXJwcmV0X2Nhc3Q8UnRsQ2FwdHVyZVN0YWNrQmFja1RyYWNlRnVuYz4oCiAg ICAgICAgIDo6R2V0UHJvY0FkZHJlc3Moa2VybmVsMzIsICJSdGxDYXB0dXJlU3RhY2tCYWNrVHJh Y2UiKSk7CiAgICAgaWYgKGNhcHR1cmVTdGFja0JhY2tUcmFjZUZ1bmMpCi0gICAgICAgICpzaXpl ID0gY2FwdHVyZVN0YWNrQmFja1RyYWNlRnVuYygxLCAqc2l6ZSwgc3RhY2ssIDApOworICAgICAg ICAqc2l6ZSA9IGNhcHR1cmVTdGFja0JhY2tUcmFjZUZ1bmMoc2tpcEZyYW1lcywgKnNpemUsIHN0 YWNrLCAwKTsKICAgICBlbHNlCiAgICAgICAgICpzaXplID0gMDsKICNlbHNlCkBAIC0xOTcsNyAr MTk3LDcgQEAgdm9pZCBXVEZSZXBvcnRCYWNrdHJhY2UoKQogICAgIHZvaWQqIHNhbXBsZXNbbWF4 RnJhbWVzXTsKICAgICBpbnQgZnJhbWVzID0gbWF4RnJhbWVzOwogCi0gICAgV1RGR2V0QmFja3Ry YWNlKHNhbXBsZXMsICZmcmFtZXMpOworICAgIFdURkdldEJhY2t0cmFjZShzYW1wbGVzLCAmZnJh bWVzLCAyKTsKIAogICAgIGZvciAoaW50IGkgPSAxOyBpIDwgZnJhbWVzOyArK2kpIHsKICAgICAg ICAgY29uc3QgY2hhciogbWFuZ2xlZE5hbWUgPSAwOwpkaWZmIC0tZ2l0IGEvU291cmNlL0phdmFT Y3JpcHRDb3JlL3d0Zi9Bc3NlcnRpb25zLmggYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0Fz c2VydGlvbnMuaAppbmRleCA2NzRkYjQ5NjkwMWQzMGU4MzY0OWM3YmY4NmQwYWY5M2MyNjZlYTMy Li43YjRlOTVkMTJhYzE0ZjZkYWM4ZTE1ZWY0MWIwYzhmYjFiNGM4OGQ2IDEwMDY0NAotLS0gYS9T b3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0Fzc2VydGlvbnMuaAorKysgYi9Tb3VyY2UvSmF2YVNj cmlwdENvcmUvd3RmL0Fzc2VydGlvbnMuaApAQCAtMTQ5LDcgKzE0OSw3IEBAIFdURl9FWFBPUlRf UFJJVkFURSB2b2lkIFdURlJlcG9ydEVycm9yKGNvbnN0IGNoYXIqIGZpbGUsIGludCBsaW5lLCBj b25zdCBjaGFyKiBmCiBXVEZfRVhQT1JUX1BSSVZBVEUgdm9pZCBXVEZMb2coV1RGTG9nQ2hhbm5l bCosIGNvbnN0IGNoYXIqIGZvcm1hdCwgLi4uKSBXVEZfQVRUUklCVVRFX1BSSU5URigyLCAzKTsK IFdURl9FWFBPUlRfUFJJVkFURSB2b2lkIFdURkxvZ1ZlcmJvc2UoY29uc3QgY2hhciogZmlsZSwg aW50IGxpbmUsIGNvbnN0IGNoYXIqIGZ1bmN0aW9uLCBXVEZMb2dDaGFubmVsKiwgY29uc3QgY2hh ciogZm9ybWF0LCAuLi4pIFdURl9BVFRSSUJVVEVfUFJJTlRGKDUsIDYpOwogCi1XVEZfRVhQT1JU X1BSSVZBVEUgdm9pZCBXVEZHZXRCYWNrdHJhY2Uodm9pZCoqIHN0YWNrLCBpbnQqIHNpemUpOwor V1RGX0VYUE9SVF9QUklWQVRFIHZvaWQgV1RGR2V0QmFja3RyYWNlKHZvaWQqKiBzdGFjaywgaW50 KiBzaXplLCBpbnQgc2tpcEZyYW1lcyA9IDEpOwogV1RGX0VYUE9SVF9QUklWQVRFIHZvaWQgV1RG UmVwb3J0QmFja3RyYWNlKCk7CiAKICNpZmRlZiBfX2NwbHVzcGx1cwo= 109803 2011-10-05 08:42:01 -0700 2011-10-05 08:45:26 -0700 Patch bug-69424-20111005114159.patch text/plain 2238 gavinp U3VidmVyc2lvbiBSZXZpc2lvbjogOTY2MDgKZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0 Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGY2 NDk3OTEzOWFjYzkzNDUyYzNlZWVhZThhZTEzYmExZjVkNzg2ZDMuLmVjMjk3NGMwODQxNTQ5NmMz NWE1OGQ1MjdmZjI0OGRmNmQ1NDMzNzIgMTAwNjQ0Ci0tLSBhL1NvdXJjZS9KYXZhU2NyaXB0Q29y ZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRDb3JlL0NoYW5nZUxvZwpAQCAtMSwz ICsxLDE3IEBACisyMDExLTEwLTA1ICBHYXZpbiBQZXRlcnMgIDxnYXZpbnBAY2hyb21pdW0ub3Jn PgorCisgICAgICAgIFJFR1JFU1NJT04gKHI5NjU5NSk6IFdURlJlcG9ydEJhY2t0cmFjZSBsaXN0 ZWQgYXMgdGhlIHRvcCBmcmFtZSBpbiBhbGwgYXNzZXJ0aW9uIGJhY2t0cmFjZXMKKyAgICAgICAg aHR0cHM6Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTY5NDI0CisKKyAgICAgICAg U2tpcCBhbiBleHRyYSBmcmFtZSBpbiBXVEZSZXBvcnRCYWNrdHJhY2UuICBBcyB3ZWxsLCBJIG5v dyBkb24ndCBjb3VudCBza2lwcGVkIGZyYW1lcyBpbiBtYXhGcmFtZXMsCisgICAgICAgIHNvIEkn dmUgdXBkYXRlZCBtYXhGcmFtZXMgdG8gMzEsIGFzIHdpdGggb25lIHNraXBwZWQgZnJhbWUgdGhl IHByZXZpb3VzIHZhbHVlIHdhcyBlZmZlY3RpdmVseQorICAgICAgICAzMSByZXBvcnRlZCBmcmFt ZXMuCisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BTISkuCisKKyAgICAgICAgKiB3 dGYvQXNzZXJ0aW9ucy5jcHA6CisgICAgICAgICogd3RmL0Fzc2VydGlvbnMuaDoKKwogMjAxMS0x MC0wNCAgQ3NhYmEgT3N6dHJvZ29uw6FjICA8b3NzeUB3ZWJraXQub3JnPgogCiAgICAgICAgIE1p bkdXIHdhcm5pbmcgZml4IGFmdGVyIHI5NjI4Ni4KZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2Ny aXB0Q29yZS93dGYvQXNzZXJ0aW9ucy5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvd3RmL0Fz c2VydGlvbnMuY3BwCmluZGV4IGNjNDI3NjFhYWUwZTkyM2Q3NzRjNzZjZTU0NWExYWFmODM0MTE3 OTAuLmE2YzZiNDM2ODZlMGVkNDA4OTU5MDc3NTUxZjQwMzE3Yjk0OWVmOGEgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS93dGYvQXNzZXJ0aW9ucy5jcHAKKysrIGIvU291cmNlL0ph dmFTY3JpcHRDb3JlL3d0Zi9Bc3NlcnRpb25zLmNwcApAQCAtMTgzLDcgKzE4Myw3IEBAIHZvaWQg V1RGR2V0QmFja3RyYWNlKHZvaWQqKiBzdGFjaywgaW50KiBzaXplKQogICAgIFJ0bENhcHR1cmVT dGFja0JhY2tUcmFjZUZ1bmMgY2FwdHVyZVN0YWNrQmFja1RyYWNlRnVuYyA9IHJlaW50ZXJwcmV0 X2Nhc3Q8UnRsQ2FwdHVyZVN0YWNrQmFja1RyYWNlRnVuYz4oCiAgICAgICAgIDo6R2V0UHJvY0Fk ZHJlc3Moa2VybmVsMzIsICJSdGxDYXB0dXJlU3RhY2tCYWNrVHJhY2UiKSk7CiAgICAgaWYgKGNh cHR1cmVTdGFja0JhY2tUcmFjZUZ1bmMpCi0gICAgICAgICpzaXplID0gY2FwdHVyZVN0YWNrQmFj a1RyYWNlRnVuYygxLCAqc2l6ZSwgc3RhY2ssIDApOworICAgICAgICAqc2l6ZSA9IGNhcHR1cmVT dGFja0JhY2tUcmFjZUZ1bmMoMCwgKnNpemUsIHN0YWNrLCAwKTsKICAgICBlbHNlCiAgICAgICAg ICpzaXplID0gMDsKICNlbHNlCkBAIC0xOTMsMTMgKzE5MywxNCBAQCB2b2lkIFdURkdldEJhY2t0 cmFjZSh2b2lkKiogc3RhY2ssIGludCogc2l6ZSkKIAogdm9pZCBXVEZSZXBvcnRCYWNrdHJhY2Uo KQogewotICAgIHN0YXRpYyBjb25zdCBpbnQgbWF4RnJhbWVzID0gMzI7Ci0gICAgdm9pZCogc2Ft cGxlc1ttYXhGcmFtZXNdOwotICAgIGludCBmcmFtZXMgPSBtYXhGcmFtZXM7CisgICAgc3RhdGlj IGNvbnN0IGludCBtYXhGcmFtZXMgPSAzMTsKKyAgICBzdGF0aWMgY29uc3QgaW50IHNraXBGcmFt ZXMgPSAyOworICAgIHZvaWQqIHNhbXBsZXNbbWF4RnJhbWVzICsgc2tpcEZyYW1lc107CisgICAg aW50IGZyYW1lcyA9IG1heEZyYW1lcyArIHNraXBGcmFtZXM7CiAKICAgICBXVEZHZXRCYWNrdHJh Y2Uoc2FtcGxlcywgJmZyYW1lcyk7CiAKLSAgICBmb3IgKGludCBpID0gMTsgaSA8IGZyYW1lczsg KytpKSB7CisgICAgZm9yIChpbnQgaSA9IHNraXBGcmFtZXM7IGkgPCBmcmFtZXM7ICsraSkgewog ICAgICAgICBjb25zdCBjaGFyKiBtYW5nbGVkTmFtZSA9IDA7CiAgICAgICAgIGNoYXIqIGN4YURl bWFuZ2xlZCA9IDA7CiAK